From eee896437b5901e0be814059a752c7c63dad122c Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 5 Mar 2019 17:16:39 -0800 Subject: [PATCH 1/9] Add command to run tests in persistent mode --- package.json | 1 + scripts/jest/config.source-persistent.js | 18 ++++++++++++++++++ scripts/jest/setupPersistent.js | 7 +++++++ scripts/jest/setupTests.persistent.js | 7 +++++++ 4 files changed, 33 insertions(+) create mode 100644 scripts/jest/config.source-persistent.js create mode 100644 scripts/jest/setupPersistent.js create mode 100644 scripts/jest/setupTests.persistent.js diff --git a/package.json b/package.json index bf399df5078d6..29e3aaea389dc 100644 --- a/package.json +++ b/package.json @@ -100,6 +100,7 @@ "postinstall": "node node_modules/fbjs-scripts/node/check-dev-engines.js package.json && node ./scripts/flow/createFlowConfigs.js", "debug-test": "cross-env NODE_ENV=development node --inspect-brk node_modules/.bin/jest --config ./scripts/jest/config.source.js --runInBand", "test": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.source.js", + "test-persistent": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.source-persistent.js", "test-fire": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.source-fire.js", "test-prod": "cross-env NODE_ENV=production jest --config ./scripts/jest/config.source.js", "test-fire-prod": "cross-env NODE_ENV=production jest --config ./scripts/jest/config.source-fire.js", diff --git a/scripts/jest/config.source-persistent.js b/scripts/jest/config.source-persistent.js new file mode 100644 index 0000000000000..edda459695f50 --- /dev/null +++ b/scripts/jest/config.source-persistent.js @@ -0,0 +1,18 @@ +'use strict'; + +const baseConfig = require('./config.base'); + +module.exports = Object.assign({}, baseConfig, { + modulePathIgnorePatterns: [ + 'ReactIncrementalPerf', + 'ReactIncrementalUpdatesMinimalism', + 'ReactIncrementalTriangle', + 'ReactIncrementalReflection', + 'forwardRef', + ], + setupFiles: [ + ...baseConfig.setupFiles, + require.resolve('./setupPersistent.js'), + require.resolve('./setupHostConfigs.js'), + ], +}); diff --git a/scripts/jest/setupPersistent.js b/scripts/jest/setupPersistent.js new file mode 100644 index 0000000000000..e07e74e456e02 --- /dev/null +++ b/scripts/jest/setupPersistent.js @@ -0,0 +1,7 @@ +'use strict'; + +jest.mock('react-noop-renderer', () => + require.requireActual('react-noop-renderer/persistent') +); + +global.__PERSISTENT__ = true; diff --git a/scripts/jest/setupTests.persistent.js b/scripts/jest/setupTests.persistent.js new file mode 100644 index 0000000000000..e07e74e456e02 --- /dev/null +++ b/scripts/jest/setupTests.persistent.js @@ -0,0 +1,7 @@ +'use strict'; + +jest.mock('react-noop-renderer', () => + require.requireActual('react-noop-renderer/persistent') +); + +global.__PERSISTENT__ = true; From db3deb10b34b3a0b3ec463dce532012bd57fd016 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 5 Mar 2019 17:35:22 -0800 Subject: [PATCH 2/9] Convert Suspense fuzz tester to use noop renderer So we can run it in persistent mode, too. --- .../ReactSuspenseFuzz-test.internal.js | 53 +++++++++---------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js index 079f70f3dbf5f..22c2c051af38d 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js @@ -1,6 +1,6 @@ let React; let Suspense; -let ReactTestRenderer; +let ReactNoop; let Scheduler; let ReactFeatureFlags; let Random; @@ -26,7 +26,7 @@ describe('ReactSuspenseFuzz', () => { ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; React = require('react'); Suspense = React.Suspense; - ReactTestRenderer = require('react-test-renderer'); + ReactNoop = require('react-noop-renderer'); Scheduler = require('scheduler'); Random = require('random-seed'); }); @@ -143,28 +143,16 @@ describe('ReactSuspenseFuzz', () => { return resolvedText; } - function renderToRoot( - root, - children, - {shouldSuspend} = {shouldSuspend: true}, - ) { - root.update( - - {children} - , - ); + function resolveAllTasks() { Scheduler.unstable_flushWithoutYielding(); - let elapsedTime = 0; while (pendingTasks && pendingTasks.size > 0) { if ((elapsedTime += 1000) > 1000000) { throw new Error('Something did not resolve properly.'); } - ReactTestRenderer.act(() => jest.advanceTimersByTime(1000)); + ReactNoop.act(() => jest.advanceTimersByTime(1000)); Scheduler.unstable_flushWithoutYielding(); } - - return root.toJSON(); } function testResolvedOutput(unwrappedChildren) { @@ -172,25 +160,32 @@ describe('ReactSuspenseFuzz', () => { {unwrappedChildren} ); - const expectedRoot = ReactTestRenderer.create(null); - const expectedOutput = renderToRoot(expectedRoot, children, { - shouldSuspend: false, - }); - expectedRoot.unmount(); + resetCache(); + ReactNoop.renderToRootWithID( + + {children} + , + 'expected', + ); + resolveAllTasks(); + const expectedOutput = ReactNoop.getChildrenAsJSX('expected'); + ReactNoop.renderToRootWithID(null, 'expected'); + Scheduler.unstable_flushWithoutYielding(); resetCache(); - const syncRoot = ReactTestRenderer.create(null); - const syncOutput = renderToRoot(syncRoot, children); + ReactNoop.renderLegacySyncRoot(children); + resolveAllTasks(); + const syncOutput = ReactNoop.getChildrenAsJSX(); expect(syncOutput).toEqual(expectedOutput); - syncRoot.unmount(); + ReactNoop.renderLegacySyncRoot(null); resetCache(); - const concurrentRoot = ReactTestRenderer.create(null, { - unstable_isConcurrent: true, - }); - const concurrentOutput = renderToRoot(concurrentRoot, children); + ReactNoop.renderToRootWithID(children, 'concurrent'); + Scheduler.unstable_flushWithoutYielding(); + resolveAllTasks(); + const concurrentOutput = ReactNoop.getChildrenAsJSX('concurrent'); expect(concurrentOutput).toEqual(expectedOutput); - concurrentRoot.unmount(); + ReactNoop.renderToRootWithID(null, 'concurrent'); Scheduler.unstable_flushWithoutYielding(); } From 1c6bf708c7df7e82ee109f6deb29413a397b35f9 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 5 Mar 2019 20:18:51 -0800 Subject: [PATCH 3/9] Don't mutate stateNode in appendAllChildren We can't mutate the stateNode in appendAllChildren because the children could be current. This is a bit weird because now the child that we append is different from the one on the fiber stateNode. I think this makes conceptual sense, but I suspect this likely breaks an assumption in Fabric. With this approach, we no longer need to clone to unhide the children, so I removed those host config methods. Fixes bug surfaced by fuzz tester. (The test case that failed was the one that's already hard coded.) --- .../src/ReactFabricHostConfig.js | 27 ---- .../src/createReactNoop.js | 43 ------ .../src/ReactFiberCompleteWork.js | 128 ++++++++---------- .../src/forks/ReactFiberHostConfig.custom.js | 3 - .../shared/HostConfigWithNoPersistence.js | 2 - 5 files changed, 59 insertions(+), 144 deletions(-) diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index 0eed32d066246..c1268eb663edb 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -387,25 +387,6 @@ export function cloneHiddenInstance( }; } -export function cloneUnhiddenInstance( - instance: Instance, - type: string, - props: Props, - internalInstanceHandle: Object, -): Instance { - const viewConfig = instance.canonical.viewConfig; - const node = instance.node; - const updatePayload = diff( - {...props, style: [props.style, {display: 'none'}]}, - props, - viewConfig.validAttributes, - ); - return { - node: cloneNodeWithNewProps(node, updatePayload), - canonical: instance.canonical, - }; -} - export function cloneHiddenTextInstance( instance: Instance, text: string, @@ -414,14 +395,6 @@ export function cloneHiddenTextInstance( throw new Error('Not yet implemented.'); } -export function cloneUnhiddenTextInstance( - instance: Instance, - text: string, - internalInstanceHandle: Object, -): TextInstance { - throw new Error('Not yet implemented.'); -} - export function createContainerChildSet(container: Container): ChildSet { return createChildNodeSet(container); } diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 62f993247cc96..f3872ac483b57 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -486,26 +486,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { return clone; }, - cloneUnhiddenInstance( - instance: Instance, - type: string, - props: Props, - internalInstanceHandle: Object, - ): Instance { - const clone = cloneInstance( - instance, - null, - type, - props, - props, - internalInstanceHandle, - true, - null, - ); - clone.hidden = props.hidden === true; - return clone; - }, - cloneHiddenTextInstance( instance: TextInstance, text: string, @@ -528,29 +508,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { }); return clone; }, - - cloneUnhiddenTextInstance( - instance: TextInstance, - text: string, - internalInstanceHandle: Object, - ): TextInstance { - const clone = { - text: instance.text, - id: instanceCounter++, - hidden: false, - context: instance.context, - }; - // Hide from unit tests - Object.defineProperty(clone, 'id', { - value: clone.id, - enumerable: false, - }); - Object.defineProperty(clone, 'context', { - value: clone.context, - enumerable: false, - }); - return clone; - }, }; const NoopRenderer = reconciler(hostConfig); diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index b1f374bb90bc6..03ef368fc3a88 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -59,9 +59,7 @@ import { supportsPersistence, cloneInstance, cloneHiddenInstance, - cloneUnhiddenInstance, cloneHiddenTextInstance, - cloneUnhiddenTextInstance, createContainerChildSet, appendChildToContainerChildSet, finalizeContainerChildren, @@ -209,31 +207,19 @@ if (supportsMutation) { // eslint-disable-next-line no-labels branches: if (node.tag === HostComponent) { let instance = node.stateNode; - if (needsVisibilityToggle) { + if (needsVisibilityToggle && isHidden) { + // This child is inside a timed out tree. Hide it. const props = node.memoizedProps; const type = node.type; - if (isHidden) { - // This child is inside a timed out tree. Hide it. - instance = cloneHiddenInstance(instance, type, props, node); - } else { - // This child was previously inside a timed out tree. If it was not - // updated during this render, it may need to be unhidden. Clone - // again to be sure. - instance = cloneUnhiddenInstance(instance, type, props, node); - } - node.stateNode = instance; + instance = cloneHiddenInstance(instance, type, props, node); } appendInitialChild(parent, instance); } else if (node.tag === HostText) { let instance = node.stateNode; - if (needsVisibilityToggle) { + if (needsVisibilityToggle && isHidden) { + // This child is inside a timed out tree. Hide it. const text = node.memoizedProps; - if (isHidden) { - instance = cloneHiddenTextInstance(instance, text, node); - } else { - instance = cloneUnhiddenTextInstance(instance, text, node); - } - node.stateNode = instance; + instance = cloneHiddenTextInstance(instance, text, node); } appendInitialChild(parent, instance); } else if (node.tag === HostPortal) { @@ -247,15 +233,22 @@ if (supportsMutation) { if (newIsHidden) { const primaryChildParent = node.child; if (primaryChildParent !== null) { - appendAllChildren(parent, primaryChildParent, true, newIsHidden); - node = primaryChildParent.sibling; - continue; + if (primaryChildParent.child !== null) { + primaryChildParent.child.return = primaryChildParent; + appendAllChildren( + parent, + primaryChildParent, + true, + newIsHidden, + ); + } + const fallbackChildParent = primaryChildParent.sibling; + if (fallbackChildParent !== null) { + fallbackChildParent.return = node; + node = fallbackChildParent; + continue; + } } - } else { - const primaryChildParent = node; - appendAllChildren(parent, primaryChildParent, true, newIsHidden); - // eslint-disable-next-line no-labels - break branches; } } if (node.child !== null) { @@ -299,31 +292,19 @@ if (supportsMutation) { // eslint-disable-next-line no-labels branches: if (node.tag === HostComponent) { let instance = node.stateNode; - if (needsVisibilityToggle) { + if (needsVisibilityToggle && isHidden) { + // This child is inside a timed out tree. Hide it. const props = node.memoizedProps; const type = node.type; - if (isHidden) { - // This child is inside a timed out tree. Hide it. - instance = cloneHiddenInstance(instance, type, props, node); - } else { - // This child was previously inside a timed out tree. If it was not - // updated during this render, it may need to be unhidden. Clone - // again to be sure. - instance = cloneUnhiddenInstance(instance, type, props, node); - } - node.stateNode = instance; + instance = cloneHiddenInstance(instance, type, props, node); } appendChildToContainerChildSet(containerChildSet, instance); } else if (node.tag === HostText) { let instance = node.stateNode; - if (needsVisibilityToggle) { + if (needsVisibilityToggle && isHidden) { + // This child is inside a timed out tree. Hide it. const text = node.memoizedProps; - if (isHidden) { - instance = cloneHiddenTextInstance(instance, text, node); - } else { - instance = cloneUnhiddenTextInstance(instance, text, node); - } - node.stateNode = instance; + instance = cloneHiddenTextInstance(instance, text, node); } appendChildToContainerChildSet(containerChildSet, instance); } else if (node.tag === HostPortal) { @@ -337,25 +318,22 @@ if (supportsMutation) { if (newIsHidden) { const primaryChildParent = node.child; if (primaryChildParent !== null) { - appendAllChildrenToContainer( - containerChildSet, - primaryChildParent, - true, - newIsHidden, - ); - node = primaryChildParent.sibling; - continue; + if (primaryChildParent.child !== null) { + primaryChildParent.child.return = primaryChildParent; + appendAllChildrenToContainer( + containerChildSet, + primaryChildParent, + true, + newIsHidden, + ); + } + const fallbackChildParent = primaryChildParent.sibling; + if (fallbackChildParent !== null) { + fallbackChildParent.return = node; + node = fallbackChildParent; + continue; + } } - } else { - const primaryChildParent = node; - appendAllChildrenToContainer( - containerChildSet, - primaryChildParent, - true, - newIsHidden, - ); - // eslint-disable-next-line no-labels - break branches; } } if (node.child !== null) { @@ -714,11 +692,23 @@ function completeWork( } } - if (nextDidTimeout || prevDidTimeout) { - // If the children are hidden, or if they were previous hidden, schedule - // an effect to toggle their visibility. This is also used to attach a - // retry listener to the promise. - workInProgress.effectTag |= Update; + if (supportsPersistence) { + if (nextDidTimeout) { + // If this boundary just timed out, schedule an effect to attach a + // retry listener to the proimse. This flag is also used to hide the + // primary children. + workInProgress.effectTag |= Update; + } + } + if (supportsMutation) { + if (nextDidTimeout || prevDidTimeout) { + // If this boundary just timed out, schedule an effect to attach a + // retry listener to the proimse. This flag is also used to hide the + // primary children. In mutation mode, we also need the flag to + // *unhide* children that were previously hidden, so check if the + // is currently timed out, too. + workInProgress.effectTag |= Update; + } } break; } diff --git a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js index 4a68193c8534d..c1f03e1085b1b 100644 --- a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js @@ -91,10 +91,7 @@ export const finalizeContainerChildren = $$$hostConfig.finalizeContainerChildren; export const replaceContainerChildren = $$$hostConfig.replaceContainerChildren; export const cloneHiddenInstance = $$$hostConfig.cloneHiddenInstance; -export const cloneUnhiddenInstance = $$$hostConfig.cloneUnhiddenInstance; export const cloneHiddenTextInstance = $$$hostConfig.cloneHiddenTextInstance; -export const cloneUnhiddenTextInstance = - $$$hostConfig.cloneUnhiddenTextInstance; // ------------------- // Hydration diff --git a/packages/shared/HostConfigWithNoPersistence.js b/packages/shared/HostConfigWithNoPersistence.js index ffd8342708e34..d5f84cf43fd6d 100644 --- a/packages/shared/HostConfigWithNoPersistence.js +++ b/packages/shared/HostConfigWithNoPersistence.js @@ -29,6 +29,4 @@ export const appendChildToContainerChildSet = shim; export const finalizeContainerChildren = shim; export const replaceContainerChildren = shim; export const cloneHiddenInstance = shim; -export const cloneUnhiddenInstance = shim; export const cloneHiddenTextInstance = shim; -export const cloneUnhiddenTextInstance = shim; From d439f74b56e8a23e5ae95345e82cd430a3049d81 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 5 Mar 2019 20:24:14 -0800 Subject: [PATCH 4/9] In persistent mode, disable test that reads a ref Refs behave differently in persistent mode. I added a TODO to write a persistent mode version of this test. --- ...tSuspenseWithNoopRenderer-test.internal.js | 63 ++++++++++--------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js index 6e8727d0c64ef..abd5fe012e213 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js @@ -1387,44 +1387,47 @@ describe('ReactSuspenseWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span('Hi')]); }); - it('toggles visibility during the mutation phase', async () => { - const {useRef, useLayoutEffect} = React; + if (!global.__PERSISTENT__) { + // TODO: Write persistent version of this test + it('toggles visibility during the mutation phase', async () => { + const {useRef, useLayoutEffect} = React; - function Parent() { - const child = useRef(null); + function Parent() { + const child = useRef(null); - useLayoutEffect(() => { - Scheduler.yieldValue('Child is hidden: ' + child.current.hidden); - }); + useLayoutEffect(() => { + Scheduler.yieldValue('Child is hidden: ' + child.current.hidden); + }); - return ( - - ); - } + return ( + + ); + } - function App(props) { - return ( - }> - - - ); - } + function App(props) { + return ( + }> + + + ); + } - ReactNoop.renderLegacySyncRoot(); + ReactNoop.renderLegacySyncRoot(); - expect(Scheduler).toHaveYielded([ - 'Suspend! [Hi]', - 'Loading...', - // The child should have already been hidden - 'Child is hidden: true', - ]); + expect(Scheduler).toHaveYielded([ + 'Suspend! [Hi]', + 'Loading...', + // The child should have already been hidden + 'Child is hidden: true', + ]); - await advanceTimers(1000); + await advanceTimers(1000); - expect(Scheduler).toHaveYielded(['Promise resolved [Hi]', 'Hi']); - }); + expect(Scheduler).toHaveYielded(['Promise resolved [Hi]', 'Hi']); + }); + } }); it('does not call lifecycles of a suspended component', async () => { From c49c24e5f3133af099bd74c2648b90e2e9dcfbd9 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 5 Mar 2019 20:25:01 -0800 Subject: [PATCH 5/9] Run persistent mode tests in CI --- scripts/circleci/test_entry_point.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/circleci/test_entry_point.sh b/scripts/circleci/test_entry_point.sh index 5b46ec45d3b6e..b190ce672d2cf 100755 --- a/scripts/circleci/test_entry_point.sh +++ b/scripts/circleci/test_entry_point.sh @@ -11,6 +11,7 @@ if [ $((0 % CIRCLE_NODE_TOTAL)) -eq "$CIRCLE_NODE_INDEX" ]; then COMMANDS_TO_RUN+=('node ./scripts/tasks/flow-ci') COMMANDS_TO_RUN+=('node ./scripts/tasks/eslint') COMMANDS_TO_RUN+=('yarn test --maxWorkers=2') + COMMANDS_TO_RUN+=('yarn test-persistent --maxWorkers=2') COMMANDS_TO_RUN+=('./scripts/circleci/check_license.sh') COMMANDS_TO_RUN+=('./scripts/circleci/check_modules.sh') COMMANDS_TO_RUN+=('./scripts/circleci/test_print_warnings.sh') From e214a2f2567868414221c5baf591de8977c34981 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 6 Mar 2019 13:37:13 -0800 Subject: [PATCH 6/9] test-persistent should skip files without noop If a file doesn't reference react-noop-renderer, we shouldn't bother running it in persistent mode, since the results will be identical to the normal test run. --- package.json | 5 ++-- scripts/jest/config.source-persistent.js | 19 ++++++++++++++ yarn.lock | 33 ++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 29e3aaea389dc..893c0fb6b6870 100644 --- a/package.json +++ b/package.json @@ -4,6 +4,7 @@ "packages/*" ], "devDependencies": { + "@mattiasbuelens/web-streams-polyfill": "0.1.0", "art": "^0.10.1", "babel-cli": "^6.6.5", "babel-code-frame": "^6.26.0", @@ -80,11 +81,11 @@ "rollup-plugin-replace": "^2.0.0", "rollup-plugin-strip-banner": "^0.2.0", "semver": "^5.5.0", + "shelljs": "^0.8.3", "targz": "^1.0.1", "through2": "^2.0.0", "tmp": "~0.0.28", - "typescript": "~1.8.10", - "@mattiasbuelens/web-streams-polyfill": "0.1.0" + "typescript": "~1.8.10" }, "devEngines": { "node": "8.x || 9.x || 10.x || 11.x" diff --git a/scripts/jest/config.source-persistent.js b/scripts/jest/config.source-persistent.js index edda459695f50..dd8cf190df2b0 100644 --- a/scripts/jest/config.source-persistent.js +++ b/scripts/jest/config.source-persistent.js @@ -1,9 +1,28 @@ 'use strict'; +const path = require('path'); +const {find, grep} = require('shelljs'); const baseConfig = require('./config.base'); +// Search for all the test files that reference react-noop-renderer +const packagesDir = path.join(process.cwd(), 'packages'); +const testRegex = new RegExp(baseConfig.testRegex); +const testFiles = find('packages').filter(filename => testRegex.test(filename)); +const testMatchFilesThatReferenceNoop = grep( + '-l', + 'react-noop-renderer', + testFiles +) + .split('\n') + .map(filename => `**/${path.relative(packagesDir, filename)}`); + module.exports = Object.assign({}, baseConfig, { + // Override base regex so that we only match the files we found above. + testRegex: null, + testMatch: testMatchFilesThatReferenceNoop, modulePathIgnorePatterns: [ + ...baseConfig.modulePathIgnorePatterns, + // These files only work in mutation mode 'ReactIncrementalPerf', 'ReactIncrementalUpdatesMinimalism', 'ReactIncrementalTriangle', diff --git a/yarn.lock b/yarn.lock index 63f2929f3668a..a688ebca3128d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2228,6 +2228,18 @@ glob@^6.0.4: once "^1.3.0" path-is-absolute "^1.0.0" +glob@^7.0.0: + version "7.1.3" + resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.3.tgz#3960832d3f1574108342dafd3a67b332c0969df1" + integrity sha512-vcfuiIxogLV4DlGBHIUOwI0IbrJ8HWPc4MU7HzviGeNho/UJDfi6B5p3sHeWIQ0KGIU0Jpxi5ZHxemQfLkkAwQ== + dependencies: + fs.realpath "^1.0.0" + inflight "^1.0.4" + inherits "2" + minimatch "^3.0.4" + once "^1.3.0" + path-is-absolute "^1.0.0" + glob@^7.0.3, glob@^7.0.5, glob@^7.1.1, glob@^7.1.2: version "7.1.2" resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.2.tgz#c19c9df9a028702d678612384a6552404c636d15" @@ -2526,6 +2538,11 @@ inquirer@^3.0.6: strip-ansi "^4.0.0" through "^2.3.6" +interpret@^1.0.0: + version "1.2.0" + resolved "https://registry.yarnpkg.com/interpret/-/interpret-1.2.0.tgz#d5061a6224be58e8083985f5014d844359576296" + integrity sha512-mT34yGKMNceBQUoVn7iCDKDntA7SC6gycMAWzGx1z/CMCTV7b2AAtXlo3nRyHZ1FelRkQbQjprHSYGwzLtkVbw== + invariant@^2.2.2: version "2.2.4" resolved "https://registry.yarnpkg.com/invariant/-/invariant-2.2.4.tgz#610f3c92c9359ce1db616e538008d23ff35158e6" @@ -4179,6 +4196,13 @@ realpath-native@^1.0.0: dependencies: util.promisify "^1.0.0" +rechoir@^0.6.2: + version "0.6.2" + resolved "https://registry.yarnpkg.com/rechoir/-/rechoir-0.6.2.tgz#85204b54dba82d5742e28c96756ef43af50e3384" + integrity sha1-hSBLVNuoLVdC4oyWdW70OvUOM4Q= + dependencies: + resolve "^1.1.6" + regenerator-runtime@^0.10.5: version "0.10.5" resolved "https://registry.yarnpkg.com/regenerator-runtime/-/regenerator-runtime-0.10.5.tgz#336c3efc1220adcedda2c9fab67b5a7955a33658" @@ -4549,6 +4573,15 @@ shebang-regex@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/shebang-regex/-/shebang-regex-1.0.0.tgz#da42f49740c0b42db2ca9728571cb190c98efea3" +shelljs@^0.8.3: + version "0.8.3" + resolved "https://registry.yarnpkg.com/shelljs/-/shelljs-0.8.3.tgz#a7f3319520ebf09ee81275b2368adb286659b097" + integrity sha512-fc0BKlAWiLpwZljmOvAOTE/gXawtCoNrP5oaY7KIaQbbyHeQVg01pSEuEGvGh3HEdBU4baCD7wQBwADmM/7f7A== + dependencies: + glob "^7.0.0" + interpret "^1.0.0" + rechoir "^0.6.2" + shellwords@^0.1.1: version "0.1.1" resolved "https://registry.yarnpkg.com/shellwords/-/shellwords-0.1.1.tgz#d6b9181c1a48d397324c84871efbcfc73fc0654b" From 2df82f57424cdb6a8b53e0708435c86c75a3e96d Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 6 Mar 2019 13:42:17 -0800 Subject: [PATCH 7/9] Remove module constructor from placeholder tests We don't need this now that we have the ability to run any test file in either mutation or persistent mode. --- .../ReactSuspensePlaceholder-test.internal.js | 964 +++++++++--------- 1 file changed, 468 insertions(+), 496 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js index 637779adbd7d8..fdd2b717aae37 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js @@ -8,532 +8,504 @@ * @jest-environment node */ -runPlaceholderTests('ReactSuspensePlaceholder (mutation)', () => - require('react-noop-renderer'), -); -runPlaceholderTests('ReactSuspensePlaceholder (persistence)', () => - require('react-noop-renderer/persistent'), -); - -function runPlaceholderTests(suiteLabel, loadReactNoop) { - let Profiler; - let React; - let ReactNoop; - let Scheduler; - let ReactFeatureFlags; - let ReactCache; - let Suspense; - let TextResource; - let textResourceShouldFail; - - describe(suiteLabel, () => { - beforeEach(() => { - jest.resetModules(); - - ReactFeatureFlags = require('shared/ReactFeatureFlags'); - ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; - ReactFeatureFlags.enableProfilerTimer = true; - ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; - React = require('react'); - ReactNoop = loadReactNoop(); - Scheduler = require('scheduler'); - ReactCache = require('react-cache'); - - Profiler = React.unstable_Profiler; - Suspense = React.Suspense; - - TextResource = ReactCache.unstable_createResource(([text, ms = 0]) => { - let listeners = null; - let status = 'pending'; - let value = null; - return { - then(resolve, reject) { - switch (status) { - case 'pending': { - if (listeners === null) { - listeners = [{resolve, reject}]; - setTimeout(() => { - if (textResourceShouldFail) { - Scheduler.yieldValue(`Promise rejected [${text}]`); - status = 'rejected'; - value = new Error('Failed to load: ' + text); - listeners.forEach(listener => listener.reject(value)); - } else { - Scheduler.yieldValue(`Promise resolved [${text}]`); - status = 'resolved'; - value = text; - listeners.forEach(listener => listener.resolve(value)); - } - }, ms); - } else { - listeners.push({resolve, reject}); - } - break; - } - case 'resolved': { - resolve(value); - break; - } - case 'rejected': { - reject(value); - break; +let Profiler; +let React; +let ReactNoop; +let Scheduler; +let ReactFeatureFlags; +let ReactCache; +let Suspense; +let TextResource; +let textResourceShouldFail; + +describe('ReactSuspensePlaceholder', () => { + beforeEach(() => { + jest.resetModules(); + + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; + ReactFeatureFlags.enableProfilerTimer = true; + ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; + React = require('react'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + ReactCache = require('react-cache'); + + Profiler = React.unstable_Profiler; + Suspense = React.Suspense; + + TextResource = ReactCache.unstable_createResource(([text, ms = 0]) => { + let listeners = null; + let status = 'pending'; + let value = null; + return { + then(resolve, reject) { + switch (status) { + case 'pending': { + if (listeners === null) { + listeners = [{resolve, reject}]; + setTimeout(() => { + if (textResourceShouldFail) { + Scheduler.yieldValue(`Promise rejected [${text}]`); + status = 'rejected'; + value = new Error('Failed to load: ' + text); + listeners.forEach(listener => listener.reject(value)); + } else { + Scheduler.yieldValue(`Promise resolved [${text}]`); + status = 'resolved'; + value = text; + listeners.forEach(listener => listener.resolve(value)); + } + }, ms); + } else { + listeners.push({resolve, reject}); } + break; } - }, - }; - }, ([text, ms]) => text); - textResourceShouldFail = false; - }); + case 'resolved': { + resolve(value); + break; + } + case 'rejected': { + reject(value); + break; + } + } + }, + }; + }, ([text, ms]) => text); + textResourceShouldFail = false; + }); - function Text({fakeRenderDuration = 0, text = 'Text'}) { - Scheduler.advanceTime(fakeRenderDuration); + function Text({fakeRenderDuration = 0, text = 'Text'}) { + Scheduler.advanceTime(fakeRenderDuration); + Scheduler.yieldValue(text); + return text; + } + + function AsyncText({fakeRenderDuration = 0, ms, text}) { + Scheduler.advanceTime(fakeRenderDuration); + try { + TextResource.read([text, ms]); Scheduler.yieldValue(text); return text; + } catch (promise) { + if (typeof promise.then === 'function') { + Scheduler.yieldValue(`Suspend! [${text}]`); + } else { + Scheduler.yieldValue(`Error! [${text}]`); + } + throw promise; } + } - function AsyncText({fakeRenderDuration = 0, ms, text}) { - Scheduler.advanceTime(fakeRenderDuration); - try { - TextResource.read([text, ms]); + it('times out children that are already hidden', () => { + class HiddenText extends React.PureComponent { + render() { + const text = this.props.text; Scheduler.yieldValue(text); - return text; - } catch (promise) { - if (typeof promise.then === 'function') { - Scheduler.yieldValue(`Suspend! [${text}]`); - } else { - Scheduler.yieldValue(`Error! [${text}]`); - } - throw promise; + return ; } } - it('times out children that are already hidden', () => { - class HiddenText extends React.PureComponent { - render() { - const text = this.props.text; - Scheduler.yieldValue(text); - return ; - } - } - - function App(props) { - return ( - }> - - - - - - - - - ); - } - - // Initial mount - ReactNoop.render(); - - expect(Scheduler).toFlushAndYield([ - 'A', - 'Suspend! [B]', - 'C', - 'Loading...', - ]); - expect(ReactNoop).toMatchRenderedOutput(null); - - jest.advanceTimersByTime(1000); - expect(Scheduler).toHaveYielded(['Promise resolved [B]']); - - expect(Scheduler).toFlushAndYield(['A', 'B', 'C']); - - expect(ReactNoop).toMatchRenderedOutput( - - - B - C - , + function App(props) { + return ( + }> + + + + + + + + ); + } - // Update - ReactNoop.render(); - expect(Scheduler).toFlushAndYield(['Suspend! [B2]', 'C', 'Loading...']); - - // Time out the update - jest.advanceTimersByTime(750); - expect(Scheduler).toFlushAndYield([]); - expect(ReactNoop).toMatchRenderedOutput( - - - - - Loading... - , - ); + // Initial mount + ReactNoop.render(); + + expect(Scheduler).toFlushAndYield(['A', 'Suspend! [B]', 'C', 'Loading...']); + expect(ReactNoop).toMatchRenderedOutput(null); + + jest.advanceTimersByTime(1000); + expect(Scheduler).toHaveYielded(['Promise resolved [B]']); + + expect(Scheduler).toFlushAndYield(['A', 'B', 'C']); + + expect(ReactNoop).toMatchRenderedOutput( + + + B + C + , + ); + + // Update + ReactNoop.render(); + expect(Scheduler).toFlushAndYield(['Suspend! [B2]', 'C', 'Loading...']); + + // Time out the update + jest.advanceTimersByTime(750); + expect(Scheduler).toFlushAndYield([]); + expect(ReactNoop).toMatchRenderedOutput( + + + + + Loading... + , + ); + + // Resolve the promise + jest.advanceTimersByTime(1000); + expect(Scheduler).toHaveYielded(['Promise resolved [B2]']); + expect(Scheduler).toFlushAndYield(['B2', 'C']); + + // Render the final update. A should still be hidden, because it was + // given a `hidden` prop. + expect(ReactNoop).toMatchRenderedOutput( + + + B2 + C + , + ); + }); - // Resolve the promise - jest.advanceTimersByTime(1000); - expect(Scheduler).toHaveYielded(['Promise resolved [B2]']); - expect(Scheduler).toFlushAndYield(['B2', 'C']); - - // Render the final update. A should still be hidden, because it was - // given a `hidden` prop. - expect(ReactNoop).toMatchRenderedOutput( - - - B2 - C - , + it('times out text nodes', async () => { + function App(props) { + return ( + }> + + + + ); - }); + } - it('times out text nodes', async () => { - function App(props) { - return ( + // Initial mount + ReactNoop.render(); + + expect(Scheduler).toFlushAndYield(['A', 'Suspend! [B]', 'C', 'Loading...']); + + expect(ReactNoop).toMatchRenderedOutput(null); + + jest.advanceTimersByTime(1000); + expect(Scheduler).toHaveYielded(['Promise resolved [B]']); + expect(Scheduler).toFlushAndYield(['A', 'B', 'C']); + expect(ReactNoop).toMatchRenderedOutput('ABC'); + + // Update + ReactNoop.render(); + expect(Scheduler).toFlushAndYield([ + 'A', + 'Suspend! [B2]', + 'C', + 'Loading...', + ]); + // Time out the update + jest.advanceTimersByTime(750); + expect(Scheduler).toFlushAndYield([]); + expect(ReactNoop).toMatchRenderedOutput('Loading...'); + + // Resolve the promise + jest.advanceTimersByTime(1000); + expect(Scheduler).toHaveYielded(['Promise resolved [B2]']); + expect(Scheduler).toFlushAndYield(['A', 'B2', 'C']); + + // Render the final update. A should still be hidden, because it was + // given a `hidden` prop. + expect(ReactNoop).toMatchRenderedOutput('AB2C'); + }); + + it('preserves host context for text nodes', () => { + function App(props) { + return ( + // uppercase is a special type that causes React Noop to render child + // text nodes as uppercase. + }> - + - + - ); - } + + ); + } - // Initial mount - ReactNoop.render(); - - expect(Scheduler).toFlushAndYield([ - 'A', - 'Suspend! [B]', - 'C', - 'Loading...', - ]); - - expect(ReactNoop).toMatchRenderedOutput(null); - - jest.advanceTimersByTime(1000); - expect(Scheduler).toHaveYielded(['Promise resolved [B]']); - expect(Scheduler).toFlushAndYield(['A', 'B', 'C']); - expect(ReactNoop).toMatchRenderedOutput('ABC'); - - // Update - ReactNoop.render(); - expect(Scheduler).toFlushAndYield([ - 'A', - 'Suspend! [B2]', - 'C', - 'Loading...', - ]); - // Time out the update - jest.advanceTimersByTime(750); - expect(Scheduler).toFlushAndYield([]); - expect(ReactNoop).toMatchRenderedOutput('Loading...'); - - // Resolve the promise - jest.advanceTimersByTime(1000); - expect(Scheduler).toHaveYielded(['Promise resolved [B2]']); - expect(Scheduler).toFlushAndYield(['A', 'B2', 'C']); - - // Render the final update. A should still be hidden, because it was - // given a `hidden` prop. - expect(ReactNoop).toMatchRenderedOutput('AB2C'); - }); + // Initial mount + ReactNoop.render(); + + expect(Scheduler).toFlushAndYield(['a', 'Suspend! [b]', 'c', 'Loading...']); + + expect(ReactNoop).toMatchRenderedOutput(null); + + jest.advanceTimersByTime(1000); + expect(Scheduler).toHaveYielded(['Promise resolved [b]']); + expect(Scheduler).toFlushAndYield(['a', 'b', 'c']); + expect(ReactNoop).toMatchRenderedOutput(ABC); + + // Update + ReactNoop.render(); + expect(Scheduler).toFlushAndYield([ + 'a', + 'Suspend! [b2]', + 'c', + 'Loading...', + ]); + // Time out the update + jest.advanceTimersByTime(750); + expect(Scheduler).toFlushAndYield([]); + expect(ReactNoop).toMatchRenderedOutput(LOADING...); + + // Resolve the promise + jest.advanceTimersByTime(1000); + expect(Scheduler).toHaveYielded(['Promise resolved [b2]']); + expect(Scheduler).toFlushAndYield(['a', 'b2', 'c']); + + // Render the final update. A should still be hidden, because it was + // given a `hidden` prop. + expect(ReactNoop).toMatchRenderedOutput(AB2C); + }); + + describe('profiler durations', () => { + let App; + let onRender; - it('preserves host context for text nodes', () => { - function App(props) { + beforeEach(() => { + // Order of parameters: id, phase, actualDuration, treeBaseDuration + onRender = jest.fn(); + + const Fallback = () => { + Scheduler.yieldValue('Fallback'); + Scheduler.advanceTime(10); + return 'Loading...'; + }; + + const Suspending = () => { + Scheduler.yieldValue('Suspending'); + Scheduler.advanceTime(2); + return ; + }; + + App = ({shouldSuspend, text = 'Text', textRenderDuration = 5}) => { + Scheduler.yieldValue('App'); return ( - // uppercase is a special type that causes React Noop to render child - // text nodes as uppercase. - - }> - - - + + }> + {shouldSuspend && } + - + ); - } - - // Initial mount - ReactNoop.render(); - - expect(Scheduler).toFlushAndYield([ - 'a', - 'Suspend! [b]', - 'c', - 'Loading...', - ]); - - expect(ReactNoop).toMatchRenderedOutput(null); - - jest.advanceTimersByTime(1000); - expect(Scheduler).toHaveYielded(['Promise resolved [b]']); - expect(Scheduler).toFlushAndYield(['a', 'b', 'c']); - expect(ReactNoop).toMatchRenderedOutput(ABC); - - // Update - ReactNoop.render(); - expect(Scheduler).toFlushAndYield([ - 'a', - 'Suspend! [b2]', - 'c', - 'Loading...', - ]); - // Time out the update - jest.advanceTimersByTime(750); - expect(Scheduler).toFlushAndYield([]); - expect(ReactNoop).toMatchRenderedOutput( - LOADING..., - ); - - // Resolve the promise - jest.advanceTimersByTime(1000); - expect(Scheduler).toHaveYielded(['Promise resolved [b2]']); - expect(Scheduler).toFlushAndYield(['a', 'b2', 'c']); - - // Render the final update. A should still be hidden, because it was - // given a `hidden` prop. - expect(ReactNoop).toMatchRenderedOutput(AB2C); + }; }); - describe('profiler durations', () => { - let App; - let onRender; - - beforeEach(() => { - // Order of parameters: id, phase, actualDuration, treeBaseDuration - onRender = jest.fn(); - - const Fallback = () => { - Scheduler.yieldValue('Fallback'); - Scheduler.advanceTime(10); - return 'Loading...'; - }; - - const Suspending = () => { - Scheduler.yieldValue('Suspending'); - Scheduler.advanceTime(2); - return ; - }; - - App = ({shouldSuspend, text = 'Text', textRenderDuration = 5}) => { - Scheduler.yieldValue('App'); - return ( - - }> - {shouldSuspend && } - - - - ); - }; + describe('when suspending during mount', () => { + it('properly accounts for base durations when a suspended times out in a sync tree', () => { + ReactNoop.renderLegacySyncRoot(); + expect(Scheduler).toHaveYielded([ + 'App', + 'Suspending', + 'Suspend! [Loaded]', + 'Text', + 'Fallback', + ]); + expect(ReactNoop).toMatchRenderedOutput('Loading...'); + expect(onRender).toHaveBeenCalledTimes(1); + + // Initial mount only shows the "Loading..." Fallback. + // The treeBaseDuration then should be 10ms spent rendering Fallback, + // but the actualDuration should also include the 8ms spent rendering the hidden tree. + expect(onRender.mock.calls[0][2]).toBe(18); + expect(onRender.mock.calls[0][3]).toBe(10); + + jest.advanceTimersByTime(1000); + + expect(Scheduler).toHaveYielded([ + 'Promise resolved [Loaded]', + 'Loaded', + ]); + expect(ReactNoop).toMatchRenderedOutput('LoadedText'); + expect(onRender).toHaveBeenCalledTimes(2); + + // When the suspending data is resolved and our final UI is rendered, + // the baseDuration should only include the 1ms re-rendering AsyncText, + // but the treeBaseDuration should include the full 8ms spent in the tree. + expect(onRender.mock.calls[1][2]).toBe(1); + expect(onRender.mock.calls[1][3]).toBe(8); }); - describe('when suspending during mount', () => { - it('properly accounts for base durations when a suspended times out in a sync tree', () => { - ReactNoop.renderLegacySyncRoot(); - expect(Scheduler).toHaveYielded([ - 'App', - 'Suspending', - 'Suspend! [Loaded]', - 'Text', - 'Fallback', - ]); - expect(ReactNoop).toMatchRenderedOutput('Loading...'); - expect(onRender).toHaveBeenCalledTimes(1); - - // Initial mount only shows the "Loading..." Fallback. - // The treeBaseDuration then should be 10ms spent rendering Fallback, - // but the actualDuration should also include the 8ms spent rendering the hidden tree. - expect(onRender.mock.calls[0][2]).toBe(18); - expect(onRender.mock.calls[0][3]).toBe(10); - - jest.advanceTimersByTime(1000); - - expect(Scheduler).toHaveYielded([ - 'Promise resolved [Loaded]', - 'Loaded', - ]); - expect(ReactNoop).toMatchRenderedOutput('LoadedText'); - expect(onRender).toHaveBeenCalledTimes(2); - - // When the suspending data is resolved and our final UI is rendered, - // the baseDuration should only include the 1ms re-rendering AsyncText, - // but the treeBaseDuration should include the full 8ms spent in the tree. - expect(onRender.mock.calls[1][2]).toBe(1); - expect(onRender.mock.calls[1][3]).toBe(8); - }); - - it('properly accounts for base durations when a suspended times out in a concurrent tree', () => { - ReactNoop.render(); - - expect(Scheduler).toFlushAndYield([ - 'App', - 'Suspending', - 'Suspend! [Loaded]', - 'Text', - 'Fallback', - ]); - expect(ReactNoop).toMatchRenderedOutput(null); - - // Show the fallback UI. - jest.advanceTimersByTime(750); - expect(ReactNoop).toMatchRenderedOutput('Loading...'); - expect(onRender).toHaveBeenCalledTimes(1); - - // Initial mount only shows the "Loading..." Fallback. - // The treeBaseDuration then should be 10ms spent rendering Fallback, - // but the actualDuration should also include the 8ms spent rendering the hidden tree. - expect(onRender.mock.calls[0][2]).toBe(18); - expect(onRender.mock.calls[0][3]).toBe(10); - - // Resolve the pending promise. - jest.advanceTimersByTime(250); - expect(Scheduler).toHaveYielded(['Promise resolved [Loaded]']); - expect(Scheduler).toFlushAndYield(['Suspending', 'Loaded', 'Text']); - expect(ReactNoop).toMatchRenderedOutput('LoadedText'); - expect(onRender).toHaveBeenCalledTimes(2); - - // When the suspending data is resolved and our final UI is rendered, - // both times should include the 8ms re-rendering Suspending and AsyncText. - expect(onRender.mock.calls[1][2]).toBe(8); - expect(onRender.mock.calls[1][3]).toBe(8); - }); + it('properly accounts for base durations when a suspended times out in a concurrent tree', () => { + ReactNoop.render(); + + expect(Scheduler).toFlushAndYield([ + 'App', + 'Suspending', + 'Suspend! [Loaded]', + 'Text', + 'Fallback', + ]); + expect(ReactNoop).toMatchRenderedOutput(null); + + // Show the fallback UI. + jest.advanceTimersByTime(750); + expect(ReactNoop).toMatchRenderedOutput('Loading...'); + expect(onRender).toHaveBeenCalledTimes(1); + + // Initial mount only shows the "Loading..." Fallback. + // The treeBaseDuration then should be 10ms spent rendering Fallback, + // but the actualDuration should also include the 8ms spent rendering the hidden tree. + expect(onRender.mock.calls[0][2]).toBe(18); + expect(onRender.mock.calls[0][3]).toBe(10); + + // Resolve the pending promise. + jest.advanceTimersByTime(250); + expect(Scheduler).toHaveYielded(['Promise resolved [Loaded]']); + expect(Scheduler).toFlushAndYield(['Suspending', 'Loaded', 'Text']); + expect(ReactNoop).toMatchRenderedOutput('LoadedText'); + expect(onRender).toHaveBeenCalledTimes(2); + + // When the suspending data is resolved and our final UI is rendered, + // both times should include the 8ms re-rendering Suspending and AsyncText. + expect(onRender.mock.calls[1][2]).toBe(8); + expect(onRender.mock.calls[1][3]).toBe(8); + }); + }); + + describe('when suspending during update', () => { + it('properly accounts for base durations when a suspended times out in a sync tree', () => { + ReactNoop.renderLegacySyncRoot( + , + ); + expect(Scheduler).toHaveYielded(['App', 'Text']); + expect(ReactNoop).toMatchRenderedOutput('Text'); + expect(onRender).toHaveBeenCalledTimes(1); + + // Initial mount only shows the "Text" text. + // It should take 5ms to render. + expect(onRender.mock.calls[0][2]).toBe(5); + expect(onRender.mock.calls[0][3]).toBe(5); + + ReactNoop.render(); + expect(Scheduler).toHaveYielded([ + 'App', + 'Suspending', + 'Suspend! [Loaded]', + 'Text', + 'Fallback', + ]); + expect(ReactNoop).toMatchRenderedOutput('Loading...'); + expect(onRender).toHaveBeenCalledTimes(2); + + // The suspense update should only show the "Loading..." Fallback. + // Both durations should include 10ms spent rendering Fallback + // plus the 8ms rendering the (hidden) components. + expect(onRender.mock.calls[1][2]).toBe(18); + expect(onRender.mock.calls[1][3]).toBe(18); + + ReactNoop.renderLegacySyncRoot( + , + ); + expect(Scheduler).toHaveYielded([ + 'App', + 'Suspending', + 'Suspend! [Loaded]', + 'New', + 'Fallback', + ]); + expect(ReactNoop).toMatchRenderedOutput('Loading...'); + expect(onRender).toHaveBeenCalledTimes(3); + + // If we force another update while still timed out, + // but this time the Text component took 1ms longer to render. + // This should impact both actualDuration and treeBaseDuration. + expect(onRender.mock.calls[2][2]).toBe(19); + expect(onRender.mock.calls[2][3]).toBe(19); + + jest.advanceTimersByTime(1000); + + expect(Scheduler).toHaveYielded([ + 'Promise resolved [Loaded]', + 'Loaded', + ]); + expect(ReactNoop).toMatchRenderedOutput('LoadedNew'); + expect(onRender).toHaveBeenCalledTimes(4); + + // When the suspending data is resolved and our final UI is rendered, + // the baseDuration should only include the 1ms re-rendering AsyncText, + // but the treeBaseDuration should include the full 9ms spent in the tree. + expect(onRender.mock.calls[3][2]).toBe(1); + expect(onRender.mock.calls[3][3]).toBe(9); }); - describe('when suspending during update', () => { - it('properly accounts for base durations when a suspended times out in a sync tree', () => { - ReactNoop.renderLegacySyncRoot( - , - ); - expect(Scheduler).toHaveYielded(['App', 'Text']); - expect(ReactNoop).toMatchRenderedOutput('Text'); - expect(onRender).toHaveBeenCalledTimes(1); - - // Initial mount only shows the "Text" text. - // It should take 5ms to render. - expect(onRender.mock.calls[0][2]).toBe(5); - expect(onRender.mock.calls[0][3]).toBe(5); - - ReactNoop.render(); - expect(Scheduler).toHaveYielded([ - 'App', - 'Suspending', - 'Suspend! [Loaded]', - 'Text', - 'Fallback', - ]); - expect(ReactNoop).toMatchRenderedOutput('Loading...'); - expect(onRender).toHaveBeenCalledTimes(2); - - // The suspense update should only show the "Loading..." Fallback. - // Both durations should include 10ms spent rendering Fallback - // plus the 8ms rendering the (hidden) components. - expect(onRender.mock.calls[1][2]).toBe(18); - expect(onRender.mock.calls[1][3]).toBe(18); - - ReactNoop.renderLegacySyncRoot( - , - ); - expect(Scheduler).toHaveYielded([ - 'App', - 'Suspending', - 'Suspend! [Loaded]', - 'New', - 'Fallback', - ]); - expect(ReactNoop).toMatchRenderedOutput('Loading...'); - expect(onRender).toHaveBeenCalledTimes(3); - - // If we force another update while still timed out, - // but this time the Text component took 1ms longer to render. - // This should impact both actualDuration and treeBaseDuration. - expect(onRender.mock.calls[2][2]).toBe(19); - expect(onRender.mock.calls[2][3]).toBe(19); - - jest.advanceTimersByTime(1000); - - expect(Scheduler).toHaveYielded([ - 'Promise resolved [Loaded]', - 'Loaded', - ]); - expect(ReactNoop).toMatchRenderedOutput('LoadedNew'); - expect(onRender).toHaveBeenCalledTimes(4); - - // When the suspending data is resolved and our final UI is rendered, - // the baseDuration should only include the 1ms re-rendering AsyncText, - // but the treeBaseDuration should include the full 9ms spent in the tree. - expect(onRender.mock.calls[3][2]).toBe(1); - expect(onRender.mock.calls[3][3]).toBe(9); - }); - - it('properly accounts for base durations when a suspended times out in a concurrent tree', () => { - ReactNoop.render( - , - ); - - expect(Scheduler).toFlushAndYield(['App', 'Text']); - expect(ReactNoop).toMatchRenderedOutput('Text'); - expect(onRender).toHaveBeenCalledTimes(1); - - // Initial mount only shows the "Text" text. - // It should take 5ms to render. - expect(onRender.mock.calls[0][2]).toBe(5); - expect(onRender.mock.calls[0][3]).toBe(5); - - ReactNoop.render(); - expect(Scheduler).toFlushAndYield([ - 'App', - 'Suspending', - 'Suspend! [Loaded]', - 'Text', - 'Fallback', - ]); - expect(ReactNoop).toMatchRenderedOutput('Text'); - - // Show the fallback UI. - jest.advanceTimersByTime(750); - expect(ReactNoop).toMatchRenderedOutput('Loading...'); - expect(onRender).toHaveBeenCalledTimes(2); - - // The suspense update should only show the "Loading..." Fallback. - // The actual duration should include 10ms spent rendering Fallback, - // plus the 8ms render all of the hidden, suspended subtree. - // But the tree base duration should only include 10ms spent rendering Fallback, - // plus the 5ms rendering the previously committed version of the hidden tree. - expect(onRender.mock.calls[1][2]).toBe(18); - expect(onRender.mock.calls[1][3]).toBe(15); - - // Update again while timed out. - ReactNoop.render( - , - ); - expect(Scheduler).toFlushAndYield([ - 'App', - 'Suspending', - 'Suspend! [Loaded]', - 'New', - 'Fallback', - ]); - expect(ReactNoop).toMatchRenderedOutput('Loading...'); - expect(onRender).toHaveBeenCalledTimes(2); - - // Resolve the pending promise. - jest.advanceTimersByTime(250); - expect(Scheduler).toHaveYielded(['Promise resolved [Loaded]']); - expect(Scheduler).toFlushAndYield([ - 'App', - 'Suspending', - 'Loaded', - 'New', - ]); - expect(onRender).toHaveBeenCalledTimes(3); - - // When the suspending data is resolved and our final UI is rendered, - // both times should include the 6ms rendering Text, - // the 2ms rendering Suspending, and the 1ms rendering AsyncText. - expect(onRender.mock.calls[2][2]).toBe(9); - expect(onRender.mock.calls[2][3]).toBe(9); - }); + it('properly accounts for base durations when a suspended times out in a concurrent tree', () => { + ReactNoop.render(); + + expect(Scheduler).toFlushAndYield(['App', 'Text']); + expect(ReactNoop).toMatchRenderedOutput('Text'); + expect(onRender).toHaveBeenCalledTimes(1); + + // Initial mount only shows the "Text" text. + // It should take 5ms to render. + expect(onRender.mock.calls[0][2]).toBe(5); + expect(onRender.mock.calls[0][3]).toBe(5); + + ReactNoop.render(); + expect(Scheduler).toFlushAndYield([ + 'App', + 'Suspending', + 'Suspend! [Loaded]', + 'Text', + 'Fallback', + ]); + expect(ReactNoop).toMatchRenderedOutput('Text'); + + // Show the fallback UI. + jest.advanceTimersByTime(750); + expect(ReactNoop).toMatchRenderedOutput('Loading...'); + expect(onRender).toHaveBeenCalledTimes(2); + + // The suspense update should only show the "Loading..." Fallback. + // The actual duration should include 10ms spent rendering Fallback, + // plus the 8ms render all of the hidden, suspended subtree. + // But the tree base duration should only include 10ms spent rendering Fallback, + // plus the 5ms rendering the previously committed version of the hidden tree. + expect(onRender.mock.calls[1][2]).toBe(18); + expect(onRender.mock.calls[1][3]).toBe(15); + + // Update again while timed out. + ReactNoop.render( + , + ); + expect(Scheduler).toFlushAndYield([ + 'App', + 'Suspending', + 'Suspend! [Loaded]', + 'New', + 'Fallback', + ]); + expect(ReactNoop).toMatchRenderedOutput('Loading...'); + expect(onRender).toHaveBeenCalledTimes(2); + + // Resolve the pending promise. + jest.advanceTimersByTime(250); + expect(Scheduler).toHaveYielded(['Promise resolved [Loaded]']); + expect(Scheduler).toFlushAndYield([ + 'App', + 'Suspending', + 'Loaded', + 'New', + ]); + expect(onRender).toHaveBeenCalledTimes(3); + + // When the suspending data is resolved and our final UI is rendered, + // both times should include the 6ms rendering Text, + // the 2ms rendering Suspending, and the 1ms rendering AsyncText. + expect(onRender.mock.calls[2][2]).toBe(9); + expect(onRender.mock.calls[2][3]).toBe(9); }); }); }); -} +}); From 4d0a38449da7ecd4482412ada1148af4f6e42728 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 8 Mar 2019 20:37:34 -0800 Subject: [PATCH 8/9] Revert "test-persistent should skip files without noop" Seb objected to adding shelljs as a dep and I'm too lazy to worry about Windows support so whatever I'll just revert this. --- package.json | 5 ++-- scripts/jest/config.source-persistent.js | 19 -------------- yarn.lock | 33 ------------------------ 3 files changed, 2 insertions(+), 55 deletions(-) diff --git a/package.json b/package.json index 893c0fb6b6870..29e3aaea389dc 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,6 @@ "packages/*" ], "devDependencies": { - "@mattiasbuelens/web-streams-polyfill": "0.1.0", "art": "^0.10.1", "babel-cli": "^6.6.5", "babel-code-frame": "^6.26.0", @@ -81,11 +80,11 @@ "rollup-plugin-replace": "^2.0.0", "rollup-plugin-strip-banner": "^0.2.0", "semver": "^5.5.0", - "shelljs": "^0.8.3", "targz": "^1.0.1", "through2": "^2.0.0", "tmp": "~0.0.28", - "typescript": "~1.8.10" + "typescript": "~1.8.10", + "@mattiasbuelens/web-streams-polyfill": "0.1.0" }, "devEngines": { "node": "8.x || 9.x || 10.x || 11.x" diff --git a/scripts/jest/config.source-persistent.js b/scripts/jest/config.source-persistent.js index dd8cf190df2b0..edda459695f50 100644 --- a/scripts/jest/config.source-persistent.js +++ b/scripts/jest/config.source-persistent.js @@ -1,28 +1,9 @@ 'use strict'; -const path = require('path'); -const {find, grep} = require('shelljs'); const baseConfig = require('./config.base'); -// Search for all the test files that reference react-noop-renderer -const packagesDir = path.join(process.cwd(), 'packages'); -const testRegex = new RegExp(baseConfig.testRegex); -const testFiles = find('packages').filter(filename => testRegex.test(filename)); -const testMatchFilesThatReferenceNoop = grep( - '-l', - 'react-noop-renderer', - testFiles -) - .split('\n') - .map(filename => `**/${path.relative(packagesDir, filename)}`); - module.exports = Object.assign({}, baseConfig, { - // Override base regex so that we only match the files we found above. - testRegex: null, - testMatch: testMatchFilesThatReferenceNoop, modulePathIgnorePatterns: [ - ...baseConfig.modulePathIgnorePatterns, - // These files only work in mutation mode 'ReactIncrementalPerf', 'ReactIncrementalUpdatesMinimalism', 'ReactIncrementalTriangle', diff --git a/yarn.lock b/yarn.lock index a688ebca3128d..63f2929f3668a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2228,18 +2228,6 @@ glob@^6.0.4: once "^1.3.0" path-is-absolute "^1.0.0" -glob@^7.0.0: - version "7.1.3" - resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.3.tgz#3960832d3f1574108342dafd3a67b332c0969df1" - integrity sha512-vcfuiIxogLV4DlGBHIUOwI0IbrJ8HWPc4MU7HzviGeNho/UJDfi6B5p3sHeWIQ0KGIU0Jpxi5ZHxemQfLkkAwQ== - dependencies: - fs.realpath "^1.0.0" - inflight "^1.0.4" - inherits "2" - minimatch "^3.0.4" - once "^1.3.0" - path-is-absolute "^1.0.0" - glob@^7.0.3, glob@^7.0.5, glob@^7.1.1, glob@^7.1.2: version "7.1.2" resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.2.tgz#c19c9df9a028702d678612384a6552404c636d15" @@ -2538,11 +2526,6 @@ inquirer@^3.0.6: strip-ansi "^4.0.0" through "^2.3.6" -interpret@^1.0.0: - version "1.2.0" - resolved "https://registry.yarnpkg.com/interpret/-/interpret-1.2.0.tgz#d5061a6224be58e8083985f5014d844359576296" - integrity sha512-mT34yGKMNceBQUoVn7iCDKDntA7SC6gycMAWzGx1z/CMCTV7b2AAtXlo3nRyHZ1FelRkQbQjprHSYGwzLtkVbw== - invariant@^2.2.2: version "2.2.4" resolved "https://registry.yarnpkg.com/invariant/-/invariant-2.2.4.tgz#610f3c92c9359ce1db616e538008d23ff35158e6" @@ -4196,13 +4179,6 @@ realpath-native@^1.0.0: dependencies: util.promisify "^1.0.0" -rechoir@^0.6.2: - version "0.6.2" - resolved "https://registry.yarnpkg.com/rechoir/-/rechoir-0.6.2.tgz#85204b54dba82d5742e28c96756ef43af50e3384" - integrity sha1-hSBLVNuoLVdC4oyWdW70OvUOM4Q= - dependencies: - resolve "^1.1.6" - regenerator-runtime@^0.10.5: version "0.10.5" resolved "https://registry.yarnpkg.com/regenerator-runtime/-/regenerator-runtime-0.10.5.tgz#336c3efc1220adcedda2c9fab67b5a7955a33658" @@ -4573,15 +4549,6 @@ shebang-regex@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/shebang-regex/-/shebang-regex-1.0.0.tgz#da42f49740c0b42db2ca9728571cb190c98efea3" -shelljs@^0.8.3: - version "0.8.3" - resolved "https://registry.yarnpkg.com/shelljs/-/shelljs-0.8.3.tgz#a7f3319520ebf09ee81275b2368adb286659b097" - integrity sha512-fc0BKlAWiLpwZljmOvAOTE/gXawtCoNrP5oaY7KIaQbbyHeQVg01pSEuEGvGh3HEdBU4baCD7wQBwADmM/7f7A== - dependencies: - glob "^7.0.0" - interpret "^1.0.0" - rechoir "^0.6.2" - shellwords@^0.1.1: version "0.1.1" resolved "https://registry.yarnpkg.com/shellwords/-/shellwords-0.1.1.tgz#d6b9181c1a48d397324c84871efbcfc73fc0654b" From 2cf0c14662d5cc1461233d725978188ce3de524e Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 11 Mar 2019 10:38:19 -0700 Subject: [PATCH 9/9] Delete duplicate file --- scripts/jest/config.source-persistent.js | 2 +- scripts/jest/setupPersistent.js | 7 ------- 2 files changed, 1 insertion(+), 8 deletions(-) delete mode 100644 scripts/jest/setupPersistent.js diff --git a/scripts/jest/config.source-persistent.js b/scripts/jest/config.source-persistent.js index edda459695f50..df6750d3f5f73 100644 --- a/scripts/jest/config.source-persistent.js +++ b/scripts/jest/config.source-persistent.js @@ -12,7 +12,7 @@ module.exports = Object.assign({}, baseConfig, { ], setupFiles: [ ...baseConfig.setupFiles, - require.resolve('./setupPersistent.js'), + require.resolve('./setupTests.persistent.js'), require.resolve('./setupHostConfigs.js'), ], }); diff --git a/scripts/jest/setupPersistent.js b/scripts/jest/setupPersistent.js deleted file mode 100644 index e07e74e456e02..0000000000000 --- a/scripts/jest/setupPersistent.js +++ /dev/null @@ -1,7 +0,0 @@ -'use strict'; - -jest.mock('react-noop-renderer', () => - require.requireActual('react-noop-renderer/persistent') -); - -global.__PERSISTENT__ = true;