2966: Migrating to Modern Yarn #2968
Conversation
bahaaTuffaha
requested review from
steffenkleinle,
ztefanie,
f1sh1918 and
LeandraH
as code owners
|
Thank you, I probably won't manage to have time today to take a look at it but maybe also check out this old PR that was never merged to see what went wrong there and if this PR fixes the issues: #2588 |
I found out it needs to be bit more in the oven.. sorry.. yarn test is not working I'm investigating...I will convert this PR to Draft for now |
| @@ -39,7 +39,7 @@ def determineBuildConfigName() { | |||
| } | |||
|
|
|||
| def createYarnProcess(buildConfigName, platform, command) { | |||
| return execCommand("yarn workspace --silent build-configs --silent manage $command $buildConfigName $platform") | |||
| return execCommand("yarn workspace build-configs manage $command $buildConfigName $platform") | |||
There was a problem hiding this comment.
❓ Why did you remove the silent flags?
There was a problem hiding this comment.
Yarn complains about them and I didn't find any mention of it in the docs.
And I tried this also:
yarnpkg/yarn#788 (comment)
There was a problem hiding this comment.
Interesting, looks like it has been completely removed, thanks for looking into it!
yarnpkg/berry#3542
package.json
Outdated
| "lint": "yarn g:lint", | ||
| "g:lint": "eslint --cache --cache-location .eslintcache . && yarn workspace web stylelint && yarn workspace native stylelint", |
There was a problem hiding this comment.
❓ Why did you decide on those two different commands that do the same thing?
There was a problem hiding this comment.
😅 one for the current dir and the other for other workspaces...
There was a problem hiding this comment.
Two things:
- This is at root, so there isn't really any code in this directory to be linted.
- If I now execute
yarn lintin the root directory, yarn checks inpackage.jsonwhat to do for the commandlintand sees that it should executeyarn g:lint. Which is the command in line 18. So both commands do the same thing with line 17 just being a reference to line 18. I don't think that's necessary.
| @@ -36,6 +35,7 @@ | |||
| "build-configs": "0.0.1", | |||
| "dompurify": "^3.1.6", | |||
| "focus-trap-react": "^10.2.3", | |||
| "htmlparser2": "^9.1.0", | |||
There was a problem hiding this comment.
There was an error that has to do with htmlparser2 so I added this dependency
| @@ -87,6 +87,7 @@ | |||
| "raf": "^3.4.1", | |||
| "react-refresh": "^0.14.2", | |||
| "react-refresh-typescript": "^2.0.9", | |||
| "rrule": "2.8.1", | |||
There was a problem hiding this comment.
Ok I didn't find a way to let the dev dependences use the ones at the root folder thats way you will see that I added jest, stylelint,typescript and prettier
- I know its duplicated but what else to do? and its a dev-dependency after all.
| @@ -3,16 +3,28 @@ | |||
| const { execSync } = require('child_process') | |||
| const { program } = require('commander') | |||
|
|
|||
| function parseBashVariables(str) { | |||
There was a problem hiding this comment.
I know this sounds bad but this what chat suggested to solve a problem that has to do with a long command in cmd (Error: spawnSync C:\WINDOWS\system32\cmd.exe ENAMETOOLONG
) when I run "start:malte" or "android:malte"
There was a problem hiding this comment.
Thanks for your PR. The reason why we were not able yet to upgrade yarn to modern were memory problems in the CI (see yarnpkg/berry#3996 and #2026). Did you check that this issue is solved? Otherwise this will cost us lots of CI credits if we have to upgrade to bigger images. Therefore, please make sure that it still works in the CI. I sadly doubt that this issue is fixed as yarnpkg/berry#3996 is still open.
Furthermore, please revert all changes that are not necessary, e.g.
- changes to autogenerated fastlane readmes
- adding shared dependencies to web
- removing tools yarn.lock
- updating gradle (should only be done in react-native upgrades)
- and others
Not sure if it is worth to invest the time though as long as the memory problem exists :/
Short description
Upgraded to
[email protected]Proposed changes
yarnrc.yml will have
nodeLinker: node-modulesmeaning its not depending on pnp more on that at (https://yarnpkg.com/features/pnp)Note we can do hybrid pnp in the future look at https://yarnpkg.com/getting-started/recipes#hybrid-pnp--node_modules-mono-repo
nohoistis deprecated so I removed it and addednmHoistingLimits: workspacesThis yarn version at some places it complains about
--silentso I removed some of them. Still looking for a way to silent these warnings.Note: not tested on iOS and focused mainly on integrate native and integrate web app.
Side effects
Not sure
Testing
I used this time: Node 22.9.0 you can install by nvm
before
yarn installmake sure to runcorepack enableonce to activate Corepack.I suggest to delete all node modules files from the repo then do yarn install.
Resolved issues
Fixes: #2966