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

Set up experimental builds #17071

Merged
merged 2 commits into from
Oct 14, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
109 changes: 94 additions & 15 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -74,6 +100,18 @@ jobs:
- *run_yarn
- run: yarn test --maxWorkers=2

test_source_experimental:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that since we're using parallel containers, this won't actually increase the duration of every CI job? That would not be great.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The workflows run in parallel, yeah

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
Expand Down Expand Up @@ -105,37 +143,60 @@ 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:
- checkout
- 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
Expand Down Expand Up @@ -208,7 +269,7 @@ jobs:

workflows:
version: 2
commit:
stable:
jobs:
- setup
- lint:
Expand All @@ -232,6 +293,9 @@ workflows:
- process_artifacts:
requires:
- build
- sizebot:
requires:
- build
- lint_build:
requires:
- build
Expand All @@ -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:
Expand Down
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ module.exports = {
spyOnProd: true,
__PROFILE__: true,
__UMD__: true,
__EXPERIMENTAL__: true,
trustedTypes: true,
},
};
12 changes: 5 additions & 7 deletions packages/react-dom/src/__tests__/ReactDOMRoot-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,7 @@ describe('ReactDOMRoot', () => {
it('throws a good message on invalid containers', () => {
expect(() => {
ReactDOM.unstable_createRoot(<div>Hi</div>);
}).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', () => {
Expand All @@ -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.',
Expand All @@ -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.',
Expand All @@ -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.",
],
Expand Down Expand Up @@ -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},
);
Expand Down
24 changes: 6 additions & 18 deletions packages/react-dom/src/client/ReactDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand All @@ -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(
Expand Down Expand Up @@ -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()?',
);
}

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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 ' +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not expect all fo these conditional names to change, given that we run this test against not-experimental builds as well. Why did they?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the first commit.

The method names don't get stripped out of the production bundles
because they are passed as arguments to the error decoder.

Slightly unrelated but I did it this way instead of checking __EXPERIMENTAL__ in the tests. We're probably going to remove the prefixed versions from the stable releases, anyway.

'passed to ReactDOM.render(). This is not supported.',
enableStableConcurrentModeAPIs ? 'createRoot' : 'unstable_createRoot',
);
container._reactHasBeenPassedToCreateRootDEV = true;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion scripts/error-codes/codes.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
{
Expand Down
1 change: 1 addition & 0 deletions scripts/flow/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions scripts/jest/setupEnvironment.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
10 changes: 8 additions & 2 deletions scripts/release/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/
Expand All @@ -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};
};
Expand Down
Loading