Skip to content

Conversation

@michaelfaith
Copy link
Contributor

@michaelfaith michaelfaith commented Feb 16, 2025

This change removes the devEngines declaration in the root package. It didn't match the package.json spec and in npm 10.9.0 (released in October), a breaking change was introduced that checks the devEngines property. This causes npm pack calls to fail, due to the malformed devEngines. Since there's already an .nvmrc defined in the repo, and no strong need to enforce a specific node version for local development, this removes the declaration altogether.

@react-sizebot
Copy link

react-sizebot commented Feb 16, 2025

Comparing: 5adf402...5750bcb

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.68 kB 6.68 kB +0.05% 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 515.71 kB 515.71 kB = 92.09 kB 92.09 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB +0.11% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 562.25 kB 562.25 kB = 100.08 kB 100.08 kB
facebook-www/ReactDOM-prod.classic.js = 636.70 kB 636.70 kB = 112.08 kB 112.08 kB
facebook-www/ReactDOM-prod.modern.js = 627.02 kB 627.02 kB = 110.49 kB 110.50 kB
oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js = 113.75 kB 94.56 kB = 22.57 kB 15.31 kB
oss-stable-semver/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js = 113.57 kB 92.36 kB = 22.54 kB 15.05 kB
oss-stable/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js = 113.57 kB 92.36 kB = 22.54 kB 15.05 kB
oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.js = 113.66 kB 84.44 kB = 22.52 kB 14.95 kB
oss-stable-semver/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.js = 113.49 kB 82.49 kB = 22.50 kB 14.67 kB
oss-stable/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.js = 113.49 kB 82.49 kB = 22.50 kB 14.67 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js = 113.75 kB 94.56 kB = 22.57 kB 15.31 kB
oss-stable-semver/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js = 113.57 kB 92.36 kB = 22.54 kB 15.05 kB
oss-stable/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js = 113.57 kB 92.36 kB = 22.54 kB 15.05 kB
oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.js = 113.66 kB 84.44 kB = 22.52 kB 14.95 kB
oss-stable-semver/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.js = 113.49 kB 82.49 kB = 22.50 kB 14.67 kB
oss-stable/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.js = 113.49 kB 82.49 kB = 22.50 kB 14.67 kB

Generated by 🚫 dangerJS against 651a9f6

@michaelfaith michaelfaith marked this pull request as ready for review February 16, 2025 20:38
This change removes the `devEngines` declaration in the root package.  It didn't match the package.json spec and in npm 10.9.0 (released in October), a breaking change was introduced that checks the `devEngines` property.  This causes `npm pack` calls to fail, due to the malformed `devEngines`.  Since there's already an `.nvmrc` defined in the repo, and no strong need to enforce a specific node version for local development, this removes the declaration altogether.
@michaelfaith michaelfaith changed the title chore: fix devEngines declaration in root package chore: remove devEngines declaration in root package Feb 16, 2025
@poteto
Copy link
Member

poteto commented Feb 16, 2025

Thanks! For context @michaelfaith and I discussed this offline, and we agreed that there's little value in enforcing local node versions unless one is building from source locally (which is convoluted anyway due to requiring an old version of Java). Since we already enforce this in CI in order to ensure builds work it seems fine to remove.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants