From 0a7b61276215efb8ad27b5dfa1641d0a4dac8ec3 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 11 Oct 2019 17:34:16 -0700 Subject: [PATCH 1/2] Don't bother including `unstable_` in error The method names don't get stripped out of the production bundles because they are passed as arguments to the error decoder. Let's just always use the unprefixed APIs in the messages. --- .../src/__tests__/ReactDOMRoot-test.js | 12 ++++------ packages/react-dom/src/client/ReactDOM.js | 24 +++++-------------- scripts/error-codes/codes.json | 2 +- ...arning-and-invariant-args-test.internal.js | 2 +- 4 files changed, 13 insertions(+), 27 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMRoot-test.js b/packages/react-dom/src/__tests__/ReactDOMRoot-test.js index 532030d0b262a..b58ae3508b696 100644 --- a/packages/react-dom/src/__tests__/ReactDOMRoot-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMRoot-test.js @@ -103,9 +103,7 @@ describe('ReactDOMRoot', () => { it('throws a good message on invalid containers', () => { expect(() => { ReactDOM.unstable_createRoot(
Hi
); - }).toThrow( - 'unstable_createRoot(...): Target container is not a DOM element.', - ); + }).toThrow('createRoot(...): Target container is not a DOM element.'); }); it('warns when rendering with legacy API into createRoot() container', () => { @@ -119,7 +117,7 @@ describe('ReactDOMRoot', () => { [ // We care about this warning: 'You are calling ReactDOM.render() on a container that was previously ' + - 'passed to ReactDOM.unstable_createRoot(). This is not supported. ' + + 'passed to ReactDOM.createRoot(). This is not supported. ' + 'Did you mean to call root.render(element)?', // This is more of a symptom but restructuring the code to avoid it isn't worth it: 'Replacing React-rendered children with a new root component.', @@ -142,7 +140,7 @@ describe('ReactDOMRoot', () => { [ // We care about this warning: 'You are calling ReactDOM.hydrate() on a container that was previously ' + - 'passed to ReactDOM.unstable_createRoot(). This is not supported. ' + + 'passed to ReactDOM.createRoot(). This is not supported. ' + 'Did you mean to call createRoot(container, {hydrate: true}).render(element)?', // This is more of a symptom but restructuring the code to avoid it isn't worth it: 'Replacing React-rendered children with a new root component.', @@ -163,7 +161,7 @@ describe('ReactDOMRoot', () => { [ // We care about this warning: 'You are calling ReactDOM.unmountComponentAtNode() on a container that was previously ' + - 'passed to ReactDOM.unstable_createRoot(). This is not supported. Did you mean to call root.unmount()?', + 'passed to ReactDOM.createRoot(). This is not supported. Did you mean to call root.unmount()?', // This is more of a symptom but restructuring the code to avoid it isn't worth it: "The node you're attempting to unmount was rendered by React and is not a top-level container.", ], @@ -202,7 +200,7 @@ describe('ReactDOMRoot', () => { expect(() => { ReactDOM.unstable_createRoot(container); }).toWarnDev( - 'You are calling ReactDOM.unstable_createRoot() on a container that was previously ' + + 'You are calling ReactDOM.createRoot() on a container that was previously ' + 'passed to ReactDOM.render(). This is not supported.', {withoutStack: true}, ); diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index 37f67e0ce8534..b0fa8e2179daa 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -451,9 +451,8 @@ const ReactDOM: Object = { warningWithoutStack( !container._reactHasBeenPassedToCreateRootDEV, 'You are calling ReactDOM.hydrate() on a container that was previously ' + - 'passed to ReactDOM.%s(). This is not supported. ' + + 'passed to ReactDOM.createRoot(). This is not supported. ' + 'Did you mean to call createRoot(container, {hydrate: true}).render(element)?', - enableStableConcurrentModeAPIs ? 'createRoot' : 'unstable_createRoot', ); } // TODO: throw or warn if we couldn't hydrate? @@ -479,9 +478,8 @@ const ReactDOM: Object = { warningWithoutStack( !container._reactHasBeenPassedToCreateRootDEV, 'You are calling ReactDOM.render() on a container that was previously ' + - 'passed to ReactDOM.%s(). This is not supported. ' + + 'passed to ReactDOM.createRoot(). This is not supported. ' + 'Did you mean to call root.render(element)?', - enableStableConcurrentModeAPIs ? 'createRoot' : 'unstable_createRoot', ); } return legacyRenderSubtreeIntoContainer( @@ -526,8 +524,7 @@ const ReactDOM: Object = { warningWithoutStack( !container._reactHasBeenPassedToCreateRootDEV, 'You are calling ReactDOM.unmountComponentAtNode() on a container that was previously ' + - 'passed to ReactDOM.%s(). This is not supported. Did you mean to call root.unmount()?', - enableStableConcurrentModeAPIs ? 'createRoot' : 'unstable_createRoot', + 'passed to ReactDOM.createRoot(). This is not supported. Did you mean to call root.unmount()?', ); } @@ -650,13 +647,9 @@ function createRoot( container: DOMContainer, options?: RootOptions, ): _ReactRoot { - const functionName = enableStableConcurrentModeAPIs - ? 'createRoot' - : 'unstable_createRoot'; invariant( isValidContainer(container), - '%s(...): Target container is not a DOM element.', - functionName, + 'createRoot(...): Target container is not a DOM element.', ); warnIfReactDOMContainerInDEV(container); return new ReactRoot(container, options); @@ -666,13 +659,9 @@ function createSyncRoot( container: DOMContainer, options?: RootOptions, ): _ReactRoot { - const functionName = enableStableConcurrentModeAPIs - ? 'createRoot' - : 'unstable_createRoot'; invariant( isValidContainer(container), - '%s(...): Target container is not a DOM element.', - functionName, + 'createRoot(...): Target container is not a DOM element.', ); warnIfReactDOMContainerInDEV(container); return new ReactSyncRoot(container, BatchedRoot, options); @@ -682,9 +671,8 @@ function warnIfReactDOMContainerInDEV(container) { if (__DEV__) { warningWithoutStack( !container._reactRootContainer, - 'You are calling ReactDOM.%s() on a container that was previously ' + + 'You are calling ReactDOM.createRoot() on a container that was previously ' + 'passed to ReactDOM.render(). This is not supported.', - enableStableConcurrentModeAPIs ? 'createRoot' : 'unstable_createRoot', ); container._reactHasBeenPassedToCreateRootDEV = true; } diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index a770a31c16c08..f0f93e9b6949d 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -297,7 +297,7 @@ "296": "Log of yielded values is not empty. Call expect(ReactTestRenderer).unstable_toHaveYielded(...) first.", "297": "The matcher `unstable_toHaveYielded` expects an instance of React Test Renderer.\n\nTry: expect(ReactTestRenderer).unstable_toHaveYielded(expectedYields)", "298": "Hooks can only be called inside the body of a function component.", - "299": "%s(...): Target container is not a DOM element.", + "299": "createRoot(...): Target container is not a DOM element.", "300": "Rendered fewer hooks than expected. This may be caused by an accidental early return statement.", "301": "Too many re-renders. React limits the number of renders to prevent an infinite loop.", "302": "It is not supported to run the profiling version of a renderer (for example, `react-dom/profiling`) without also replacing the `scheduler/tracing` module with `scheduler/tracing-profiling`. Your bundler might have a setting for aliasing both modules. Learn more at http://fb.me/react-profiling", diff --git a/scripts/eslint-rules/__tests__/warning-and-invariant-args-test.internal.js b/scripts/eslint-rules/__tests__/warning-and-invariant-args-test.internal.js index 3bf12ae0282f9..4e408113e6a93 100644 --- a/scripts/eslint-rules/__tests__/warning-and-invariant-args-test.internal.js +++ b/scripts/eslint-rules/__tests__/warning-and-invariant-args-test.internal.js @@ -20,7 +20,7 @@ ruleTester.run('eslint-rules/warning-and-invariant-args', rule, { 'arbitraryFunction(a, b)', // These messages are in the error code map "invariant(false, 'Do not override existing functions.')", - "invariant(false, '%s(...): Target container is not a DOM element.', str)", + "invariant(false, 'createRoot(...): Target container is not a DOM element.')", ], invalid: [ { From 9977c2cc0535d2c549e38e40ae3c902556a918a8 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 11 Oct 2019 15:25:13 -0700 Subject: [PATCH 2/2] Set up experimental builds The experimental builds are packaged exactly like builds in the stable release channel: same file structure, entry points, and npm package names. The goal is to match what will eventually be released in stable as closely as possible, but with additional features turned on. Versioning and Releasing ------------------------ The experimental builds will be published to the same registry and package names as the stable ones. However, they will be versioned using a separate scheme. Instead of semver versions, experimental releases will receive arbitrary version strings based on their content hashes. The motivation is to thwart attempts to use a version range to match against future experimental releases. The only way to install or depend on an experimental release is to refer to the specific version number. Building -------- I did not use the existing feature flag infra to configure the experimental builds. The reason is because feature flags are designed to configure a single package. They're not designed to generate multiple forks of the same package; for each set of feature flags, you must create a separate package configuration. Instead, I've added a new build dimension called the **release channel**. By default, builds use the **stable** channel. There's also an **experimental** release channel. We have the option to add more in the future. There are now two dimensions per artifact: build type (production, development, or profiling), and release channel (stable or experimental). These are separate dimensions because they are combinatorial: there are stable and experimental production builds, stable and experimental developmenet builds, and so on. You can add something to an experimental build by gating on `__EXPERIMENTAL__`, similar to how we use `__DEV__`. Anything inside these branches will be excluded from the stable builds. This gives us a low effort way to add experimental behavior in any package without setting up feature flags or configuring a new package. --- .circleci/config.yml | 109 +++++++++++++++++++++++---- .eslintrc.js | 1 + packages/shared/ReactFeatureFlags.js | 2 +- scripts/flow/environment.js | 1 + scripts/jest/setupEnvironment.js | 1 + scripts/release/utils.js | 10 ++- scripts/rollup/build.js | 9 ++- 7 files changed, 113 insertions(+), 20 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 535563556ee95..bfd2cc916b90d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -22,6 +22,32 @@ aliases: - &attach_workspace at: build + - &process_artifacts + docker: *docker + environment: *environment + steps: + - checkout + - attach_workspace: *attach_workspace + - *restore_yarn_cache + - *run_yarn + - run: node ./scripts/rollup/consolidateBundleSizes.js + - run: ./scripts/circleci/upload_build.sh + - run: ./scripts/circleci/pack_and_store_artifact.sh + - store_artifacts: + path: ./node_modules.tgz + - store_artifacts: + path: ./build.tgz + - store_artifacts: + path: ./build/bundle-sizes.json + - store_artifacts: + # TODO: Update release script to use local file instead of pulling + # from artifacts. + path: ./scripts/error-codes/codes.json + - persist_to_workspace: + root: build + paths: + - bundle-sizes.json + jobs: setup: docker: *docker @@ -74,6 +100,18 @@ jobs: - *run_yarn - run: yarn test --maxWorkers=2 + test_source_experimental: + docker: *docker + environment: *environment + steps: + - checkout + - *restore_yarn_cache + - *run_yarn + - run: + environment: + RELEASE_CHANNEL: experimental + command: yarn test --maxWorkers=2 + test_source_persistent: docker: *docker environment: *environment @@ -105,16 +143,49 @@ jobs: - run: ./scripts/circleci/add_build_info_json.sh - run: ./scripts/circleci/update_package_versions.sh - run: yarn build + - run: echo "stable" >> build/RELEASE_CHANNEL + - persist_to_workspace: + root: build + paths: + - RELEASE_CHANNEL + - facebook-www + - node_modules + - react-native + - dist + - sizes/*.json + + build_experimental: + docker: *docker + environment: *environment + parallelism: 20 + steps: + - checkout + - *restore_yarn_cache + - *run_yarn + - run: + environment: + RELEASE_CHANNEL: experimental + command: | + ./scripts/circleci/add_build_info_json.sh + ./scripts/circleci/update_package_versions.sh + yarn build + - run: echo "experimental" >> build/RELEASE_CHANNEL - persist_to_workspace: root: build paths: + - RELEASE_CHANNEL - facebook-www - node_modules - react-native - dist - sizes/*.json - process_artifacts: + # These jobs are named differently so we can distinguish the stable and + # and experimental artifacts + process_artifacts: *process_artifacts + process_artifacts_experimental: *process_artifacts + + sizebot: docker: *docker environment: *environment steps: @@ -122,20 +193,10 @@ jobs: - attach_workspace: *attach_workspace - *restore_yarn_cache - *run_yarn + # This runs in the process_artifacts job, too, but it's faster to run + # this step in both jobs instead of running the jobs sequentially - run: node ./scripts/rollup/consolidateBundleSizes.js - run: node ./scripts/tasks/danger - - run: ./scripts/circleci/upload_build.sh - - run: ./scripts/circleci/pack_and_store_artifact.sh - - store_artifacts: - path: ./node_modules.tgz - - store_artifacts: - path: ./build.tgz - - store_artifacts: - path: ./build/bundle-sizes.json - - store_artifacts: - # TODO: Update release script to use local file instead of pulling - # from artifacts. - path: ./scripts/error-codes/codes.json lint_build: docker: *docker @@ -208,7 +269,7 @@ jobs: workflows: version: 2 - commit: + stable: jobs: - setup - lint: @@ -232,6 +293,9 @@ workflows: - process_artifacts: requires: - build + - sizebot: + requires: + - build - lint_build: requires: - build @@ -247,9 +311,24 @@ workflows: - test_dom_fixtures: requires: - build - hourly: + + experimental: + jobs: + - setup + - test_source_experimental: + requires: + - setup + - build_experimental: + requires: + - setup + - process_artifacts_experimental: + requires: + - build_experimental + + fuzz_tests: triggers: - schedule: + # Fuzz tests run hourly cron: "0 * * * *" filters: branches: diff --git a/.eslintrc.js b/.eslintrc.js index 603f20a3cacba..df147c90bd3e6 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -149,6 +149,7 @@ module.exports = { spyOnProd: true, __PROFILE__: true, __UMD__: true, + __EXPERIMENTAL__: true, trustedTypes: true, }, }; diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index c1645f0497674..ddb12f71b7993 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -52,7 +52,7 @@ export const disableInputAttributeSyncing = false; // These APIs will no longer be "unstable" in the upcoming 16.7 release, // Control this behavior with a flag to support 16.6 minor releases in the meanwhile. -export const enableStableConcurrentModeAPIs = false; +export const enableStableConcurrentModeAPIs = __EXPERIMENTAL__; export const warnAboutShorthandPropertyCollision = false; diff --git a/scripts/flow/environment.js b/scripts/flow/environment.js index 8eeda9784000d..0a742aad4022f 100644 --- a/scripts/flow/environment.js +++ b/scripts/flow/environment.js @@ -11,6 +11,7 @@ declare var __PROFILE__: boolean; declare var __UMD__: boolean; +declare var __EXPERIMENTAL__: boolean; declare var __REACT_DEVTOOLS_GLOBAL_HOOK__: any; /*?{ inject: ?((stuff: Object) => void) diff --git a/scripts/jest/setupEnvironment.js b/scripts/jest/setupEnvironment.js index 10ff1234b4c72..1e694b2d31575 100644 --- a/scripts/jest/setupEnvironment.js +++ b/scripts/jest/setupEnvironment.js @@ -7,6 +7,7 @@ if (NODE_ENV !== 'development' && NODE_ENV !== 'production') { global.__DEV__ = NODE_ENV === 'development'; global.__PROFILE__ = NODE_ENV === 'development'; global.__UMD__ = false; +global.__EXPERIMENTAL__ = process.env.RELEASE_CHANNEL === 'experimental'; if (typeof window !== 'undefined') { global.requestIdleCallback = function(callback) { diff --git a/scripts/release/utils.js b/scripts/release/utils.js index 861b83ad34d6c..816f0fdfc2853 100644 --- a/scripts/release/utils.js +++ b/scripts/release/utils.js @@ -78,12 +78,16 @@ const getArtifactsList = async buildID => { const getBuildInfo = async () => { const cwd = join(__dirname, '..', '..'); + const isExperimental = process.env.RELEASE_CHANNEL === 'experimental'; + const branch = await execRead('git branch | grep \\* | cut -d " " -f2', { cwd, }); const commit = await execRead('git show -s --format=%h', {cwd}); const checksum = await getChecksumForCurrentRevision(cwd); - const version = `0.0.0-${commit}`; + const version = isExperimental + ? `0.0.0-experimental-${commit}` + : `0.0.0-${commit}`; // Only available for Circle CI builds. // https://circleci.com/docs/2.0/env-vars/ @@ -94,7 +98,9 @@ const getBuildInfo = async () => { const packageJSON = await readJson( join(cwd, 'packages', 'react', 'package.json') ); - const reactVersion = `${packageJSON.version}-canary-${commit}`; + const reactVersion = isExperimental + ? `${packageJSON.version}-experimental-canary-${commit}` + : `${packageJSON.version}-canary-${commit}`; return {branch, buildNumber, checksum, commit, reactVersion, version}; }; diff --git a/scripts/rollup/build.js b/scripts/rollup/build.js index 92f1f3a754648..34184efab63e1 100644 --- a/scripts/rollup/build.js +++ b/scripts/rollup/build.js @@ -304,7 +304,8 @@ function getPlugins( bundleType, globalName, moduleType, - pureExternalModules + pureExternalModules, + isExperimentalBuild ) { const findAndRecordErrorCodes = extractErrorCodes(errorCodeOpts); const forks = Modules.getForks(bundleType, entry, moduleType); @@ -362,6 +363,7 @@ function getPlugins( __PROFILE__: isProfiling || !isProduction ? 'true' : 'false', __UMD__: isUMDBundle ? 'true' : 'false', 'process.env.NODE_ENV': isProduction ? "'production'" : "'development'", + __EXPERIMENTAL__: isExperimentalBuild, }), // We still need CommonJS for external deps like object-assign. commonjs(), @@ -485,6 +487,8 @@ async function createBundle(bundle, bundleType) { module => !importSideEffects[module] ); + const isExperimentalBuild = process.env.RELEASE_CHANNEL === 'experimental'; + const rollupConfig = { input: resolvedEntry, treeshake: { @@ -508,7 +512,8 @@ async function createBundle(bundle, bundleType) { bundleType, bundle.global, bundle.moduleType, - pureExternalModules + pureExternalModules, + isExperimentalBuild ), // We can't use getters in www. legacy: