Skip to content
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

Bump NodeJS supported versions to ^16.16 || ^18.13 and add engines to package.json #8201

Merged
merged 15 commits into from
Jan 30, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,6 @@ module.exports = {
'import/extensions': 'off',
'import/no-default-export': 'off',
'import/no-anonymous-default-export': 'off',
// 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'},
],
Comment on lines -119 to -124
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.

},
},
{
Expand Down
2 changes: 1 addition & 1 deletion .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ Each contributor is required to [sign a CLA](https://cla.shopify.com/). This pro

### Contribution prerequisites

- You have Node installed at v16.13.0+ and Yarn at v1.22.18+
- You have Node installed at v18.13.0+ and Yarn at v1.22.18+

### Best practices

Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/changelog.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ jobs:
with:
fetch-depth: 0

- name: Setup Node with v16.13.0
- name: Setup Node with v18.13.0
uses: actions/setup-node@v3
with:
node-version: 16.13.0
node-version: 18.13.0

- name: Check for Changeset
run: npx @changesets/cli status --since="origin/main"
8 changes: 4 additions & 4 deletions .github/workflows/ci-a11y-vrt.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ jobs:
sudo rm -rf "/usr/local/share/boost"
sudo rm -rf "$AGENT_TOOLSDIRECTORY"

- name: Setup Node with v16.13.0
- name: Setup Node with v18.13.0
uses: actions/setup-node@v3
with:
node-version: 16.13.0
node-version: 18.13.0
cache: yarn

- name: Restore cache
Expand Down Expand Up @@ -72,10 +72,10 @@ jobs:
with:
fetch-depth: 0 # Chromatic git history to track changes

- name: Setup Node with v16.13.0
- name: Setup Node with v18.13.0
uses: actions/setup-node@v3
with:
node-version: 16.13.0
node-version: 18.13.0
cache: yarn

- name: Restore cache
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
node-version: ['14.18.0', '16.13.0']
node-version: ['16.16.0', '18.13.0']
steps:
- name: Checkout branch
uses: actions/checkout@v3
Expand Down
2 changes: 1 addition & 1 deletion .nvmrc
Original file line number Diff line number Diff line change
@@ -1 +1 @@
v16.13.0
v18.13.0
2 changes: 1 addition & 1 deletion dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: polaris
up:
- node:
yarn: v1.22.18
version: v16.13.0 # to be kept in sync with .nvmrc and .github/workflows/ci.yml
version: v18.13.0 # to be kept in sync with .nvmrc and .github/workflows/ci.yml
- custom:
name: Build monorepo
met?: yarn build --output-logs=none
Expand Down
16 changes: 8 additions & 8 deletions documentation/Nodejs support.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,38 +16,38 @@ The `package.json` engines. This should match the `.github/workflows/ci.yml` and

```json
"engines": {
"node": "^14.18.0 || ^16.13.0"
"node": "^16.16.0 || ^18.13.0"
},
```

The GitHub actions `.github/workflows/ci.yml` file. This should match the `package.json` and list all supported versions.

```yml
node-version: ['14.18.0', '16.13.0']
node-version: ['16.16.0', '18.13.0']
```

The `dev.yml` file which creates a local development environment. This should match the `.nvmrc` file.

```yml
version: v16.13.0
version: v18.13.0
```

The `.nvmrc` file for local development. This should match the `dev.yml` file.

```
v16.13.0
v18.13.0
```

The `shipit.yml` files. This should point towards the GitHub Actions the packages require to pass before publishing.

```yml
ci:
require:
- 'Validate with Node v14.18.0'
- 'Validate with Node v16.13.0'
- 'Validate with Node v16.16.0'
- 'Validate with Node v18.13.0'

merge:
require:
- 'Validate with Node v14.18.0'
- 'Validate with Node v16.13.0'
- 'Validate with Node v16.16.0'
- 'Validate with Node v18.13.0'
```
5 changes: 5 additions & 0 deletions documentation/guides/migrating-from-v10-to-v11.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@ Polaris v11.0.0 ([full release notes](https://github.com/Shopify/polaris/release

## Table of Contents

- [Node support](#node-support)
- [Components](#components)
- [Removed deprecated Collapsible argument](#removed-deprecated-collapsible-argument)
- [Removed KonamiCode component](#removed-konamicode-component)
- [Tokens](#tokens)

## Node support

NodeJS version 14 is no longer supported. NodeJS 18 is recommended and 16 is supported.

## Components

### Removed deprecated Collapsible argument
Expand Down
30 changes: 16 additions & 14 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"version": "0.0.0",
"private": true,
"engines": {
"node": "^14.18.0 || ^16.13.0"
"node": "^16.16.0 || ^18.13.0"
},
"workspaces": {
"packages": [
Expand Down Expand Up @@ -44,28 +44,30 @@
"postinstall": "patch-package"
},
"devDependencies": {
"@babel/core": "^7.15.0",
"@babel/node": "^7.14.9",
"@babel/core": "^7.20.12",
"@babel/node": "^7.20.7",
"@babel/preset-typescript": "^7.18.6",
"@changesets/changelog-github": "^0.4.4",
"@changesets/cli": "^2.23.0",
"@changesets/get-release-plan": "^3.0.14",
"@next/eslint-plugin-next": "^12.1.4",
"@rollup/plugin-babel": "^5.3.1",
"@rollup/plugin-commonjs": "^21.1.0",
"@rollup/plugin-image": "^2.0.5",
"@rollup/plugin-json": "^4.1.0",
"@rollup/plugin-node-resolve": "^13.2.1",
"@rollup/plugin-replace": "^2.3.3",
"@rollup/plugin-virtual": "^2.0.3",
"@rollup/pluginutils": "^4.1.0",
"@shopify/babel-preset": "^24.1.2",
"@rollup/plugin-babel": "^6.0.3",
"@rollup/plugin-commonjs": "^24.0.1",
"@rollup/plugin-image": "^3.0.2",
"@rollup/plugin-json": "^6.0.0",
"@rollup/plugin-node-resolve": "^15.0.1",
"@rollup/plugin-replace": "^5.0.2",
"@rollup/plugin-virtual": "^3.0.1",
"@rollup/pluginutils": "^5.0.2",
"@shopify/babel-preset": "^25.0.0",
"@shopify/cli": "^3.10.1",
"@shopify/eslint-plugin": "^42.0.1",
"@shopify/prettier-config": "^1.1.2",
"@shopify/stylelint-plugin": "^11.0.0",
"@shopify/typescript-configs": "^5.1.0",
"@size-limit/preset-small-lib": "^5.0.3",
"@types/jest": "^27.5.1",
"babel-loader": "^9.1.2",
"downlevel-dts": "^0.6.0",
"eslint": "^8.3.0",
"execa": "^5.0.0",
Expand All @@ -76,8 +78,8 @@
"patch-package": "^6.4.7",
"postinstall-postinstall": "^2.1.0",
"prettier": "^2.5.0",
"rollup": "^2.70.2",
"rollup-plugin-node-externals": "^4.0.0",
"rollup": "^3.12.0",
"rollup-plugin-node-externals": "^5.1.0",
"size-limit": "^5.0.3",
"stylelint": "^14.15.0",
"ts-node": "^10.7.0",
Expand Down
2 changes: 1 addition & 1 deletion polaris-cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
},
"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.

},
"os": [
"darwin",
Expand Down
2 changes: 1 addition & 1 deletion polaris-for-vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"shopify"
],
"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

},
"categories": [
"Other"
Expand Down
3 changes: 3 additions & 0 deletions polaris-icons/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
]
}
},
"engines": {
"node": "^16.16.0 || ^18.13.0"
},
"scripts": {
"build": "rollup -c",
"dev": "rollup -c -w",
Expand Down
20 changes: 11 additions & 9 deletions polaris-icons/rollup.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -222,24 +222,26 @@ export default [
interop,
entryFileNames: '[name].js',
chunkFileNames: '[name].js',
manualChunks: (id) => {
if (id.startsWith(iconBasePath)) {
return id.replace(iconBasePath, 'icons/');
}
},
},
{
dir: 'dist',
format: 'esm',
interop,
entryFileNames: '[name].mjs',
chunkFileNames: '[name].mjs',
manualChunks: (id) => {
if (id.startsWith(iconBasePath)) {
return id.replace(iconBasePath, 'icons/');
}
},
},
],
manualChunks: (id) => {
// Generate distinct chunks for each icon
// This allows consuming apps to split up the icons into multiple subchunks
// containing a few icons each instead of always having to put every icon
// into a single shared chunk
if (id.startsWith(iconBasePath)) {
return id.replace(iconBasePath, 'icons/');
}
},

external: ['react'],
onwarn: (warning, warn) => {
// Unresolved imports means Rollup couldn't find an import, possibly because
Expand Down
7 changes: 3 additions & 4 deletions polaris-migrator/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
"author": "Shopify <[email protected]>",
"homepage": "https://polaris.shopify.com",
"repository": "https://github.com/Shopify/polaris",
"engines": {
"node": "^16.16.0 || ^18.13.0"
},
"bugs": {
"url": "https://github.com/Shopify/polaris/issues"
},
Expand Down Expand Up @@ -48,10 +51,6 @@
"devDependencies": {
"@types/is-git-clean": "^1.1.0",
"@types/jscodeshift": "^0.11.5",
"@rollup/plugin-babel": "^5.3.1",
"@rollup/plugin-commonjs": "^22.0.2",
"@rollup/plugin-json": "^4.1.0",
"@rollup/plugin-node-resolve": "^13.3.0",
"@shopify/polaris": "^10.24.0",
"plop": "^3.1.1",
"plop-dir": "^0.0.5",
Expand Down
6 changes: 3 additions & 3 deletions polaris-migrator/rollup.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ export default {
output: [
{
format: /** @type {const} */ ('cjs'),
entryFileNames: '[name][assetExtname].js',
entryFileNames: '[name].js',
dir: path.dirname(pkg.main),
preserveModules: true,
exports: 'auto',
},
{
format: /** @type {const} */ ('esm'),
entryFileNames: '[name][assetExtname].mjs',
entryFileNames: '[name].mjs',
dir: path.dirname(pkg.module),
preserveModules: true,
},
Expand All @@ -45,7 +45,7 @@ export default {
include: ['src/**/*'],
babelHelpers: 'bundled',
envName: 'production',
targets: 'node 14.13',
targets: 'node 16.16',
}),
json({compact: true}),
],
Expand Down
7 changes: 3 additions & 4 deletions polaris-react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
"bugs": {
"url": "https://github.com/Shopify/polaris/issues"
},
"engines": {
"node": "^16.16.0 || ^18.13.0"
},
"publishConfig": {
"access": "public",
"@shopify:registry": "https://registry.npmjs.org"
Expand Down Expand Up @@ -75,8 +78,6 @@
"react-dom": "^16.14.0 || ^17.0.0 || ^18.0.0"
},
"devDependencies": {
"@babel/core": "^7.15.0",
"@babel/node": "^7.14.9",
"@changesets/get-release-plan": "^3.0.13",
"@shopify/browserslist-config": "^3.0.0",
"@shopify/jest-dom-mocks": "^3.0.5",
Expand All @@ -93,8 +94,6 @@
"@types/react": "^18.0.15",
"@types/react-dom": "^18.0.6",
"ajv-cli": "^5.0.0",
"babel-core": "7.0.0-bridge.0",
"babel-loader": "^8.1.0",
"change-case": "^3.1.0",
"chromatic": "^6.5.4",
"core-js": "^3.6.5",
Expand Down
3 changes: 1 addition & 2 deletions polaris-react/playground/DetailsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -527,8 +527,7 @@ export function DetailsPage() {
alt={file.name}
source={
validImageTypes.indexOf(file.type) > 0
? // eslint-disable-next-line node/no-unsupported-features/node-builtins
URL.createObjectURL(file)
? URL.createObjectURL(file)
: 'https://cdn.shopify.com/s/files/1/0757/9955/files/New_Post.png?12678548500147524304'
}
/>
Expand Down
8 changes: 4 additions & 4 deletions polaris-react/rollup.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ function generateConfig({output, targets, stylesConfig}) {
/** @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...

targets: 'extends @shopify/browserslist-config, node 16.16',
stylesConfig: {
mode: 'standalone',
output: 'styles.css',
Expand All @@ -69,14 +69,14 @@ export default [
format: 'cjs',
dir: path.dirname(pkg.main),
preserveModules: true,
entryFileNames: '[name][assetExtname].js',
entryFileNames: '[name].js',
exports: 'named',
},
{
format: 'esm',
dir: path.dirname(pkg.module),
preserveModules: true,
entryFileNames: '[name][assetExtname].js',
entryFileNames: '[name].js',
},
],
}),
Expand All @@ -95,7 +95,7 @@ export default [
format: 'esm',
dir: path.dirname(pkg.esnext),
preserveModules: true,
entryFileNames: '[name][assetExtname].esnext',
entryFileNames: '[name].esnext',
},
],
}),
Expand Down
1 change: 0 additions & 1 deletion polaris-react/scripts/build-validate.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// eslint-disable-next-line node/no-unsupported-features/node-builtins
const assert = require('assert').strict;
const fs = require('fs');

Expand Down
Loading