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

Update supported devEngines #21364

Merged
merged 4 commits into from Apr 27, 2021
Merged

Update supported devEngines #21364

merged 4 commits into from Apr 27, 2021

Conversation

@eps1lon
Copy link
Collaborator

@eps1lon eps1lon commented Apr 27, 2021

Summary

Change devEngines to indicate the required node version to perform every development workflow.

With node@12.16.0 yarn build react-server-dom-webpack/node-loader throws with

$ node ./scripts/rollup/build.js react-server-dom-webpack/node-loader
/home/eps1lon/Development/forks/react/scripts/rollup/build.js:44
  throw err;
  ^

Error: Cannot find module 'react-server-dom-webpack/node-loader'
Require stack:
- /home/eps1lon/Development/forks/react/scripts/rollup/build.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:980:15)
    at Function.resolve (internal/modules/cjs/helpers.js:83:19)
    at createBundle (/home/eps1lon/Development/forks/react/scripts/rollup/build.js:556:13)
    at buildEverything (/home/eps1lon/Development/forks/react/scripts/rollup/build.js:776:11) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ '/home/eps1lon/Development/forks/react/scripts/rollup/build.js' ]
}
error Command failed with exit code 1.

Probably because of the way exports field in packages/react-server-dom-webpack/package.json is constructed.

"exports": {
    ".": "./index.js",
    "./plugin": "./plugin.js",
    "./writer": {
      "react-server": {
        "node": "./writer.node.server.js",
        "browser": "./writer.browser.server.js"
      },
      "default": "./writer.js"
    },
    "./writer.node.server": "./writer.node.server.js",
    "./writer.browser.server": "./writer.browser.server.js",
    "./node-loader": "./esm/react-server-dom-webpack-node-loader.js",
    "./node-register": "./node-register.js",
    "./package.json": "./package.json"
  },

With node@12.17.0 the command passes.

Test Plan

  • CI green
  • yarn build react-server-dom-webpack/node-loader passes in node@12.17.0 and node@16.0.0
CI uses 12.19
@bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Apr 27, 2021

The Code Sandbox documentation says:

  // Node.js version to use for building the PR.
  // Supported versions are '10' (10.23.0, default), '12' (12.20.0) and '14' (14.15.1).
  "node": "14"

And CI is failing with an error message that says:

AssertionError [ERR_ASSERTION]: Current node version is not supported for development, expected "10.23.0" to satisfy "^12.17.0 || 13.x || 14.x || 15.x || 16.x".

Which suggests to me that we should update the .codesandbox/ci.json config file to override the default of Node 10 with either 12 or 14, e.g.:

  "node": "14"
eps1lon added 2 commits Apr 27, 2021
@sizebot
Copy link

@sizebot sizebot commented Apr 27, 2021

Comparing: 9a25916...c683596

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.min.js = 122.76 kB 122.76 kB = 39.42 kB 39.42 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 129.34 kB 129.34 kB = 41.51 kB 41.51 kB
facebook-www/ReactDOM-prod.classic.js = 406.92 kB 406.92 kB = 75.31 kB 75.31 kB
facebook-www/ReactDOM-prod.modern.js = 394.95 kB 394.95 kB = 73.41 kB 73.41 kB
facebook-www/ReactDOMForked-prod.classic.js = 406.92 kB 406.92 kB = 75.31 kB 75.32 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against c683596

@eps1lon eps1lon marked this pull request as ready for review Apr 27, 2021
@eps1lon
Copy link
Collaborator Author

@eps1lon eps1lon commented Apr 27, 2021

Which suggests to me that we should update the .codesandbox/ci.json config file to override the default of Node 10 with either 12 or 14, e.g.:

Good point. I went with the lowest possible version that's also being used in CI.

@bvaughn bvaughn merged commit 4edbcdc into facebook:master Apr 27, 2021
34 of 35 checks passed
34 of 35 checks passed
ci/circleci: RELEASE_CHANNEL_stable_yarn_lint_build CircleCI is running your tests
Details
Facebook CLA Check Contributor License Agreement is valid!
Details
ci/circleci: RELEASE_CHANNEL_stable_yarn_build Your tests passed on CircleCI!
Details
ci/circleci: RELEASE_CHANNEL_stable_yarn_test_dom_fixtures Your tests passed on CircleCI!
Details
ci/circleci: build_devtools_and_process_artifacts Your tests passed on CircleCI!
Details
ci/circleci: build_devtools_scheduling_profiler Your tests passed on CircleCI!
Details
ci/circleci: get_base_build Your tests passed on CircleCI!
Details
ci/circleci: process_artifacts_combined Your tests passed on CircleCI!
Details
ci/circleci: setup Your tests passed on CircleCI!
Details
ci/circleci: sizebot Your tests passed on CircleCI!
Details
ci/circleci: sync_reconciler_forks Your tests passed on CircleCI!
Details
ci/circleci: yarn_build Your tests passed on CircleCI!
Details
ci/circleci: yarn_build_combined Your tests passed on CircleCI!
Details
ci/circleci: yarn_flow Your tests passed on CircleCI!
Details
ci/circleci: yarn_lint Your tests passed on CircleCI!
Details
ci/circleci: yarn_lint_build Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=experimental --env=development Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=experimental --env=production Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=stable --env=development Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=stable --env=development --persistent Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=stable --env=production Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=www-classic --env=development --variant=false Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=www-classic --env=development --variant=true Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=www-classic --env=production --variant=false Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=www-classic --env=production --variant=true Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=www-modern --env=development --variant=false Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=www-modern --env=development --variant=true Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=www-modern --env=production --variant=false Your tests passed on CircleCI!
Details
ci/circleci: yarn_test--r=www-modern --env=production --variant=true Your tests passed on CircleCI!
Details
ci/circleci: yarn_test_build---project=devtools -r=experimental Your tests passed on CircleCI!
Details
ci/circleci: yarn_test_build--r=experimental --env=development Your tests passed on CircleCI!
Details
ci/circleci: yarn_test_build--r=experimental --env=production Your tests passed on CircleCI!
Details
ci/circleci: yarn_test_build--r=stable --env=development Your tests passed on CircleCI!
Details
ci/circleci: yarn_test_build--r=stable --env=production Your tests passed on CircleCI!
Details
ci/codesandbox Building packages succeeded.
Details
@bvaughn bvaughn deleted the eps1lon:chore/dev-engines branch Apr 27, 2021
@bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Apr 27, 2021

Thanks~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants