-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Bump NodeJS supported versions to ^16.16 || ^18.13 and add engines to package.json #8201
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
size-limit report 📦
|
### WHY are these changes introduced? We currently set `engines` on the root `package.json`. The problem with this is that consumers of the library do not get this information and they can use the libraries without any warning of what version of NodeJS it supports. This change adds the engines field to each package to ensure our consumers are using supported versions of NodeJS. ### WHAT is this pull request doing? Adding the engines field to package.json files
| /** @type {import('rollup').RollupOptions} */ | ||
| export default [ | ||
| generateConfig({ | ||
| targets: 'extends @shopify/browserslist-config, node 12.20', |
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.
This scared me but nothing seems to be bad since changing the version...
| // We could omit this if we set `engines` fields properly | ||
| // As we don't set them then eslint thinks we're using node 8 | ||
| 'node/no-unsupported-features/node-builtins': [ | ||
| 'error', | ||
| {version: '>=16.0.0'}, | ||
| ], |
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.
We set the engines field now so this is no longer needed.
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.
Looks good! I did see that the @types/node in the polaris-for-vscode/package.json is on ^14. I think it might be good to update that as well to 16 or even 18.
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.
The versions chosen seem suprising to me
- Using "^16.16.0 || ^18.13.0" for the per-package engines shall block installation on node 19 (and 20 when it comes out in April). Currently supported and future node versions seems like a bad idea.
- Node 14 is still supported till April. I'm ok with preemptively dropping support if you feel like this shall help improve something.
- The first LTS version of v16 was 16.13, and the first LTS version of 18 was 18.12. Is there a reason why we're choosing to claim minimum polaris versions are 16.16 and 18.13?
| ], | ||
| "engines": { | ||
| "vscode": "^1.64.0" | ||
| "node": "^16.16.0 || ^18.13.0" |
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.
This seems incorrect
| "engine-strict": true, | ||
| "engines": { | ||
| "node": ">=14.13.1" | ||
| "node": "^16.16.0 || ^18.13.0" |
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.
By setting the engines on the specific node packages, they shall refuse to install if the node version does not match these versions. This is great for blocking installation on older unsupported versions, however the configuration of "^16.16.0 || ^18.13.0" means "Only node 16 or 18" - you're currently also blocking the installation of these packages on the currently supported node 19 (and the lts node 20 when it comes out in April).
For these per-package engine fields I strongly think we should allow current and future versions for the sake of forwards compatibility - ^16.16.0 || >=18.13.0.
## @shopify/polaris v11.0.0 ### Dependencies - [x] #8200 ### NodeJS - [x] #8201 ### TypeScript - [x] #8203 ### Components - [x] #7349 - [x] #7397 - [x] #7962 - [x] #8187 - [x] #8184 - [x] #8206 - [x] #7990 - [x] #8468 - [x] #8577 - [x] #8631 - [x] #8962 ## @shopify/polaris-tokens v7.0.0 ### Tokens - [x] #6920 - [x] #8245 - [x] #4826 - [x] #8405 ## @shopify/stylelint-polaris v7.0.0 - [x] #7622 - [x] #8419 # Post @shopify/polaris v11 shipping - [ ] #8420 ## Low priority or not ready breaking changes - [x] Remove deprecated layout components - [x] Release Layout primitive components --------- Co-authored-by: Tim Layton <tmlayton@users.noreply.github.com> Co-authored-by: Ryan Musgrave <ryan.musgrave@shopify.com> Co-authored-by: Ryan Musgrave <ryanm128@gmail.com> Co-authored-by: aveline <aveline@users.noreply.github.com> Co-authored-by: Kyle Durand <kyledurand@users.noreply.github.com> Co-authored-by: Matt Gregg <matt.gregg@shopify.com> Co-authored-by: Alex Page <hi@alexpage.dev> Co-authored-by: Lo Kim <lo.kim@shopify.com> Co-authored-by: Ben Scott <227292+BPScott@users.noreply.github.com> Co-authored-by: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com> Co-authored-by: Sam Rose <11774595+samrose3@users.noreply.github.com> Co-authored-by: Sam Rose <sam.rose@shopify.com> Co-authored-by: Marc Thomas <marc.thomas@shopify.com> Co-authored-by: Alex Page <19199063+alex-page@users.noreply.github.com> Co-authored-by: Chloe Rice <18447883+chloerice@users.noreply.github.com> Co-authored-by: Chloe Rice <chloerice@users.noreply.github.com> Co-authored-by: Joe Thomas <joe.thomas@shopify.com> Co-authored-by: Yuraima Estevez <yuraima.estevez@shopify.com> Co-authored-by: shopify-github-actions-access[bot] <109624739+shopify-github-actions-access[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
## @shopify/polaris v11.0.0 ### Dependencies - [x] Shopify#8200 ### NodeJS - [x] Shopify#8201 ### TypeScript - [x] Shopify#8203 ### Components - [x] Shopify#7349 - [x] Shopify#7397 - [x] Shopify#7962 - [x] Shopify#8187 - [x] Shopify#8184 - [x] Shopify#8206 - [x] Shopify#7990 - [x] Shopify#8468 - [x] Shopify#8577 - [x] Shopify#8631 - [x] Shopify#8962 ## @shopify/polaris-tokens v7.0.0 ### Tokens - [x] Shopify#6920 - [x] Shopify#8245 - [x] Shopify#4826 - [x] Shopify#8405 ## @shopify/stylelint-polaris v7.0.0 - [x] Shopify#7622 - [x] Shopify#8419 # Post @shopify/polaris v11 shipping - [ ] Shopify#8420 ## Low priority or not ready breaking changes - [x] Remove deprecated layout components - [x] Release Layout primitive components --------- Co-authored-by: Tim Layton <tmlayton@users.noreply.github.com> Co-authored-by: Ryan Musgrave <ryan.musgrave@shopify.com> Co-authored-by: Ryan Musgrave <ryanm128@gmail.com> Co-authored-by: aveline <aveline@users.noreply.github.com> Co-authored-by: Kyle Durand <kyledurand@users.noreply.github.com> Co-authored-by: Matt Gregg <matt.gregg@shopify.com> Co-authored-by: Alex Page <hi@alexpage.dev> Co-authored-by: Lo Kim <lo.kim@shopify.com> Co-authored-by: Ben Scott <227292+BPScott@users.noreply.github.com> Co-authored-by: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com> Co-authored-by: Sam Rose <11774595+samrose3@users.noreply.github.com> Co-authored-by: Sam Rose <sam.rose@shopify.com> Co-authored-by: Marc Thomas <marc.thomas@shopify.com> Co-authored-by: Alex Page <19199063+alex-page@users.noreply.github.com> Co-authored-by: Chloe Rice <18447883+chloerice@users.noreply.github.com> Co-authored-by: Chloe Rice <chloerice@users.noreply.github.com> Co-authored-by: Joe Thomas <joe.thomas@shopify.com> Co-authored-by: Yuraima Estevez <yuraima.estevez@shopify.com> Co-authored-by: shopify-github-actions-access[bot] <109624739+shopify-github-actions-access[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
## @shopify/polaris v11.0.0 ### Dependencies - [x] Shopify#8200 ### NodeJS - [x] Shopify#8201 ### TypeScript - [x] Shopify#8203 ### Components - [x] Shopify#7349 - [x] Shopify#7397 - [x] Shopify#7962 - [x] Shopify#8187 - [x] Shopify#8184 - [x] Shopify#8206 - [x] Shopify#7990 - [x] Shopify#8468 - [x] Shopify#8577 - [x] Shopify#8631 - [x] Shopify#8962 ## @shopify/polaris-tokens v7.0.0 ### Tokens - [x] Shopify#6920 - [x] Shopify#8245 - [x] Shopify#4826 - [x] Shopify#8405 ## @shopify/stylelint-polaris v7.0.0 - [x] Shopify#7622 - [x] Shopify#8419 # Post @shopify/polaris v11 shipping - [ ] Shopify#8420 ## Low priority or not ready breaking changes - [x] Remove deprecated layout components - [x] Release Layout primitive components --------- Co-authored-by: Tim Layton <tmlayton@users.noreply.github.com> Co-authored-by: Ryan Musgrave <ryan.musgrave@shopify.com> Co-authored-by: Ryan Musgrave <ryanm128@gmail.com> Co-authored-by: aveline <aveline@users.noreply.github.com> Co-authored-by: Kyle Durand <kyledurand@users.noreply.github.com> Co-authored-by: Matt Gregg <matt.gregg@shopify.com> Co-authored-by: Alex Page <hi@alexpage.dev> Co-authored-by: Lo Kim <lo.kim@shopify.com> Co-authored-by: Ben Scott <227292+BPScott@users.noreply.github.com> Co-authored-by: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com> Co-authored-by: Sam Rose <11774595+samrose3@users.noreply.github.com> Co-authored-by: Sam Rose <sam.rose@shopify.com> Co-authored-by: Marc Thomas <marc.thomas@shopify.com> Co-authored-by: Alex Page <19199063+alex-page@users.noreply.github.com> Co-authored-by: Chloe Rice <18447883+chloerice@users.noreply.github.com> Co-authored-by: Chloe Rice <chloerice@users.noreply.github.com> Co-authored-by: Joe Thomas <joe.thomas@shopify.com> Co-authored-by: Yuraima Estevez <yuraima.estevez@shopify.com> Co-authored-by: shopify-github-actions-access[bot] <109624739+shopify-github-actions-access[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
WHY are these changes introduced?
NodeJS 14 is EOL and we are removing support in v11.
We currently set
engineson the rootpackage.json. The problem with this is that consumers of the library do not get this information and they can use the libraries without any warning of what version of NodeJS it supports.WHAT is this pull request doing?
I would like to get NodeJS to version 16.19 before we launch version 11. However there is an issue with babel/rollup/browserslist not finding the latest version of NodeJS:
How to 🎩