-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[chore]: Upgrade to yarn 4 #2610
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for keplergl2 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -10,7 +10,9 @@ umd/ | |||
|
|||
typedoc/ | |||
|
|||
*/**/yarn.lock | |||
examples/**/yarn.lock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are the yarn4 gitignore recommendations that I have been following in the vis.gl repos https://github.com/visgl/luma.gl/blob/master/.gitignore#L33
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am trying different things to get netlify deployment works on this PR. So far I noticed if I don't have yarn.lock in the website and examples/demo-app folder, running yarn install in these two folders to build the website will fail with a 'issing yarn.lock' message
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes you should check in the yarn.lock file
@@ -70,5 +74,6 @@ | |||
"peerDependencies": { | |||
"react": "18.x", | |||
"react-dom": "18.x" | |||
} | |||
}, | |||
"packageManager": "[email protected]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had issues mixing volta and packageManager. I personally prefer volta as it makes it way easier to move between repos and directories.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is possible to add a volta section that extends the base package.json, but I got some trouble when adding website to workspaces, so keep a separate volta section in website. Just my experience...
|
||
- name: Use Node.js 12.x | ||
uses: actions/setup-node@v3 | ||
uses: actions/setup-node@v4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are using volta locally, don't we want volta to be used in CI too?
https://github.com/visgl/luma.gl/blob/master/.github/workflows/test.yml#L19
.yarnrc.yml
Outdated
# https://yarnpkg.com/configuration/yarnrc | ||
nodeLinker: node-modules | ||
# Define the registry to use when fetching packages. | ||
npmRegistryServer: "https://registry.yarnpkg.com" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surprised that registry entry is needed.
website/package.json
Outdated
"node": ">=10.0.0" | ||
"node": ">=18" | ||
}, | ||
"volta": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep engines and volta at the bottom of package.json? Just used to having them there. Or at least together with the packageManager field, I missed these in my first read through.
@@ -10,7 +10,9 @@ umd/ | |||
|
|||
typedoc/ | |||
|
|||
*/**/yarn.lock | |||
examples/**/yarn.lock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes you should check in the yarn.lock file
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
…script Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
8402b6f
to
60fc619
Compare
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
…package.json Signed-off-by: Shan He <[email protected]>
…files Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
…for testing Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
Signed-off-by: Shan He <[email protected]>
Congrats, that was quite an epic PR! |
Uh oh!
There was an error while loading. Please reload this page.