Skip to content

Conversation

@alex-page
Copy link
Member

@alex-page alex-page commented Jan 29, 2023

WHY are these changes introduced?

NodeJS 14 is EOL and we are removing support in v11.

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.

WHAT is this pull request doing?

  • Increasing the supported versions of NodeJS
  • Add the engines field for NodeJS to package.json files

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:

(plugin babel) BrowserslistError: Unknown version 16.19 of Node.js

How to 🎩

  • CI completes successfully

@alex-page alex-page changed the base branch from main to bump-build-deps January 29, 2023 23:17
@alex-page alex-page added 🤖Skip Changelog Causes CI to ignore changelog update check. and removed 🤖Skip Changelog Causes CI to ignore changelog update check. labels Jan 29, 2023
@alex-page alex-page marked this pull request as ready for review January 29, 2023 23:24
@github-actions
Copy link
Contributor

github-actions bot commented Jan 29, 2023

size-limit report 📦

Path Size
polaris-react-cjs 212.06 KB (0%)
polaris-react-esm 136.31 KB (0%)
polaris-react-esnext 189.85 KB (0%)
polaris-react-css 40.63 KB (0%)

### 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',
Copy link
Member Author

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...

Base automatically changed from bump-build-deps to v11-major January 30, 2023 00:11
Comment on lines -119 to -124
// 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'},
],
Copy link
Member Author

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.

Copy link
Member

@sam-b-rose sam-b-rose left a 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.

@alex-page alex-page changed the title 2/x Bump NodeJS supported versions to 16.16 | 18.13 2/x Bump NodeJS supported versions to ^16.16 || ^18.13 Jan 30, 2023
@alex-page alex-page merged commit 89c8ae4 into v11-major Jan 30, 2023
@alex-page alex-page deleted the bump-nodejs branch January 30, 2023 01:27
@alex-page alex-page changed the title 2/x Bump NodeJS supported versions to ^16.16 || ^18.13 Bump NodeJS supported versions to ^16.16 || ^18.13 and add engines to package.json Jan 30, 2023
Copy link
Member

@BPScott BPScott left a 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"
Copy link
Member

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"
Copy link
Member

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.

sam-b-rose added a commit that referenced this pull request May 26, 2023
## @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>
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
## @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>
ascherkus pushed a commit to ascherkus/polaris that referenced this pull request Feb 19, 2025
## @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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖Skip Changelog Causes CI to ignore changelog update check.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants