diff --git a/.circleci/config.yml b/.circleci/config.yml index f859b2c59a309..b5f7604059020 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,4 +1,4 @@ -version: 2 +version: 2.1 aliases: - &docker @@ -104,139 +104,6 @@ jobs: - *restore_node_modules - run: node ./scripts/tasks/flow-ci - yarn_test-stable: - docker: *docker - environment: *environment - parallelism: *TEST_PARALLELISM - - steps: - - checkout - - run: yarn workspaces info | head -n -1 > workspace_info.txt - - *restore_node_modules - - run: yarn test-stable --ci - - yarn_test: - docker: *docker - environment: *environment - parallelism: *TEST_PARALLELISM - steps: - - checkout - - run: yarn workspaces info | head -n -1 > workspace_info.txt - - *restore_node_modules - - run: yarn test --ci - - yarn_test-classic: - docker: *docker - environment: *environment - parallelism: *TEST_PARALLELISM - steps: - - checkout - - run: yarn workspaces info | head -n -1 > workspace_info.txt - - *restore_node_modules - - run: yarn test-classic --ci - - yarn_test-classic_variant: - docker: *docker - environment: *environment - parallelism: *TEST_PARALLELISM - steps: - - checkout - - run: yarn workspaces info | head -n -1 > workspace_info.txt - - *restore_node_modules - - run: yarn test-classic --variant --ci - - yarn_test-classic_prod: - docker: *docker - environment: *environment - parallelism: *TEST_PARALLELISM - steps: - - checkout - - run: yarn workspaces info | head -n -1 > workspace_info.txt - - *restore_node_modules - - run: yarn test-classic --prod --ci - - yarn_test-classic_prod_variant: - docker: *docker - environment: *environment - parallelism: *TEST_PARALLELISM - steps: - - checkout - - run: yarn workspaces info | head -n -1 > workspace_info.txt - - *restore_node_modules - - run: yarn test-classic --prod --variant --ci - - yarn_test-www: - docker: *docker - environment: *environment - parallelism: *TEST_PARALLELISM - steps: - - checkout - - run: yarn workspaces info | head -n -1 > workspace_info.txt - - *restore_node_modules - - run: yarn test-www --ci - - yarn_test-www_variant: - docker: *docker - environment: *environment - parallelism: *TEST_PARALLELISM - steps: - - checkout - - run: yarn workspaces info | head -n -1 > workspace_info.txt - - *restore_node_modules - - run: yarn test-www --variant --ci - - yarn_test-www_prod: - docker: *docker - environment: *environment - parallelism: *TEST_PARALLELISM - steps: - - checkout - - run: yarn workspaces info | head -n -1 > workspace_info.txt - - *restore_node_modules - - run: yarn test-www --prod --ci - - yarn_test-www_prod_variant: - docker: *docker - environment: *environment - parallelism: *TEST_PARALLELISM - steps: - - checkout - - run: yarn workspaces info | head -n -1 > workspace_info.txt - - *restore_node_modules - - run: yarn test-www --prod --variant --ci - - yarn_test-stable_persistent: - docker: *docker - environment: *environment - parallelism: *TEST_PARALLELISM - - steps: - - checkout - - run: yarn workspaces info | head -n -1 > workspace_info.txt - - *restore_node_modules - - run: yarn test-stable --persistent --ci - - yarn_test-stable_prod: - docker: *docker - environment: *environment - parallelism: *TEST_PARALLELISM - - steps: - - checkout - - run: yarn workspaces info | head -n -1 > workspace_info.txt - - *restore_node_modules - - run: yarn test-stable --prod --ci - - yarn_test_prod: - docker: *docker - environment: *environment - parallelism: *TEST_PARALLELISM - steps: - - checkout - - run: yarn workspaces info | head -n -1 > workspace_info.txt - - *restore_node_modules - - run: yarn test --prod --ci - RELEASE_CHANNEL_stable_yarn_build: docker: *docker environment: *environment @@ -293,6 +160,41 @@ jobs: - dist - sizes/*.json + yarn_build_combined: + docker: *docker + environment: *environment + parallelism: 40 + steps: + - checkout + - run: yarn workspaces info | head -n -1 > workspace_info.txt + - *restore_node_modules + - run: yarn build-combined + - persist_to_workspace: + root: build2 + paths: + - facebook-www + - facebook-react-native + - facebook-relay + - oss-stable + - oss-experimental + - react-native + - dist + - sizes/*.json + + process_artifacts_combined: + docker: *docker + environment: *environment + steps: + - checkout + - attach_workspace: + at: build2 + - run: yarn workspaces info | head -n -1 > workspace_info.txt + - *restore_node_modules + # Compress build directory into a single tarball for easy download + - run: tar -zcvf ./build2.tgz ./build2 + - store_artifacts: + path: ./build2.tgz + build_devtools_and_process_artifacts: docker: *docker environment: *environment @@ -416,37 +318,33 @@ jobs: command: yarn lint-build - run: scripts/circleci/check_minified_errors.sh - yarn_test-stable_build: + yarn_test: docker: *docker environment: *environment parallelism: *TEST_PARALLELISM + parameters: + args: + type: string steps: - checkout - - attach_workspace: *attach_workspace - run: yarn workspaces info | head -n -1 > workspace_info.txt - *restore_node_modules - - run: yarn test-stable --build --ci + - run: yarn test <> --ci yarn_test_build: docker: *docker environment: *environment parallelism: *TEST_PARALLELISM + parameters: + args: + type: string steps: - checkout - - attach_workspace: *attach_workspace - - run: yarn workspaces info | head -n -1 > workspace_info.txt - - *restore_node_modules - - run: yarn test --build --ci - - yarn_test_build_devtools: - docker: *docker - environment: *environment - steps: - - checkout - - attach_workspace: *attach_workspace + - attach_workspace: + at: build2 - run: yarn workspaces info | head -n -1 > workspace_info.txt - *restore_node_modules - - run: yarn test --project=devtools --build --ci + - run: yarn test --build <> --ci RELEASE_CHANNEL_stable_yarn_test_dom_fixtures: docker: *docker @@ -479,28 +377,6 @@ jobs: FUZZ_TEST_SEED=$RANDOM yarn test fuzz --ci FUZZ_TEST_SEED=$RANDOM yarn test --prod fuzz --ci - yarn_test-stable_build_prod: - docker: *docker - environment: *environment - parallelism: *TEST_PARALLELISM - steps: - - checkout - - attach_workspace: *attach_workspace - - run: yarn workspaces info | head -n -1 > workspace_info.txt - - *restore_node_modules - - run: yarn test-stable --build --prod --ci - - yarn_test_build_prod: - docker: *docker - environment: *environment - parallelism: *TEST_PARALLELISM - steps: - - checkout - - attach_workspace: *attach_workspace - - run: yarn workspaces info | head -n -1 > workspace_info.txt - - *restore_node_modules - - run: yarn test --build --prod --ci - workflows: version: 2 stable: @@ -512,27 +388,6 @@ workflows: - yarn_flow: requires: - setup - - yarn_test-stable: - requires: - - setup - - yarn_test-stable_prod: - requires: - - setup - - yarn_test-stable_persistent: - requires: - - setup - - yarn_test-classic: - requires: - - setup - - yarn_test-classic_variant: - requires: - - setup - - yarn_test-classic_prod: - requires: - - setup - - yarn_test-classic_prod_variant: - requires: - - setup - RELEASE_CHANNEL_stable_yarn_build: requires: - setup @@ -545,12 +400,6 @@ workflows: - RELEASE_CHANNEL_stable_yarn_lint_build: requires: - RELEASE_CHANNEL_stable_yarn_build - - yarn_test-stable_build: - requires: - - RELEASE_CHANNEL_stable_yarn_build - - yarn_test-stable_build_prod: - requires: - - RELEASE_CHANNEL_stable_yarn_build - RELEASE_CHANNEL_stable_yarn_test_dom_fixtures: requires: - RELEASE_CHANNEL_stable_yarn_build @@ -558,24 +407,6 @@ workflows: experimental: jobs: - setup - - yarn_test: - requires: - - setup - - yarn_test_prod: - requires: - - setup - - yarn_test-www: - requires: - - setup - - yarn_test-www_variant: - requires: - - setup - - yarn_test-www_prod: - requires: - - setup - - yarn_test-www_prod_variant: - requires: - - setup - yarn_build: requires: - setup @@ -585,18 +416,9 @@ workflows: - sizebot_experimental: requires: - yarn_build - - yarn_test_build: - requires: - - yarn_build - - yarn_test_build_prod: - requires: - - yarn_build - yarn_lint_build: requires: - yarn_build - - yarn_test_build_devtools: - requires: - - yarn_build - build_devtools_and_process_artifacts: requires: - yarn_build @@ -611,6 +433,68 @@ workflows: only: - master + # New workflow that will replace "stable" and "experimental" + build_and_test: + jobs: + - setup + - yarn_test: + requires: + - setup + matrix: + parameters: + args: + # Intentionally passing these as strings instead of creating a + # separate parameter per CLI argument, since it's easier to + # control/see which combinations we want to run. + - "-r=stable --env=development" + - "-r=stable --env=production" + - "-r=experimental --env=development" + - "-r=experimental --env=production" + - "-r=www-classic --env=development" + - "-r=www-classic --env=production" + - "-r=www-classic --env=development --variant" + - "-r=www-classic --env=production --variant" + - "-r=www-modern --env=development" + - "-r=www-modern --env=production" + - "-r=www-modern --env=development --variant" + - "-r=www-modern --env=production --variant" + + # TODO: Test more persistent configurations? + - '-r=stable --env=development --persistent' + - yarn_build_combined: + requires: + - setup + - process_artifacts_combined: + requires: + - yarn_build_combined + - yarn_test_build: + requires: + - yarn_build_combined + matrix: + parameters: + args: + # Intentionally passing these as strings instead of creating a + # separate parameter per CLI argument, since it's easier to + # control/see which combinations we want to run. + - "-r=stable --env=development" + - "-r=stable --env=production" + - "-r=experimental --env=development" + - "-r=experimental --env=production" + + # Dev Tools + - "--project=devtools -r=experimental" + + # TODO: Update test config to support www build tests + # - "-r=www-classic --env=development" + # - "-r=www-classic --env=production" + # - "-r=www-classic --env=development --variant" + # - "-r=www-classic --env=production --variant" + # - "-r=www-modern --env=development" + # - "-r=www-modern --env=production" + # - "-r=www-modern --env=development --variant" + # - "-r=www-modern --env=production --variant" + + # TODO: Test more persistent configurations? fuzz_tests: triggers: - schedule: diff --git a/.codesandbox/ci.json b/.codesandbox/ci.json index 6b3a763cf0839..6f276d052e4a8 100644 --- a/.codesandbox/ci.json +++ b/.codesandbox/ci.json @@ -1,6 +1,6 @@ { "packages": ["packages/react", "packages/react-dom", "packages/scheduler"], - "buildCommand": "build --type=NODE react/index,react-dom/index,react-dom/server,react-dom/test-utils,scheduler/index,scheduler/tracing", + "buildCommand": "build --type=NODE react/index,react-dom/index,react-dom/server,react-dom/test-utils,scheduler/index,scheduler/unstable_no_dom,scheduler/tracing", "publishDirectory": { "react": "build/node_modules/react", "react-dom": "build/node_modules/react-dom", diff --git a/.gitignore b/.gitignore index 5a913ac6e5278..fed2a08aa5d9e 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ scripts/flow/*/.flowconfig _SpecRunner.html __benchmarks__ build/ +build2/ remote-repo/ coverage/ .module-cache diff --git a/package.json b/package.json index bf225d9cdc81c..bca998508f1dc 100644 --- a/package.json +++ b/package.json @@ -106,7 +106,8 @@ }, "scripts": { "build": "node ./scripts/rollup/build.js", - "build-for-devtools": "cross-env RELEASE_CHANNEL=experimental yarn build react/index,react-dom,react-is,react-debug-tools,scheduler,react-test-renderer,react-refresh", + "build-combined": "node ./scripts/rollup/build-all-release-channels.js", + "build-for-devtools": "cross-env RELEASE_CHANNEL=experimental yarn build-combined react/index,react-dom,react-is,react-debug-tools,scheduler,react-test-renderer,react-refresh", "build-for-devtools-dev": "yarn build-for-devtools --type=NODE_DEV", "build-for-devtools-prod": "yarn build-for-devtools --type=NODE_PROD", "linc": "node ./scripts/tasks/linc.js", diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js index 0c782dc25ae02..2a2786dca8850 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -1692,6 +1692,42 @@ const tests = { }, ], }, + { + code: normalizeIndent` + function MyComponent() { + useEffect() + useLayoutEffect() + useCallback() + useMemo() + } + `, + errors: [ + { + message: + 'React Hook useEffect requires an effect callback. ' + + 'Did you forget to pass a callback to the hook?', + suggestions: undefined, + }, + { + message: + 'React Hook useLayoutEffect requires an effect callback. ' + + 'Did you forget to pass a callback to the hook?', + suggestions: undefined, + }, + { + message: + 'React Hook useCallback requires an effect callback. ' + + 'Did you forget to pass a callback to the hook?', + suggestions: undefined, + }, + { + message: + 'React Hook useMemo requires an effect callback. ' + + 'Did you forget to pass a callback to the hook?', + suggestions: undefined, + }, + ], + }, { // Regression test code: normalizeIndent` diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js index 7c970f7c8f6ac..ee259b2d37cd6 100644 --- a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js +++ b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js @@ -1119,6 +1119,19 @@ export default { const declaredDependenciesNode = node.arguments[callbackIndex + 1]; const isEffect = /Effect($|[^a-z])/g.test(reactiveHookName); + // Check whether a callback is supplied. If there is no callback supplied + // then the hook will not work and React will throw a TypeError. + // So no need to check for dependency inclusion. + if (!callback) { + reportProblem({ + node: reactiveHook, + message: + `React Hook ${reactiveHookName} requires an effect callback. ` + + `Did you forget to pass a callback to the hook?`, + }); + return; + } + // Check the declared dependencies for this reactive hook. If there is no // second argument then the reactive callback will re-run on every render. // So no need to check for dependency inclusion. diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index ce62b5ce8b9ef..eb7a8f6d4ef14 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -73,6 +73,10 @@ function getPrimitiveStackCache(): Map> { Dispatcher.useState(null); Dispatcher.useReducer((s, a) => s, null); Dispatcher.useRef(null); + if (typeof Dispatcher.useCacheRefresh === 'function') { + // This type check is for Flow only. + Dispatcher.useCacheRefresh(); + } Dispatcher.useLayoutEffect(() => {}); Dispatcher.useEffect(() => {}); Dispatcher.useImperativeHandle(undefined, () => null); @@ -171,6 +175,16 @@ function useRef(initialValue: T): {|current: T|} { return ref; } +function useCacheRefresh(): () => void { + const hook = nextHook(); + hookLog.push({ + primitive: 'CacheRefresh', + stackError: new Error(), + value: hook !== null ? hook.memoizedState : function refresh() {}, + }); + return () => {}; +} + function useLayoutEffect( create: () => (() => void) | void, inputs: Array | void | null, @@ -305,6 +319,7 @@ function useOpaqueIdentifier(): OpaqueIDType | void { const Dispatcher: DispatcherType = { getCacheForType, readContext, + useCacheRefresh, useCallback, useContext, useEffect, diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElementContext-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElementContext-test.js.snap deleted file mode 100644 index b5e3d4fb7a04a..0000000000000 --- a/packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElementContext-test.js.snap +++ /dev/null @@ -1,687 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`InspectedElementContext should allow component prop value and value\`s prototype has same name params.: 1: Inspected element 2 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "data": { - "b": null, - "c": null, - "d": "normal" - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should dehydrate complex nested values when requested: 1: Initially inspect element 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "set_of_sets": { - "0": {}, - "1": {} - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should dehydrate complex nested values when requested: 2: Inspect props.set_of_sets.0 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "set_of_sets": { - "0": { - "0": 1, - "1": 2, - "2": 3 - }, - "1": {} - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should display complex values of useDebugValue: DisplayedComplexValue 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": [ - { - "id": null, - "isStateEditable": false, - "name": "DebuggableHook", - "value": { - "foo": 2 - }, - "subHooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": 1, - "subHooks": [] - } - ] - } - ], - "props": {}, - "state": null -} -`; - -exports[`InspectedElementContext should include updates for nested values that were previously hydrated: 1: Initially inspect element 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "nestedObject": { - "a": {}, - "c": {} - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should include updates for nested values that were previously hydrated: 2: Inspect props.nestedObject.a 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "nestedObject": { - "a": { - "value": 1, - "b": { - "value": 1 - } - }, - "c": {} - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should include updates for nested values that were previously hydrated: 3: Inspect props.nestedObject.c 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "nestedObject": { - "a": { - "value": 1, - "b": { - "value": 1 - } - }, - "c": { - "value": 1, - "d": { - "value": 1, - "e": {} - } - } - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should include updates for nested values that were previously hydrated: 4: update inspected element 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "nestedObject": { - "a": { - "value": 2, - "b": { - "value": 2 - } - }, - "c": { - "value": 2, - "d": { - "value": 2, - "e": {} - } - } - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should inspect hooks for components that only use context: 1: Inspected element 2 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": [ - { - "id": null, - "isStateEditable": false, - "name": "Context", - "value": true, - "subHooks": [] - } - ], - "props": { - "a": 1, - "b": "abc" - }, - "state": null -} -`; - -exports[`InspectedElementContext should inspect the currently selected element: 1: Inspected element 2 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": 1, - "subHooks": [] - } - ], - "props": { - "a": 1, - "b": "abc" - }, - "state": null -} -`; - -exports[`InspectedElementContext should not consume iterables while inspecting: 1: Inspected element 2 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "prop": {} - }, - "state": null -} -`; - -exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 1: Initially inspect element 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": { - "foo": {} - }, - "subHooks": [] - } - ], - "props": { - "nestedObject": { - "a": {} - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 2: Inspect props.nestedObject.a 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": { - "foo": {} - }, - "subHooks": [] - } - ], - "props": { - "nestedObject": { - "a": { - "b": { - "c": {} - } - } - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 3: Inspect props.nestedObject.a.b.c 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": { - "foo": {} - }, - "subHooks": [] - } - ], - "props": { - "nestedObject": { - "a": { - "b": { - "c": [ - { - "d": {} - } - ] - } - } - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 4: Inspect props.nestedObject.a.b.c.0.d 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": { - "foo": {} - }, - "subHooks": [] - } - ], - "props": { - "nestedObject": { - "a": { - "b": { - "c": [ - { - "d": { - "e": {} - } - } - ] - } - } - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 5: Inspect hooks.0.value 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": { - "foo": { - "bar": {} - } - }, - "subHooks": [] - } - ], - "props": { - "nestedObject": { - "a": { - "b": { - "c": [ - { - "d": { - "e": {} - } - } - ] - } - } - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 6: Inspect hooks.0.value.foo.bar 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": { - "foo": { - "bar": { - "baz": "hi" - } - } - }, - "subHooks": [] - } - ], - "props": { - "nestedObject": { - "a": { - "b": { - "c": [ - { - "d": { - "e": {} - } - } - ] - } - } - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should not re-render a function with hooks if it did not update since it was last inspected: 1: initial render 1`] = ` -{ - "id": 3, - "owners": null, - "context": null, - "hooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": 0, - "subHooks": [] - } - ], - "props": { - "a": 1, - "b": "abc" - }, - "state": null -} -`; - -exports[`InspectedElementContext should not re-render a function with hooks if it did not update since it was last inspected: 2: updated state 1`] = ` -{ - "id": 3, - "owners": null, - "context": null, - "hooks": [ - { - "id": 0, - "isStateEditable": true, - "name": "State", - "value": 0, - "subHooks": [] - } - ], - "props": { - "a": 2, - "b": "def" - }, - "state": null -} -`; - -exports[`InspectedElementContext should not tear if hydration is requested after an update: 1: Initially inspect element 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "nestedObject": { - "value": 1, - "a": {} - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should not tear if hydration is requested after an update: 2: Inspect props.nestedObject.a 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "nestedObject": { - "value": 2, - "a": { - "value": 2, - "b": { - "value": 2 - } - } - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should poll for updates for the currently selected element: 1: initial render 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "a": 1, - "b": "abc" - }, - "state": null -} -`; - -exports[`InspectedElementContext should poll for updates for the currently selected element: 2: updated state 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "a": 2, - "b": "def" - }, - "state": null -} -`; - -exports[`InspectedElementContext should support complex data types: 1: Inspected element 2 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "anonymous_fn": {}, - "array_buffer": {}, - "array_of_arrays": [ - {} - ], - "big_int": {}, - "bound_fn": {}, - "data_view": {}, - "date": {}, - "fn": {}, - "html_element": {}, - "immutable": { - "0": {}, - "1": {}, - "2": {} - }, - "map": { - "0": {}, - "1": {} - }, - "map_of_maps": { - "0": {}, - "1": {} - }, - "object_of_objects": { - "inner": {} - }, - "object_with_symbol": { - "Symbol(name)": "hello" - }, - "proxy": {}, - "react_element": {}, - "regexp": {}, - "set": { - "0": "abc", - "1": 123 - }, - "set_of_sets": { - "0": {}, - "1": {} - }, - "symbol": {}, - "typed_array": { - "0": 100, - "1": -100, - "2": 0 - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should support custom objects with enumerable properties and getters: 1: Inspected element 2 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "data": { - "_number": 42, - "number": 42 - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should support objects with no prototype: 1: Inspected element 2 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "object": { - "string": "abc", - "number": 123, - "boolean": true - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should support objects with overridden hasOwnProperty: 1: Inspected element 2 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "object": { - "name": "blah", - "hasOwnProperty": true - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should support objects with with inherited keys: 1: Inspected element 2 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "object": { - "123": 3, - "enumerableString": 2, - "Symbol(enumerableSymbol)": 3, - "enumerableStringBase": 1, - "Symbol(enumerableSymbolBase)": 1 - } - }, - "state": null -} -`; - -exports[`InspectedElementContext should support simple data types: 1: Initial inspection 1`] = ` -{ - "id": 2, - "owners": null, - "context": null, - "hooks": null, - "props": { - "boolean_false": false, - "boolean_true": true, - "infinity": null, - "integer_zero": 0, - "integer_one": 1, - "float": 1.23, - "string": "abc", - "string_empty": "", - "nan": null, - "value_null": null - }, - "state": null -} -`; diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/store-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/store-test.js.snap index cc3c9b6c3cc2d..5ff48f317f479 100644 --- a/packages/react-devtools-shared/src/__tests__/__snapshots__/store-test.js.snap +++ b/packages/react-devtools-shared/src/__tests__/__snapshots__/store-test.js.snap @@ -685,4 +685,5 @@ exports[`Store should show the right display names for special component types 1 [withFoo][withBar] [Memo][withFoo][withBar] [ForwardRef][withFoo][withBar] + `; diff --git a/packages/react-devtools-shared/src/__tests__/dehydratedValueSerializer.js b/packages/react-devtools-shared/src/__tests__/dehydratedValueSerializer.js new file mode 100644 index 0000000000000..d8eac57df035c --- /dev/null +++ b/packages/react-devtools-shared/src/__tests__/dehydratedValueSerializer.js @@ -0,0 +1,39 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +// test() is part of Jest's serializer API +export function test(maybeDehydratedValue) { + const {meta} = require('react-devtools-shared/src/hydration'); + return ( + maybeDehydratedValue !== null && + typeof maybeDehydratedValue === 'object' && + maybeDehydratedValue.hasOwnProperty(meta.inspectable) && + maybeDehydratedValue[meta.inspected] !== true + ); +} + +// print() is part of Jest's serializer API +export function print(dehydratedValue, serialize, indent) { + const {meta} = require('react-devtools-shared/src/hydration'); + const indentation = Math.max(indent('.').indexOf('.') - 2, 0); + const paddingLeft = ' '.repeat(indentation); + return ( + 'Dehydrated {\n' + + paddingLeft + + ' "preview_short": ' + + dehydratedValue[meta.preview_short] + + ',\n' + + paddingLeft + + ' "preview_long": ' + + dehydratedValue[meta.preview_long] + + ',\n' + + paddingLeft + + '}' + ); +} diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js similarity index 75% rename from packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js rename to packages/react-devtools-shared/src/__tests__/inspectedElement-test.js index d782d917264c6..70a59b135e85e 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js @@ -8,16 +8,12 @@ */ import typeof ReactTestRenderer from 'react-test-renderer'; -import type { - CopyInspectedElementPath, - GetInspectedElementPath, - StoreAsGlobal, -} from 'react-devtools-shared/src/devtools/views/Components/InspectedElementContext'; +import {withErrorsOrWarningsIgnored} from 'react-devtools-shared/src/__tests__/utils'; + import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; import type Store from 'react-devtools-shared/src/devtools/store'; -import {withErrorsOrWarningsIgnored} from 'react-devtools-shared/src/__tests__/utils'; -describe('InspectedElementContext', () => { +describe('InspectedElement', () => { let React; let ReactDOM; let PropTypes; @@ -81,14 +77,26 @@ describe('InspectedElementContext', () => { - - {children} - + + + {children} + + ); + function useInspectedElement(id: number) { + const {inspectedElement} = React.useContext(InspectedElementContext); + return inspectedElement; + } + + function useInspectElementPath(id: number) { + const {inspectPaths} = React.useContext(InspectedElementContext); + return inspectPaths; + } + it('should inspect the currently selected element', async done => { const Example = () => { const [count] = React.useState(1); @@ -105,9 +113,29 @@ describe('InspectedElementContext', () => { let didFinish = false; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - const inspectedElement = getInspectedElement(id); - expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`); + const inspectedElement = useInspectedElement(id); + expect(inspectedElement).toMatchInlineSnapshot(` + Object { + "context": null, + "events": undefined, + "hooks": Array [ + Object { + "id": 0, + "isStateEditable": true, + "name": "State", + "subHooks": Array [], + "value": 1, + }, + ], + "id": 2, + "owners": null, + "props": Object { + "a": 1, + "b": "abc", + }, + "state": null, + } + `); didFinish = true; return null; } @@ -211,8 +239,7 @@ describe('InspectedElementContext', () => { ]; function Suspender({target, shouldHaveLegacyContext}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - const inspectedElement = getInspectedElement(target); + const inspectedElement = useInspectedElement(target); expect(inspectedElement.context).not.toBe(null); expect(inspectedElement.hasLegacyContext).toBe(shouldHaveLegacyContext); @@ -257,8 +284,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(id); + inspectedElement = useInspectedElement(id); return null; } @@ -273,14 +299,28 @@ describe('InspectedElementContext', () => { , ); }, false); - expect(inspectedElement).toMatchSnapshot('1: initial render'); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "a": 1, + "b": "abc", + } + `); await utils.actAsync( () => ReactDOM.render(, container), false, ); - inspectedElement = null; + // TODO (cache) + // This test only passes if both the check-for-updates poll AND the test renderer.update() call are included below. + // It seems like either one of the two should be sufficient but: + // 1. Running only check-for-updates schedules a transition that React never renders. + // 2. Running only renderer.update() loads stale data (first props) + + // Wait for our check-for-updates poll to get the new data. + jest.runOnlyPendingTimers(); + await Promise.resolve(); + await utils.actAsync( () => renderer.update( @@ -294,7 +334,12 @@ describe('InspectedElementContext', () => { ), false, ); - expect(inspectedElement).toMatchSnapshot('2: updated state'); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "a": 2, + "b": "def", + } + `); done(); }); @@ -305,6 +350,7 @@ describe('InspectedElementContext', () => { const Wrapper = ({children}) => children; const Target = React.memo(props => { targetRenderCount++; + // Even though his hook isn't referenced, it's used to observe backend rendering. React.useState(0); return null; }); @@ -324,8 +370,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(target); + inspectedElement = useInspectedElement(target); return null; } @@ -346,7 +391,12 @@ describe('InspectedElementContext', () => { false, ); expect(targetRenderCount).toBe(1); - expect(inspectedElement).toMatchSnapshot('1: initial render'); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "a": 1, + "b": "abc", + } + `); const initialInspectedElement = inspectedElement; @@ -369,6 +419,7 @@ describe('InspectedElementContext', () => { expect(inspectedElement).toEqual(initialInspectedElement); targetRenderCount = 0; + inspectedElement = null; await utils.actAsync( () => @@ -382,8 +433,26 @@ describe('InspectedElementContext', () => { ); // Target should have been rendered once (by ReactDOM) and once by DevTools for inspection. + await utils.actAsync( + () => + renderer.update( + + + + + , + ), + false, + ); expect(targetRenderCount).toBe(2); - expect(inspectedElement).toMatchSnapshot('2: updated state'); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "a": 2, + "b": "def", + } + `); done(); }); @@ -426,8 +495,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(target); + inspectedElement = useInspectedElement(target); return null; } @@ -482,8 +550,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(id); + inspectedElement = useInspectedElement(id); return null; } @@ -501,9 +568,6 @@ describe('InspectedElementContext', () => { false, ); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('1: Initial inspection'); - const {props} = (inspectedElement: any); expect(props.boolean_false).toBe(false); expect(props.boolean_true).toBe(true); @@ -606,8 +670,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(id); + inspectedElement = useInspectedElement(id); return null; } @@ -625,9 +688,6 @@ describe('InspectedElementContext', () => { false, ); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`); - const { anonymous_fn, array_buffer, @@ -820,8 +880,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(id); + inspectedElement = useInspectedElement(id); return null; } @@ -839,9 +898,6 @@ describe('InspectedElementContext', () => { false, ); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`); - const {prop} = (inspectedElement: any).props; expect(prop[meta.inspectable]).toBe(false); expect(prop[meta.name]).toBe('Generator'); @@ -870,8 +926,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(id); + inspectedElement = useInspectedElement(id); return null; } @@ -889,13 +944,15 @@ describe('InspectedElementContext', () => { false, ); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`); - expect(inspectedElement.props.object).toEqual({ - boolean: true, - number: 123, - string: 'abc', - }); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "object": Object { + "boolean": true, + "number": 123, + "string": "abc", + }, + } + `); done(); }); @@ -918,8 +975,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(id); + inspectedElement = useInspectedElement(id); return null; } @@ -937,12 +993,10 @@ describe('InspectedElementContext', () => { false, ); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`); - expect(inspectedElement.props.object).toEqual({ - name: 'blah', - hasOwnProperty: true, - }); + // TRICKY: Don't use toMatchInlineSnapshot() for this test! + // Our snapshot serializer relies on hasOwnProperty() for feature detection. + expect(inspectedElement.props.object.name).toBe('blah'); + expect(inspectedElement.props.object.hasOwnProperty).toBe(true); done(); }); @@ -977,9 +1031,15 @@ describe('InspectedElementContext', () => { let didFinish = false; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - const inspectedElement = getInspectedElement(id); - expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`); + const inspectedElement = useInspectedElement(id); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "data": Object { + "_number": 42, + "number": 42, + }, + } + `); didFinish = true; return null; } @@ -1075,8 +1135,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(id); + inspectedElement = useInspectedElement(id); return null; } @@ -1094,15 +1153,17 @@ describe('InspectedElementContext', () => { false, ); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`); - expect(inspectedElement.props.object).toEqual({ - 123: 3, - 'Symbol(enumerableSymbol)': 3, - 'Symbol(enumerableSymbolBase)': 1, - enumerableString: 2, - enumerableStringBase: 1, - }); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "object": Object { + "123": 3, + "Symbol(enumerableSymbol)": 3, + "Symbol(enumerableSymbolBase)": 1, + "enumerableString": 2, + "enumerableStringBase": 1, + }, + } + `); done(); }); @@ -1153,8 +1214,7 @@ describe('InspectedElementContext', () => { let inspectedElement = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - inspectedElement = getInspectedElement(id); + inspectedElement = useInspectedElement(target); return null; } @@ -1171,14 +1231,16 @@ describe('InspectedElementContext', () => { ), false, ); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`); - expect(inspectedElement.props.data).toEqual({ - a: undefined, - b: Infinity, - c: NaN, - d: 'normal', - }); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "data": Object { + "a": undefined, + "b": Infinity, + "c": NaN, + "d": "normal", + }, + } + `); done(); }); @@ -1220,96 +1282,155 @@ describe('InspectedElementContext', () => { const id = ((store.getElementIDAtIndex(0): any): number); - let getInspectedElementPath: GetInspectedElementPath = ((null: any): GetInspectedElementPath); let inspectedElement = null; + let inspectElementPath = null; - function Suspender({target}) { - const context = React.useContext(InspectedElementContext); - getInspectedElementPath = context.getInspectedElementPath; - inspectedElement = context.getInspectedElement(target); + function Suspender({path, target}) { + inspectedElement = useInspectedElement(id); + inspectElementPath = useInspectElementPath(id); return null; } - await utils.actAsync( - () => - TestRenderer.create( - - - - - , - ), - false, - ); - expect(getInspectedElementPath).not.toBeNull(); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('1: Initially inspect element'); + const renderer = TestRenderer.create(null); - inspectedElement = null; - TestUtilsAct(() => { - TestRendererAct(() => { - getInspectedElementPath(id, ['props', 'nestedObject', 'a']); - jest.runOnlyPendingTimers(); - }); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('2: Inspect props.nestedObject.a'); + async function getInspectedElement() { + await utils.actAsync( + () => + renderer.update( + + + + + , + ), + false, + ); + } - inspectedElement = null; - TestUtilsAct(() => { - TestRendererAct(() => { - getInspectedElementPath(id, ['props', 'nestedObject', 'a', 'b', 'c']); - jest.runOnlyPendingTimers(); - }); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot( - '3: Inspect props.nestedObject.a.b.c', - ); + // Render once to get a handle on inspectElementPath() + await getInspectedElement(); - inspectedElement = null; - TestUtilsAct(() => { - TestRendererAct(() => { - getInspectedElementPath(id, [ - 'props', - 'nestedObject', - 'a', - 'b', - 'c', - 0, - 'd', - ]); - jest.runOnlyPendingTimers(); + async function loadPath(path) { + TestUtilsAct(() => { + TestRendererAct(() => { + inspectElementPath(path); + jest.runOnlyPendingTimers(); + }); }); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot( - '4: Inspect props.nestedObject.a.b.c.0.d', - ); + await getInspectedElement(); + } - inspectedElement = null; - TestUtilsAct(() => { - TestRendererAct(() => { - getInspectedElementPath(id, ['hooks', 0, 'value']); - jest.runOnlyPendingTimers(); - }); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('5: Inspect hooks.0.value'); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Dehydrated { + "preview_short": {…}, + "preview_long": {b: {…}}, + }, + }, + } + `); + + await loadPath(['props', 'nestedObject', 'a']); + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Object { + "b": Object { + "c": Dehydrated { + "preview_short": Array(1), + "preview_long": [{…}], + }, + }, + }, + }, + } + `); + + await loadPath(['props', 'nestedObject', 'a', 'b', 'c']); + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Object { + "b": Object { + "c": Array [ + Object { + "d": Dehydrated { + "preview_short": {…}, + "preview_long": {e: {…}}, + }, + }, + ], + }, + }, + }, + } + `); + + await loadPath(['props', 'nestedObject', 'a', 'b', 'c', 0, 'd']); + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Object { + "b": Object { + "c": Array [ + Object { + "d": Object { + "e": Object {}, + }, + }, + ], + }, + }, + }, + } + `); - inspectedElement = null; - TestUtilsAct(() => { - TestRendererAct(() => { - getInspectedElementPath(id, ['hooks', 0, 'value', 'foo', 'bar']); - jest.runOnlyPendingTimers(); - }); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot( - '6: Inspect hooks.0.value.foo.bar', - ); + await loadPath(['hooks', 0, 'value']); + + expect(inspectedElement.hooks).toMatchInlineSnapshot(` + Array [ + Object { + "id": 0, + "isStateEditable": true, + "name": "State", + "subHooks": Array [], + "value": Object { + "foo": Object { + "bar": Dehydrated { + "preview_short": {…}, + "preview_long": {baz: "hi"}, + }, + }, + }, + }, + ] + `); + + await loadPath(['hooks', 0, 'value', 'foo', 'bar']); + + expect(inspectedElement.hooks).toMatchInlineSnapshot(` + Array [ + Object { + "id": 0, + "isStateEditable": true, + "name": "State", + "subHooks": Array [], + "value": Object { + "foo": Object { + "bar": Object { + "baz": "hi", + }, + }, + }, + }, + ] + `); done(); }); @@ -1329,42 +1450,79 @@ describe('InspectedElementContext', () => { const id = ((store.getElementIDAtIndex(0): any): number); - let getInspectedElementPath: GetInspectedElementPath = ((null: any): GetInspectedElementPath); let inspectedElement = null; + let inspectElementPath = null; - function Suspender({target}) { - const context = React.useContext(InspectedElementContext); - getInspectedElementPath = context.getInspectedElementPath; - inspectedElement = context.getInspectedElement(target); + function Suspender({path, target}) { + inspectedElement = useInspectedElement(id); + inspectElementPath = useInspectElementPath(id); return null; } - await utils.actAsync( - () => - TestRenderer.create( - - - - - , - ), - false, - ); - expect(getInspectedElementPath).not.toBeNull(); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('1: Initially inspect element'); + const renderer = TestRenderer.create(null); - inspectedElement = null; - TestUtilsAct(() => { - TestRendererAct(() => { - getInspectedElementPath(id, ['props', 'set_of_sets', 0]); - jest.runOnlyPendingTimers(); + async function getInspectedElement() { + await utils.actAsync( + () => + renderer.update( + + + + + , + ), + false, + ); + } + + // Render once to get a handle on inspectElementPath() + await getInspectedElement(); + + async function loadPath(path) { + TestUtilsAct(() => { + TestRendererAct(() => { + inspectElementPath(path); + jest.runOnlyPendingTimers(); + }); }); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('2: Inspect props.set_of_sets.0'); + await getInspectedElement(); + } + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "set_of_sets": Object { + "0": Dehydrated { + "preview_short": Set(3), + "preview_long": Set(3) {1, 2, 3}, + }, + "1": Dehydrated { + "preview_short": Set(3), + "preview_long": Set(3) {"a", "b", "c"}, + }, + }, + } + `); + + await loadPath(['props', 'set_of_sets', 0]); + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "set_of_sets": Object { + "0": Object { + "0": 1, + "1": 2, + "2": 3, + }, + "1": Object { + "0": "a", + "1": "b", + "2": "c", + }, + }, + } + `); done(); }); @@ -1400,48 +1558,107 @@ describe('InspectedElementContext', () => { const id = ((store.getElementIDAtIndex(0): any): number); - let getInspectedElementPath: GetInspectedElementPath = ((null: any): GetInspectedElementPath); let inspectedElement = null; + let inspectElementPath = null; - function Suspender({target}) { - const context = React.useContext(InspectedElementContext); - getInspectedElementPath = context.getInspectedElementPath; - inspectedElement = context.getInspectedElement(id); + function Suspender({path, target}) { + inspectedElement = useInspectedElement(id); + inspectElementPath = useInspectElementPath(id); return null; } - await utils.actAsync( - () => - TestRenderer.create( - - - - - , - ), - false, - ); - expect(getInspectedElementPath).not.toBeNull(); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('1: Initially inspect element'); + const renderer = TestRenderer.create(null); - inspectedElement = null; - TestRendererAct(() => { - getInspectedElementPath(id, ['props', 'nestedObject', 'a']); - jest.runOnlyPendingTimers(); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('2: Inspect props.nestedObject.a'); + async function getInspectedElement() { + await utils.actAsync( + () => + renderer.update( + + + + + , + ), + false, + ); + } - inspectedElement = null; - TestRendererAct(() => { - getInspectedElementPath(id, ['props', 'nestedObject', 'c']); - jest.runOnlyPendingTimers(); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('3: Inspect props.nestedObject.c'); + // Render once to get a handle on inspectElementPath() + await getInspectedElement(); + + async function loadPath(path) { + TestUtilsAct(() => { + TestRendererAct(() => { + inspectElementPath(path); + jest.runOnlyPendingTimers(); + }); + }); + await getInspectedElement(); + } + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Dehydrated { + "preview_short": {…}, + "preview_long": {b: {…}, value: 1}, + }, + "c": Dehydrated { + "preview_short": {…}, + "preview_long": {d: {…}, value: 1}, + }, + }, + } + `); + + await loadPath(['props', 'nestedObject', 'a']); + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Object { + "b": Object { + "value": 1, + }, + "value": 1, + }, + "c": Object { + "d": Dehydrated { + "preview_short": {…}, + "preview_long": {e: {…}, value: 1}, + }, + "value": 1, + }, + }, + } + `); + + await loadPath(['props', 'nestedObject', 'c']); + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Object { + "b": Object { + "value": 1, + }, + "value": 1, + }, + "c": Object { + "d": Object { + "e": Dehydrated { + "preview_short": {…}, + "preview_long": {value: 1}, + }, + "value": 1, + }, + "value": 1, + }, + }, + } + `); TestRendererAct(() => { TestUtilsAct(() => { @@ -1470,12 +1687,33 @@ describe('InspectedElementContext', () => { }); }); - TestRendererAct(() => { - inspectedElement = null; - jest.advanceTimersByTime(1000); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('4: update inspected element'); + // Wait for pending poll-for-update and then update inspected element data. + jest.runOnlyPendingTimers(); + await Promise.resolve(); + await getInspectedElement(); + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Object { + "b": Object { + "value": 2, + }, + "value": 2, + }, + "c": Object { + "d": Object { + "e": Dehydrated { + "preview_short": {…}, + "preview_long": {value: 2}, + }, + "value": 2, + }, + "value": 2, + }, + }, + } + `); done(); }); @@ -1503,32 +1741,57 @@ describe('InspectedElementContext', () => { const id = ((store.getElementIDAtIndex(0): any): number); - let getInspectedElementPath: GetInspectedElementPath = ((null: any): GetInspectedElementPath); let inspectedElement = null; + let inspectElementPath = null; - function Suspender({target}) { - const context = React.useContext(InspectedElementContext); - getInspectedElementPath = context.getInspectedElementPath; - inspectedElement = context.getInspectedElement(id); + function Suspender({path, target}) { + inspectedElement = useInspectedElement(id); + inspectElementPath = useInspectElementPath(id); return null; } - await utils.actAsync( - () => - TestRenderer.create( - - - - - , - ), - false, - ); - expect(getInspectedElementPath).not.toBeNull(); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('1: Initially inspect element'); + const renderer = TestRenderer.create(null); + + async function getInspectedElement() { + await utils.actAsync( + () => + renderer.update( + + + + + , + ), + false, + ); + } + + // Render once to get a handle on inspectElementPath() + await getInspectedElement(); + + async function loadPath(path) { + TestUtilsAct(() => { + TestRendererAct(() => { + inspectElementPath(path); + jest.runOnlyPendingTimers(); + }); + }); + await getInspectedElement(); + } + + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Dehydrated { + "preview_short": {…}, + "preview_long": {b: {…}, value: 1}, + }, + "value": 1, + }, + } + `); TestUtilsAct(() => { ReactDOM.render( @@ -1547,16 +1810,21 @@ describe('InspectedElementContext', () => { ); }); - inspectedElement = null; + await loadPath(['props', 'nestedObject', 'a']); - TestRendererAct(() => { - TestUtilsAct(() => { - getInspectedElementPath(id, ['props', 'nestedObject', 'a']); - jest.runOnlyPendingTimers(); - }); - }); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('2: Inspect props.nestedObject.a'); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Object { + "b": Object { + "value": 2, + }, + "value": 2, + }, + "value": 2, + }, + } + `); done(); }); @@ -1578,9 +1846,29 @@ describe('InspectedElementContext', () => { let didFinish = false; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - const inspectedElement = getInspectedElement(id); - expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`); + const inspectedElement = useInspectedElement(id); + expect(inspectedElement).toMatchInlineSnapshot(` + Object { + "context": null, + "events": undefined, + "hooks": Array [ + Object { + "id": null, + "isStateEditable": false, + "name": "Context", + "subHooks": Array [], + "value": true, + }, + ], + "id": 2, + "owners": null, + "props": Object { + "a": 1, + "b": "abc", + }, + "state": null, + } + `); didFinish = true; return null; } @@ -1630,8 +1918,20 @@ describe('InspectedElementContext', () => { let storeAsGlobal: StoreAsGlobal = ((null: any): StoreAsGlobal); function Suspender({target}) { - const context = React.useContext(InspectedElementContext); - storeAsGlobal = context.storeAsGlobal; + storeAsGlobal = (elementID: number, path: Array) => { + const rendererID = store.getRendererIDForElement(elementID); + if (rendererID !== null) { + const { + storeAsGlobal: storeAsGlobalAPI, + } = require('react-devtools-shared/src/backendAPI'); + storeAsGlobalAPI({ + bridge, + id: elementID, + path, + rendererID, + }); + } + }; return null; } @@ -1648,23 +1948,22 @@ describe('InspectedElementContext', () => { ), false, ); - expect(storeAsGlobal).not.toBeNull(); jest.spyOn(console, 'log').mockImplementation(() => {}); // Should store the whole value (not just the hydrated parts) storeAsGlobal(id, ['props', 'nestedObject']); jest.runOnlyPendingTimers(); - expect(console.log).toHaveBeenCalledWith('$reactTemp1'); - expect(global.$reactTemp1).toBe(nestedObject); + expect(console.log).toHaveBeenCalledWith('$reactTemp0'); + expect(global.$reactTemp0).toBe(nestedObject); console.log.mockReset(); // Should store the nested property specified (not just the outer value) storeAsGlobal(id, ['props', 'nestedObject', 'a', 'b']); jest.runOnlyPendingTimers(); - expect(console.log).toHaveBeenCalledWith('$reactTemp2'); - expect(global.$reactTemp2).toBe(nestedObject.a.b); + expect(console.log).toHaveBeenCalledWith('$reactTemp1'); + expect(global.$reactTemp1).toBe(nestedObject.a.b); done(); }); @@ -1696,8 +1995,20 @@ describe('InspectedElementContext', () => { let copyPath: CopyInspectedElementPath = ((null: any): CopyInspectedElementPath); function Suspender({target}) { - const context = React.useContext(InspectedElementContext); - copyPath = context.copyInspectedElementPath; + copyPath = (elementID: number, path: Array) => { + const rendererID = store.getRendererIDForElement(elementID); + if (rendererID !== null) { + const { + copyInspectedElementPath, + } = require('react-devtools-shared/src/backendAPI'); + copyInspectedElementPath({ + bridge, + id: elementID, + path, + rendererID, + }); + } + }; return null; } @@ -1788,8 +2099,20 @@ describe('InspectedElementContext', () => { let copyPath: CopyInspectedElementPath = ((null: any): CopyInspectedElementPath); function Suspender({target}) { - const context = React.useContext(InspectedElementContext); - copyPath = context.copyInspectedElementPath; + copyPath = (elementID: number, path: Array) => { + const rendererID = store.getRendererIDForElement(elementID); + if (rendererID !== null) { + const { + copyInspectedElementPath, + } = require('react-devtools-shared/src/backendAPI'); + copyInspectedElementPath({ + bridge, + id: elementID, + path, + rendererID, + }); + } + }; return null; } @@ -1837,12 +2160,9 @@ describe('InspectedElementContext', () => { }); it('should display complex values of useDebugValue', async done => { - let getInspectedElementPath: GetInspectedElementPath = ((null: any): GetInspectedElementPath); let inspectedElement = null; function Suspender({target}) { - const context = React.useContext(InspectedElementContext); - getInspectedElementPath = context.getInspectedElementPath; - inspectedElement = context.getInspectedElement(target); + inspectedElement = useInspectedElement(target); return null; } @@ -1879,9 +2199,27 @@ describe('InspectedElementContext', () => { ), false, ); - expect(getInspectedElementPath).not.toBeNull(); - expect(inspectedElement).not.toBeNull(); - expect(inspectedElement).toMatchSnapshot('DisplayedComplexValue'); + expect(inspectedElement.hooks).toMatchInlineSnapshot(` + Array [ + Object { + "id": null, + "isStateEditable": false, + "name": "DebuggableHook", + "subHooks": Array [ + Object { + "id": 0, + "isStateEditable": true, + "name": "State", + "subHooks": Array [], + "value": 1, + }, + ], + "value": Object { + "foo": 2, + }, + }, + ] + `); done(); }); @@ -1901,8 +2239,7 @@ describe('InspectedElementContext', () => { let warnings = null; function Suspender({target}) { - const {getInspectedElement} = React.useContext(InspectedElementContext); - const inspectedElement = getInspectedElement(id); + const inspectedElement = useInspectedElement(id); errors = inspectedElement.errors; warnings = inspectedElement.warnings; return null; @@ -2114,7 +2451,11 @@ describe('InspectedElementContext', () => { ); }); - store.clearErrorsAndWarnings(); + const { + clearErrorsAndWarnings, + } = require('react-devtools-shared/src/backendAPI'); + clearErrorsAndWarnings({bridge, store}); + // Flush events to the renderer. jest.runOnlyPendingTimers(); @@ -2127,7 +2468,7 @@ describe('InspectedElementContext', () => { `); }); - it('can be cleared for a particular Fiber (only errors)', async () => { + it('can be cleared for a particular Fiber (only warnings)', async () => { const Example = ({id}) => { console.error(`test-only: render error #${id}`); console.warn(`test-only: render warning #${id}`); @@ -2147,7 +2488,14 @@ describe('InspectedElementContext', () => { ); }); - store.clearWarningsForElement(2); + let id = ((store.getElementIDAtIndex(1): any): number); + const rendererID = store.getRendererIDForElement(id); + + const { + clearWarningsForElement, + } = require('react-devtools-shared/src/backendAPI'); + clearWarningsForElement({bridge, id, rendererID}); + // Flush events to the renderer. jest.runOnlyPendingTimers(); @@ -2183,7 +2531,9 @@ describe('InspectedElementContext', () => { ] `); - store.clearWarningsForElement(1); + id = ((store.getElementIDAtIndex(0): any): number); + clearWarningsForElement({bridge, id, rendererID}); + // Flush events to the renderer. jest.runOnlyPendingTimers(); @@ -2215,7 +2565,7 @@ describe('InspectedElementContext', () => { `); }); - it('can be cleared for a particular Fiber (only warnings)', async () => { + it('can be cleared for a particular Fiber (only errors)', async () => { const Example = ({id}) => { console.error(`test-only: render error #${id}`); console.warn(`test-only: render warning #${id}`); @@ -2235,7 +2585,14 @@ describe('InspectedElementContext', () => { ); }); - store.clearErrorsForElement(2); + let id = ((store.getElementIDAtIndex(1): any): number); + const rendererID = store.getRendererIDForElement(id); + + const { + clearErrorsForElement, + } = require('react-devtools-shared/src/backendAPI'); + clearErrorsForElement({bridge, id, rendererID}); + // Flush events to the renderer. jest.runOnlyPendingTimers(); @@ -2271,7 +2628,9 @@ describe('InspectedElementContext', () => { ] `); - store.clearErrorsForElement(1); + id = ((store.getElementIDAtIndex(0): any): number); + clearErrorsForElement({bridge, id, rendererID}); + // Flush events to the renderer. jest.runOnlyPendingTimers(); diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElementSerializer.js b/packages/react-devtools-shared/src/__tests__/inspectedElementSerializer.js index b6290daf61a95..a615777421fe3 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElementSerializer.js +++ b/packages/react-devtools-shared/src/__tests__/inspectedElementSerializer.js @@ -12,17 +12,14 @@ export function test(maybeInspectedElement) { // print() is part of Jest's serializer API export function print(inspectedElement, serialize, indent) { - return JSON.stringify( - { - id: inspectedElement.id, - owners: inspectedElement.owners, - context: inspectedElement.context, - events: inspectedElement.events, - hooks: inspectedElement.hooks, - props: inspectedElement.props, - state: inspectedElement.state, - }, - null, - 2, - ); + // Don't stringify this object; that would break nested serializers. + return serialize({ + context: inspectedElement.context, + events: inspectedElement.events, + hooks: inspectedElement.hooks, + id: inspectedElement.id, + owners: inspectedElement.owners, + props: inspectedElement.props, + state: inspectedElement.state, + }); } diff --git a/packages/react-devtools-shared/src/__tests__/legacy/__snapshots__/inspectElement-test.js.snap b/packages/react-devtools-shared/src/__tests__/legacy/__snapshots__/inspectElement-test.js.snap deleted file mode 100644 index bea8302e937f4..0000000000000 --- a/packages/react-devtools-shared/src/__tests__/legacy/__snapshots__/inspectElement-test.js.snap +++ /dev/null @@ -1,324 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`InspectedElementContext should allow component prop value and value\`s prototype has same name params.: 1: Initially inspect element 1`] = ` -Object { - "id": 2, - "type": "full-data", - "value": { - "id": 2, - "owners": null, - "context": {}, - "hooks": null, - "props": { - "data": { - "b": null, - "c": null, - "d": "normal" - } - }, - "state": null -}, -} -`; - -exports[`InspectedElementContext should inspect the currently selected element: 1: Initial inspection 1`] = ` -Object { - "id": 2, - "type": "full-data", - "value": { - "id": 2, - "owners": null, - "context": {}, - "hooks": null, - "props": { - "a": 1, - "b": "abc" - }, - "state": null -}, -} -`; - -exports[`InspectedElementContext should not consume iterables while inspecting: 1: Initial inspection 1`] = ` -Object { - "id": 2, - "type": "full-data", - "value": { - "id": 2, - "owners": null, - "context": {}, - "hooks": null, - "props": { - "iteratable": {} - }, - "state": null -}, -} -`; - -exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 1: Initially inspect element 1`] = ` -Object { - "id": 2, - "type": "full-data", - "value": { - "id": 2, - "owners": null, - "context": {}, - "hooks": null, - "props": { - "nestedObject": { - "a": {} - } - }, - "state": null -}, -} -`; - -exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 2: Inspect props.nestedObject.a 1`] = ` -Object { - "id": 2, - "type": "full-data", - "value": { - "id": 2, - "owners": null, - "context": {}, - "hooks": null, - "props": { - "nestedObject": { - "a": { - "b": { - "c": {} - } - } - } - }, - "state": null -}, -} -`; - -exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 3: Inspect props.nestedObject.a.b.c 1`] = ` -Object { - "id": 2, - "type": "full-data", - "value": { - "id": 2, - "owners": null, - "context": {}, - "hooks": null, - "props": { - "nestedObject": { - "a": { - "b": { - "c": [ - { - "d": {} - } - ] - } - } - } - }, - "state": null -}, -} -`; - -exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 4: Inspect props.nestedObject.a.b.c.0.d 1`] = ` -Object { - "id": 2, - "type": "full-data", - "value": { - "id": 2, - "owners": null, - "context": {}, - "hooks": null, - "props": { - "nestedObject": { - "a": { - "b": { - "c": [ - { - "d": { - "e": {} - } - } - ] - } - } - } - }, - "state": null -}, -} -`; - -exports[`InspectedElementContext should support complex data types: 1: Initial inspection 1`] = ` -Object { - "id": 2, - "type": "full-data", - "value": { - "id": 2, - "owners": null, - "context": {}, - "hooks": null, - "props": { - "anonymous_fn": {}, - "array_buffer": {}, - "array_of_arrays": [ - {} - ], - "big_int": {}, - "bound_fn": {}, - "data_view": {}, - "date": {}, - "fn": {}, - "html_element": {}, - "immutable": { - "0": {}, - "1": {}, - "2": {} - }, - "map": { - "0": {}, - "1": {} - }, - "map_of_maps": { - "0": {}, - "1": {} - }, - "object_of_objects": { - "inner": {} - }, - "react_element": {}, - "regexp": {}, - "set": { - "0": "abc", - "1": 123 - }, - "set_of_sets": { - "0": {}, - "1": {} - }, - "symbol": {}, - "typed_array": { - "0": 100, - "1": -100, - "2": 0 - } - }, - "state": null -}, -} -`; - -exports[`InspectedElementContext should support custom objects with enumerable properties and getters: 1: Initial inspection 1`] = ` -Object { - "id": 2, - "type": "full-data", - "value": { - "id": 2, - "owners": null, - "context": {}, - "hooks": null, - "props": { - "data": { - "_number": 42, - "number": 42 - } - }, - "state": null -}, -} -`; - -exports[`InspectedElementContext should support objects with no prototype: 1: Initial inspection 1`] = ` -Object { - "id": 2, - "type": "full-data", - "value": { - "id": 2, - "owners": null, - "context": {}, - "hooks": null, - "props": { - "object": { - "string": "abc", - "number": 123, - "boolean": true - } - }, - "state": null -}, -} -`; - -exports[`InspectedElementContext should support objects with overridden hasOwnProperty: 1: Initial inspection 1`] = ` -Object { - "id": 2, - "type": "full-data", - "value": { - "id": 2, - "owners": null, - "context": {}, - "hooks": null, - "props": { - "object": { - "name": "blah", - "hasOwnProperty": true - } - }, - "state": null -}, -} -`; - -exports[`InspectedElementContext should support objects with with inherited keys: 1: Initial inspection 1`] = ` -Object { - "id": 2, - "type": "full-data", - "value": { - "id": 2, - "owners": null, - "context": {}, - "hooks": null, - "props": { - "data": { - "123": 3, - "enumerableString": 2, - "Symbol(enumerableSymbol)": 3, - "enumerableStringBase": 1, - "Symbol(enumerableSymbolBase)": 1 - } - }, - "state": null -}, -} -`; - -exports[`InspectedElementContext should support simple data types: 1: Initial inspection 1`] = ` -Object { - "id": 2, - "type": "full-data", - "value": { - "id": 2, - "owners": null, - "context": {}, - "hooks": null, - "props": { - "boolean_false": false, - "boolean_true": true, - "infinity": null, - "integer_zero": 0, - "integer_one": 1, - "float": 1.23, - "string": "abc", - "string_empty": "", - "nan": null, - "value_null": null - }, - "state": null -}, -} -`; diff --git a/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js b/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js index b4b883560d00c..6d446ca481de9 100644 --- a/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js +++ b/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js @@ -7,71 +7,50 @@ * @flow */ -import type {InspectedElementPayload} from 'react-devtools-shared/src/backend/types'; -import type {DehydratedData} from 'react-devtools-shared/src/devtools/views/Components/types'; import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; import type Store from 'react-devtools-shared/src/devtools/store'; describe('InspectedElementContext', () => { let React; let ReactDOM; - let hydrate; - let meta; let bridge: FrontendBridge; let store: Store; + let backendAPI; + const act = (callback: Function) => { callback(); jest.runAllTimers(); // Flush Bridge operations }; - function dehydrateHelper( - dehydratedData: DehydratedData | null, - ): Object | null { - if (dehydratedData !== null) { - return hydrate( - dehydratedData.data, - dehydratedData.cleaned, - dehydratedData.unserializable, - ); - } else { - return null; - } - } - async function read( id: number, - path?: Array, + inspectedPaths?: Object = {}, ): Promise { - return new Promise((resolve, reject) => { - const rendererID = ((store.getRendererIDForElement(id): any): number); - - const onInspectedElement = (payload: InspectedElementPayload) => { - bridge.removeListener('inspectedElement', onInspectedElement); - - if (payload.type === 'full-data' && payload.value !== null) { - payload.value.context = dehydrateHelper(payload.value.context); - payload.value.props = dehydrateHelper(payload.value.props); - payload.value.state = dehydrateHelper(payload.value.state); - } - - resolve(payload); - }; + const rendererID = ((store.getRendererIDForElement(id): any): number); + const promise = backendAPI + .inspectElement({ + bridge, + forceUpdate: true, + id, + inspectedPaths, + rendererID, + }) + .then(data => + backendAPI.convertInspectedElementBackendToFrontend(data.value), + ); - bridge.addListener('inspectedElement', onInspectedElement); - bridge.send('inspectElement', {id, path, rendererID}); + jest.runOnlyPendingTimers(); - jest.runOnlyPendingTimers(); - }); + return promise; } beforeEach(() => { bridge = global.bridge; store = global.store; - hydrate = require('react-devtools-shared/src/hydration').hydrate; - meta = require('react-devtools-shared/src/hydration').meta; + backendAPI = require('react-devtools-shared/src/backendAPI'); // Redirect all React/ReactDOM requires to the v15 UMD. // We use the UMD because Jest doesn't enable us to mock deep imports (e.g. "react/lib/Something"). @@ -94,7 +73,20 @@ describe('InspectedElementContext', () => { const id = ((store.getElementIDAtIndex(0): any): number); const inspectedElement = await read(id); - expect(inspectedElement).toMatchSnapshot('1: Initial inspection'); + expect(inspectedElement).toMatchInlineSnapshot(` + Object { + "context": Object {}, + "events": undefined, + "hooks": null, + "id": 2, + "owners": null, + "props": Object { + "a": 1, + "b": "abc", + }, + "state": null, + } + `); done(); }); @@ -124,20 +116,29 @@ describe('InspectedElementContext', () => { const id = ((store.getElementIDAtIndex(0): any): number); const inspectedElement = await read(id); - expect(inspectedElement).toMatchSnapshot('1: Initial inspection'); - - const {props} = inspectedElement.value; - expect(props.boolean_false).toBe(false); - expect(props.boolean_true).toBe(true); - expect(Number.isFinite(props.infinity)).toBe(false); - expect(props.integer_zero).toEqual(0); - expect(props.integer_one).toEqual(1); - expect(props.float).toEqual(1.23); - expect(props.string).toEqual('abc'); - expect(props.string_empty).toEqual(''); - expect(props.nan).toBeNaN(); - expect(props.value_null).toBeNull(); - expect(props.value_undefined).toBeUndefined(); + expect(inspectedElement).toMatchInlineSnapshot(` + Object { + "context": Object {}, + "events": undefined, + "hooks": null, + "id": 2, + "owners": null, + "props": Object { + "boolean_false": false, + "boolean_true": true, + "float": 1.23, + "infinity": Infinity, + "integer_one": 1, + "integer_zero": 0, + "nan": NaN, + "string": "abc", + "string_empty": "", + "value_null": null, + "value_undefined": undefined, + }, + "state": null, + } + `); done(); }); @@ -211,8 +212,6 @@ describe('InspectedElementContext', () => { const id = ((store.getElementIDAtIndex(0): any): number); const inspectedElement = await read(id); - expect(inspectedElement).toMatchSnapshot('1: Initial inspection'); - const { anonymous_fn, array_buffer, @@ -233,7 +232,9 @@ describe('InspectedElementContext', () => { set_of_sets, symbol, typed_array, - } = inspectedElement.value.props; + } = inspectedElement.props; + + const {meta} = require('react-devtools-shared/src/hydration'); expect(anonymous_fn[meta.inspectable]).toBe(false); expect(anonymous_fn[meta.name]).toBe('function'); @@ -360,12 +361,15 @@ describe('InspectedElementContext', () => { const id = ((store.getElementIDAtIndex(0): any): number); const inspectedElement = await read(id); - expect(inspectedElement).toMatchSnapshot('1: Initial inspection'); - expect(inspectedElement.value.props.object).toEqual({ - boolean: true, - number: 123, - string: 'abc', - }); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "object": Object { + "boolean": true, + "number": 123, + "string": "abc", + }, + } + `); done(); }); @@ -388,11 +392,10 @@ describe('InspectedElementContext', () => { const id = ((store.getElementIDAtIndex(0): any): number); const inspectedElement = await read(id); - expect(inspectedElement).toMatchSnapshot('1: Initial inspection'); - expect(inspectedElement.value.props.object).toEqual({ - name: 'blah', - hasOwnProperty: true, - }); + // TRICKY: Don't use toMatchInlineSnapshot() for this test! + // Our snapshot serializer relies on hasOwnProperty() for feature detection. + expect(inspectedElement.props.object.name).toBe('blah'); + expect(inspectedElement.props.object.hasOwnProperty).toBe(true); done(); }); @@ -417,7 +420,22 @@ describe('InspectedElementContext', () => { const id = ((store.getElementIDAtIndex(0): any): number); const inspectedElement = await read(id); - expect(inspectedElement).toMatchSnapshot('1: Initial inspection'); + expect(inspectedElement).toMatchInlineSnapshot(` + Object { + "context": Object {}, + "events": undefined, + "hooks": null, + "id": 2, + "owners": null, + "props": Object { + "iteratable": Dehydrated { + "preview_short": Generator, + "preview_long": Generator, + }, + }, + "state": null, + } + `); // Inspecting should not consume the iterable. expect(iteratable.next().value).toEqual(1); @@ -457,7 +475,22 @@ describe('InspectedElementContext', () => { const id = ((store.getElementIDAtIndex(0): any): number); const inspectedElement = await read(id); - expect(inspectedElement).toMatchSnapshot('1: Initial inspection'); + expect(inspectedElement).toMatchInlineSnapshot(` + Object { + "context": Object {}, + "events": undefined, + "hooks": null, + "id": 2, + "owners": null, + "props": Object { + "data": Object { + "_number": 42, + "number": 42, + }, + }, + "state": null, + } + `); done(); }); @@ -532,7 +565,25 @@ describe('InspectedElementContext', () => { const id = ((store.getElementIDAtIndex(0): any): number); const inspectedElement = await read(id); - expect(inspectedElement).toMatchSnapshot('1: Initial inspection'); + expect(inspectedElement).toMatchInlineSnapshot(` + Object { + "context": Object {}, + "events": undefined, + "hooks": null, + "id": 2, + "owners": null, + "props": Object { + "data": Object { + "123": 3, + "Symbol(enumerableSymbol)": 3, + "Symbol(enumerableSymbolBase)": 1, + "enumerableString": 2, + "enumerableStringBase": 1, + }, + }, + "state": null, + } + `); done(); }); @@ -572,6 +623,7 @@ describe('InspectedElementContext', () => { }, }, ); + const Example = ({data}) => null; act(() => ReactDOM.render( @@ -579,14 +631,20 @@ describe('InspectedElementContext', () => { document.createElement('div'), ), ); + const id = ((store.getElementIDAtIndex(0): any): number); const inspectedElement = await read(id); - expect(inspectedElement).toMatchSnapshot('1: Initially inspect element'); - const {props} = inspectedElement.value; - expect(props.data.a).toBeUndefined(); - expect(Number.isFinite(props.data.b)).toBe(false); - expect(props.data.c).toBeNaN(); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "data": Object { + "a": undefined, + "b": Infinity, + "c": NaN, + "d": "normal", + }, + } + `); done(); }); @@ -617,28 +675,75 @@ describe('InspectedElementContext', () => { const id = ((store.getElementIDAtIndex(0): any): number); let inspectedElement = await read(id); - expect(inspectedElement).toMatchSnapshot('1: Initially inspect element'); - - inspectedElement = await read(id, ['props', 'nestedObject', 'a']); - expect(inspectedElement).toMatchSnapshot('2: Inspect props.nestedObject.a'); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Dehydrated { + "preview_short": {…}, + "preview_long": {b: {…}}, + }, + }, + } + `); + + inspectedElement = await read(id, {props: {nestedObject: {a: {}}}}); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Object { + "b": Object { + "c": Dehydrated { + "preview_short": Array(1), + "preview_long": [{…}], + }, + }, + }, + }, + } + `); - inspectedElement = await read(id, ['props', 'nestedObject', 'a', 'b', 'c']); - expect(inspectedElement).toMatchSnapshot( - '3: Inspect props.nestedObject.a.b.c', - ); + inspectedElement = await read(id, { + props: {nestedObject: {a: {b: {c: {}}}}}, + }); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Object { + "b": Object { + "c": Array [ + Object { + "d": Dehydrated { + "preview_short": {…}, + "preview_long": {e: {…}}, + }, + }, + ], + }, + }, + }, + } + `); - inspectedElement = await read(id, [ - 'props', - 'nestedObject', - 'a', - 'b', - 'c', - 0, - 'd', - ]); - expect(inspectedElement).toMatchSnapshot( - '4: Inspect props.nestedObject.a.b.c.0.d', - ); + inspectedElement = await read(id, { + props: {nestedObject: {a: {b: {c: {0: {d: {}}}}}}}, + }); + expect(inspectedElement.props).toMatchInlineSnapshot(` + Object { + "nestedObject": Object { + "a": Object { + "b": Object { + "c": Array [ + Object { + "d": Object { + "e": Object {}, + }, + }, + ], + }, + }, + }, + } + `); done(); }); @@ -672,28 +777,30 @@ describe('InspectedElementContext', () => { spyOn(console, 'log').and.callFake(logSpy); // Should store the whole value (not just the hydrated parts) - bridge.send('storeAsGlobal', { - count: 1, + backendAPI.storeAsGlobal({ + bridge, id, path: ['props', 'nestedObject'], rendererID, }); + jest.runOnlyPendingTimers(); - expect(logSpy).toHaveBeenCalledWith('$reactTemp1'); - expect(global.$reactTemp1).toBe(nestedObject); + expect(logSpy).toHaveBeenCalledWith('$reactTemp0'); + expect(global.$reactTemp0).toBe(nestedObject); logSpy.mockReset(); // Should store the nested property specified (not just the outer value) - bridge.send('storeAsGlobal', { - count: 2, + backendAPI.storeAsGlobal({ + bridge, id, path: ['props', 'nestedObject', 'a', 'b'], rendererID, }); + jest.runOnlyPendingTimers(); - expect(logSpy).toHaveBeenCalledWith('$reactTemp2'); - expect(global.$reactTemp2).toBe(nestedObject.a.b); + expect(logSpy).toHaveBeenCalledWith('$reactTemp1'); + expect(global.$reactTemp1).toBe(nestedObject.a.b); }); it('should enable inspected values to be copied to the clipboard', () => { @@ -722,11 +829,13 @@ describe('InspectedElementContext', () => { const rendererID = ((store.getRendererIDForElement(id): any): number); // Should copy the whole value (not just the hydrated parts) - bridge.send('copyElementPath', { + backendAPI.copyInspectedElementPath({ + bridge, id, path: ['props', 'nestedObject'], rendererID, }); + jest.runOnlyPendingTimers(); expect(global.mockClipboardCopy).toHaveBeenCalledTimes(1); expect(global.mockClipboardCopy).toHaveBeenCalledWith( @@ -736,11 +845,13 @@ describe('InspectedElementContext', () => { global.mockClipboardCopy.mockReset(); // Should copy the nested property specified (not just the outer value) - bridge.send('copyElementPath', { + backendAPI.copyInspectedElementPath({ + bridge, id, path: ['props', 'nestedObject', 'a', 'b'], rendererID, }); + jest.runOnlyPendingTimers(); expect(global.mockClipboardCopy).toHaveBeenCalledTimes(1); expect(global.mockClipboardCopy).toHaveBeenCalledWith( @@ -798,7 +909,8 @@ describe('InspectedElementContext', () => { const rendererID = ((store.getRendererIDForElement(id): any): number); // Should copy the whole value (not just the hydrated parts) - bridge.send('copyElementPath', { + backendAPI.copyInspectedElementPath({ + bridge, id, path: ['props'], rendererID, @@ -809,7 +921,8 @@ describe('InspectedElementContext', () => { global.mockClipboardCopy.mockReset(); // Should copy the nested property specified (not just the outer value) - bridge.send('copyElementPath', { + backendAPI.copyInspectedElementPath({ + bridge, id, path: ['props', 'bigInt'], rendererID, @@ -823,7 +936,8 @@ describe('InspectedElementContext', () => { global.mockClipboardCopy.mockReset(); // Should copy the nested property specified (not just the outer value) - bridge.send('copyElementPath', { + backendAPI.copyInspectedElementPath({ + bridge, id, path: ['props', 'typedArray'], rendererID, diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index 33d3bc0a072a3..d5b9c3cbc9213 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -12,12 +12,14 @@ describe('Store', () => { let ReactDOM; let agent; let act; + let bridge; let getRendererID; let store; let withErrorsOrWarningsIgnored; beforeEach(() => { agent = global.agent; + bridge = global.bridge; store = global.store; React = require('react'); @@ -888,6 +890,7 @@ describe('Store', () => { + ); @@ -1158,7 +1161,11 @@ describe('Store', () => { ✕⚠ `); - store.clearErrorsAndWarnings(); + const { + clearErrorsAndWarnings, + } = require('react-devtools-shared/src/backendAPI'); + clearErrorsAndWarnings({bridge, store}); + // flush events to the renderer jest.runAllTimers(); @@ -1195,7 +1202,14 @@ describe('Store', () => { ✕⚠ `); - store.clearWarningsForElement(2); + const id = ((store.getElementIDAtIndex(1): any): number); + const rendererID = store.getRendererIDForElement(id); + + const { + clearWarningsForElement, + } = require('react-devtools-shared/src/backendAPI'); + clearWarningsForElement({bridge, id, rendererID}); + // Flush events to the renderer. jest.runAllTimers(); @@ -1233,7 +1247,14 @@ describe('Store', () => { ✕⚠ `); - store.clearErrorsForElement(2); + const id = ((store.getElementIDAtIndex(1): any): number); + const rendererID = store.getRendererIDForElement(id); + + const { + clearErrorsForElement, + } = require('react-devtools-shared/src/backendAPI'); + clearErrorsForElement({bridge, id, rendererID}); + // Flush events to the renderer. jest.runAllTimers(); diff --git a/packages/react-devtools-shared/src/__tests__/treeContext-test.js b/packages/react-devtools-shared/src/__tests__/treeContext-test.js index 8c057fb720807..36f615c1155b6 100644 --- a/packages/react-devtools-shared/src/__tests__/treeContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/treeContext-test.js @@ -1441,20 +1441,28 @@ describe('TreeListContext', () => { }); describe('inline errors/warnings state', () => { + const { + clearErrorsAndWarnings: clearErrorsAndWarningsAPI, + clearErrorsForElement: clearErrorsForElementAPI, + clearWarningsForElement: clearWarningsForElementAPI, + } = require('react-devtools-shared/src/backendAPI'); + function clearAllErrors() { - utils.act(() => store.clearErrorsAndWarnings()); + utils.act(() => clearErrorsAndWarningsAPI({bridge, store})); // flush events to the renderer jest.runAllTimers(); } function clearErrorsForElement(id) { - utils.act(() => store.clearErrorsForElement(id)); + const rendererID = store.getRendererIDForElement(id); + utils.act(() => clearErrorsForElementAPI({bridge, id, rendererID})); // flush events to the renderer jest.runAllTimers(); } function clearWarningsForElement(id) { - utils.act(() => store.clearWarningsForElement(id)); + const rendererID = store.getRendererIDForElement(id); + utils.act(() => clearWarningsForElementAPI({bridge, id, rendererID})); // flush events to the renderer jest.runAllTimers(); } diff --git a/packages/react-devtools-shared/src/backend/agent.js b/packages/react-devtools-shared/src/backend/agent.js index 326b0fb2e340f..e570febf5d67e 100644 --- a/packages/react-devtools-shared/src/backend/agent.js +++ b/packages/react-devtools-shared/src/backend/agent.js @@ -70,8 +70,10 @@ type CopyElementParams = {| type InspectElementParams = {| id: number, - path?: Array, + inspectedPaths: Object, + forceUpdate: boolean, rendererID: number, + requestID: number, |}; type OverrideHookParams = {| @@ -328,12 +330,21 @@ export default class Agent extends EventEmitter<{| } }; - inspectElement = ({id, path, rendererID}: InspectElementParams) => { + inspectElement = ({ + id, + inspectedPaths, + forceUpdate, + rendererID, + requestID, + }: InspectElementParams) => { const renderer = this._rendererInterfaces[rendererID]; if (renderer == null) { console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`); } else { - this._bridge.send('inspectedElement', renderer.inspectElement(id, path)); + this._bridge.send( + 'inspectedElement', + renderer.inspectElement(requestID, id, inspectedPaths, forceUpdate), + ); // When user selects an element, stop trying to restore the selection, // and instead remember the current selection for the next reload. diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index 90f0f3ad12b46..63c9b1660e75e 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -584,25 +584,12 @@ export function attach( } let currentlyInspectedElementID: number | null = null; - let currentlyInspectedPaths: Object = {}; - - // Track the intersection of currently inspected paths, - // so that we can send their data along if the element is re-rendered. - function mergeInspectedPaths(path: Array) { - let current = currentlyInspectedPaths; - path.forEach(key => { - if (!current[key]) { - current[key] = {}; - } - current = current[key]; - }); - } - function createIsPathAllowed(key: string) { + function createIsPathAllowed(key: string, inspectedPaths: Object) { // This function helps prevent previously-inspected paths from being dehydrated in updates. // This is important to avoid a bad user experience where expanded toggles collapse on update. return function isPathAllowed(path: Array): boolean { - let current = currentlyInspectedPaths[key]; + let current = inspectedPaths[key]; if (!current) { return false; } @@ -691,26 +678,23 @@ export function attach( } function inspectElement( + requestID: number, id: number, - path?: Array, + inspectedPaths: Object, ): InspectedElementPayload { if (currentlyInspectedElementID !== id) { currentlyInspectedElementID = id; - currentlyInspectedPaths = {}; } const inspectedElement = inspectElementRaw(id); if (inspectedElement === null) { return { id, + responseID: requestID, type: 'not-found', }; } - if (path != null) { - mergeInspectedPaths(path); - } - // Any time an inspected element has an update, // we should update the selected $r value as wel. // Do this before dehyration (cleanForBridge). @@ -718,19 +702,20 @@ export function attach( inspectedElement.context = cleanForBridge( inspectedElement.context, - createIsPathAllowed('context'), + createIsPathAllowed('context', inspectedPaths), ); inspectedElement.props = cleanForBridge( inspectedElement.props, - createIsPathAllowed('props'), + createIsPathAllowed('props', inspectedPaths), ); inspectedElement.state = cleanForBridge( inspectedElement.state, - createIsPathAllowed('state'), + createIsPathAllowed('state', inspectedPaths), ); return { id, + responseID: requestID, type: 'full-data', value: inspectedElement, }; diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index e44b58f71009e..ee90dfee688ac 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -176,6 +176,7 @@ export function getInternalReactConstants( // Currently the version in Git is 17.0.2 (but that version has not been/may not end up being released). if (gt(version, '17.0.1')) { ReactTypeOfWork = { + CacheComponent: 24, // Experimental ClassComponent: 1, ContextConsumer: 9, ContextProvider: 10, @@ -205,6 +206,7 @@ export function getInternalReactConstants( }; } else if (gte(version, '17.0.0-alpha')) { ReactTypeOfWork = { + CacheComponent: -1, // Doesn't exist yet ClassComponent: 1, ContextConsumer: 9, ContextProvider: 10, @@ -234,6 +236,7 @@ export function getInternalReactConstants( }; } else if (gte(version, '16.6.0-beta.0')) { ReactTypeOfWork = { + CacheComponent: -1, // Doens't exist yet ClassComponent: 1, ContextConsumer: 9, ContextProvider: 10, @@ -263,6 +266,7 @@ export function getInternalReactConstants( }; } else if (gte(version, '16.4.3-alpha')) { ReactTypeOfWork = { + CacheComponent: -1, // Doens't exist yet ClassComponent: 2, ContextConsumer: 11, ContextProvider: 12, @@ -292,6 +296,7 @@ export function getInternalReactConstants( }; } else { ReactTypeOfWork = { + CacheComponent: -1, // Doens't exist yet ClassComponent: 2, ContextConsumer: 12, ContextProvider: 13, @@ -335,6 +340,7 @@ export function getInternalReactConstants( } const { + CacheComponent, ClassComponent, IncompleteClassComponent, FunctionComponent, @@ -382,6 +388,8 @@ export function getInternalReactConstants( let resolvedContext: any = null; switch (tag) { + case CacheComponent: + return 'Cache'; case ClassComponent: case IncompleteClassComponent: return getDisplayName(resolvedType); @@ -486,12 +494,13 @@ export function attach( } = getInternalReactConstants(renderer.version); const {Incomplete, NoFlags, PerformedWork, Placement} = ReactTypeOfSideEffect; const { - FunctionComponent, + CacheComponent, ClassComponent, ContextConsumer, DehydratedSuspenseComponent, - Fragment, ForwardRef, + Fragment, + FunctionComponent, HostRoot, HostPortal, HostComponent, @@ -2518,6 +2527,10 @@ export function attach( tag === ForwardRef) && (!!memoizedState || !!dependencies); + // TODO Show custom UI for Cache like we do for Suspense + // For now, just hide state data entirely since it's not meant to be inspected. + const showState = !usesHooks && tag !== CacheComponent; + const typeSymbol = getTypeSymbol(type); let canViewSource = false; @@ -2687,7 +2700,7 @@ export function attach( context, hooks, props: memoizedProps, - state: usesHooks ? null : memoizedState, + state: showState ? memoizedState : null, errors: Array.from(errors.entries()), warnings: Array.from(warnings.entries()), @@ -2705,7 +2718,6 @@ export function attach( let mostRecentlyInspectedElement: InspectedElement | null = null; let hasElementUpdatedSinceLastInspected: boolean = false; - let currentlyInspectedPaths: Object = {}; function isMostRecentlyInspectedElementCurrent(id: number): boolean { return ( @@ -2715,21 +2727,10 @@ export function attach( ); } - // Track the intersection of currently inspected paths, - // so that we can send their data along if the element is re-rendered. - function mergeInspectedPaths(path: Array) { - let current = currentlyInspectedPaths; - path.forEach(key => { - if (!current[key]) { - current[key] = {}; - } - current = current[key]; - }); - } - function createIsPathAllowed( key: string | null, secondaryCategory: 'hooks' | null, + inspectedPaths: Object, ) { // This function helps prevent previously-inspected paths from being dehydrated in updates. // This is important to avoid a bad user experience where expanded toggles collapse on update. @@ -2754,8 +2755,7 @@ export function attach( break; } - let current = - key === null ? currentlyInspectedPaths : currentlyInspectedPaths[key]; + let current = key === null ? inspectedPaths : inspectedPaths[key]; if (!current) { return false; } @@ -2850,97 +2850,66 @@ export function attach( } function inspectElement( + requestID: number, id: number, - path?: Array, + inspectedPaths: Object, + forceUpdate: boolean, ): InspectedElementPayload { - const isCurrent = isMostRecentlyInspectedElementCurrent(id); + const isCurrent = !forceUpdate && isMostRecentlyInspectedElementCurrent(id); if (isCurrent) { - if (path != null) { - mergeInspectedPaths(path); - - let secondaryCategory = null; - if (path[0] === 'hooks') { - secondaryCategory = 'hooks'; - } - - // If this element has not been updated since it was last inspected, - // we can just return the subset of data in the newly-inspected path. - return { - id, - type: 'hydrated-path', - path, - value: cleanForBridge( - getInObject( - ((mostRecentlyInspectedElement: any): InspectedElement), - path, - ), - createIsPathAllowed(null, secondaryCategory), - path, - ), - }; - } else { - // If this element has not been updated since it was last inspected, we don't need to re-run it. - // Instead we can just return the ID to indicate that it has not changed. - return { - id, - type: 'no-change', - }; - } - } else { - hasElementUpdatedSinceLastInspected = false; - - if ( - mostRecentlyInspectedElement === null || - mostRecentlyInspectedElement.id !== id - ) { - currentlyInspectedPaths = {}; - } - - mostRecentlyInspectedElement = inspectElementRaw(id); - if (mostRecentlyInspectedElement === null) { - return { - id, - type: 'not-found', - }; - } - - if (path != null) { - mergeInspectedPaths(path); - } - - // Any time an inspected element has an update, - // we should update the selected $r value as wel. - // Do this before dehydration (cleanForBridge). - updateSelectedElement(mostRecentlyInspectedElement); + // If this element has not been updated since it was last inspected, we don't need to return it. + // Instead we can just return the ID to indicate that it has not changed. + return { + id, + responseID: requestID, + type: 'no-change', + }; + } - // Clone before cleaning so that we preserve the full data. - // This will enable us to send patches without re-inspecting if hydrated paths are requested. - // (Reducing how often we shallow-render is a better DX for function components that use hooks.) - const cleanedInspectedElement = {...mostRecentlyInspectedElement}; - cleanedInspectedElement.context = cleanForBridge( - cleanedInspectedElement.context, - createIsPathAllowed('context', null), - ); - cleanedInspectedElement.hooks = cleanForBridge( - cleanedInspectedElement.hooks, - createIsPathAllowed('hooks', 'hooks'), - ); - cleanedInspectedElement.props = cleanForBridge( - cleanedInspectedElement.props, - createIsPathAllowed('props', null), - ); - cleanedInspectedElement.state = cleanForBridge( - cleanedInspectedElement.state, - createIsPathAllowed('state', null), - ); + hasElementUpdatedSinceLastInspected = false; + mostRecentlyInspectedElement = inspectElementRaw(id); + if (mostRecentlyInspectedElement === null) { return { id, - type: 'full-data', - value: cleanedInspectedElement, + responseID: requestID, + type: 'not-found', }; } + + // Any time an inspected element has an update, + // we should update the selected $r value as wel. + // Do this before dehydration (cleanForBridge). + updateSelectedElement(mostRecentlyInspectedElement); + + // Clone before cleaning so that we preserve the full data. + // This will enable us to send patches without re-inspecting if hydrated paths are requested. + // (Reducing how often we shallow-render is a better DX for function components that use hooks.) + const cleanedInspectedElement = {...mostRecentlyInspectedElement}; + cleanedInspectedElement.context = cleanForBridge( + cleanedInspectedElement.context, + createIsPathAllowed('context', null, inspectedPaths), + ); + cleanedInspectedElement.hooks = cleanForBridge( + cleanedInspectedElement.hooks, + createIsPathAllowed('hooks', 'hooks', inspectedPaths), + ); + cleanedInspectedElement.props = cleanForBridge( + cleanedInspectedElement.props, + createIsPathAllowed('props', null, inspectedPaths), + ); + cleanedInspectedElement.state = cleanForBridge( + cleanedInspectedElement.state, + createIsPathAllowed('state', null, inspectedPaths), + ); + + return { + id, + responseID: requestID, + type: 'full-data', + value: cleanedInspectedElement, + }; } function logElementToConsole(id) { diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 93ac3c7827e8b..09b32aa1ceef3 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -26,6 +26,7 @@ export type WorkFlags = number; export type ExpirationTime = number; export type WorkTagMap = {| + CacheComponent: WorkTag, ClassComponent: WorkTag, ContextConsumer: WorkTag, ContextProvider: WorkTag, @@ -256,34 +257,28 @@ export type InspectedElement = {| export const InspectElementFullDataType = 'full-data'; export const InspectElementNoChangeType = 'no-change'; export const InspectElementNotFoundType = 'not-found'; -export const InspectElementHydratedPathType = 'hydrated-path'; type InspectElementFullData = {| id: number, + responseID: number, type: 'full-data', value: InspectedElement, |}; -type InspectElementHydratedPath = {| - id: number, - type: 'hydrated-path', - path: Array, - value: any, -|}; - type InspectElementNoChange = {| id: number, + responseID: number, type: 'no-change', |}; type InspectElementNotFound = {| id: number, + responseID: number, type: 'not-found', |}; export type InspectedElementPayload = | InspectElementFullData - | InspectElementHydratedPath | InspectElementNoChange | InspectElementNotFound; @@ -318,8 +313,10 @@ export type RendererInterface = { handleCommitFiberRoot: (fiber: Object, commitPriority?: number) => void, handleCommitFiberUnmount: (fiber: Object) => void, inspectElement: ( + requestID: number, id: number, - path?: Array, + inspectedPaths: Object, + forceUpdate: boolean, ) => InspectedElementPayload, logElementToConsole: (id: number) => void, overrideSuspense: (id: number, forceFallback: boolean) => void, diff --git a/packages/react-devtools-shared/src/backendAPI.js b/packages/react-devtools-shared/src/backendAPI.js new file mode 100644 index 0000000000000..372fd247d3b5a --- /dev/null +++ b/packages/react-devtools-shared/src/backendAPI.js @@ -0,0 +1,283 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import {hydrate, fillInPath} from 'react-devtools-shared/src/hydration'; +import {separateDisplayNameAndHOCs} from 'react-devtools-shared/src/utils'; +import Store from 'react-devtools-shared/src/devtools/store'; + +import type { + InspectedElement as InspectedElementBackend, + InspectedElementPayload, +} from 'react-devtools-shared/src/backend/types'; +import type { + BackendEvents, + FrontendBridge, +} from 'react-devtools-shared/src/bridge'; +import type { + DehydratedData, + InspectedElement as InspectedElementFrontend, +} from 'react-devtools-shared/src/devtools/views/Components/types'; + +export function clearErrorsAndWarnings({ + bridge, + store, +}: {| + bridge: FrontendBridge, + store: Store, +|}): void { + store.rootIDToRendererID.forEach(rendererID => { + bridge.send('clearErrorsAndWarnings', {rendererID}); + }); +} + +export function clearErrorsForElement({ + bridge, + id, + rendererID, +}: {| + bridge: FrontendBridge, + id: number, + rendererID: number, +|}): void { + bridge.send('clearErrorsForFiberID', { + rendererID, + id, + }); +} + +export function clearWarningsForElement({ + bridge, + id, + rendererID, +}: {| + bridge: FrontendBridge, + id: number, + rendererID: number, +|}): void { + bridge.send('clearWarningsForFiberID', { + rendererID, + id, + }); +} + +export function copyInspectedElementPath({ + bridge, + id, + path, + rendererID, +}: {| + bridge: FrontendBridge, + id: number, + path: Array, + rendererID: number, +|}): void { + bridge.send('copyElementPath', { + id, + path, + rendererID, + }); +} + +export function inspectElement({ + bridge, + forceUpdate, + id, + inspectedPaths, + rendererID, +}: {| + bridge: FrontendBridge, + forceUpdate: boolean, + id: number, + inspectedPaths: Object, + rendererID: number, +|}): Promise { + const requestID = requestCounter++; + const promise = getPromiseForRequestID( + requestID, + 'inspectedElement', + bridge, + ); + + bridge.send('inspectElement', { + forceUpdate, + id, + inspectedPaths, + rendererID, + requestID, + }); + + return promise; +} + +let storeAsGlobalCount = 0; + +export function storeAsGlobal({ + bridge, + id, + path, + rendererID, +}: {| + bridge: FrontendBridge, + id: number, + path: Array, + rendererID: number, +|}): void { + bridge.send('storeAsGlobal', { + count: storeAsGlobalCount++, + id, + path, + rendererID, + }); +} + +const TIMEOUT_DELAY = 5000; + +let requestCounter = 0; + +function getPromiseForRequestID( + requestID: number, + eventType: $Keys, + bridge: FrontendBridge, +): Promise { + return new Promise((resolve, reject) => { + const cleanup = () => { + bridge.removeListener(eventType, onInspectedElement); + + clearTimeout(timeoutID); + }; + + const onInspectedElement = (data: any) => { + if (data.responseID === requestID) { + cleanup(); + resolve((data: T)); + } + }; + + const onTimeout = () => { + cleanup(); + reject(); + }; + + bridge.addListener(eventType, onInspectedElement); + + const timeoutID = setTimeout(onTimeout, TIMEOUT_DELAY); + }); +} + +export function cloneInspectedElementWithPath( + inspectedElement: InspectedElementFrontend, + path: Array, + value: Object, +): InspectedElementFrontend { + const hydratedValue = hydrateHelper(value, path); + const clonedInspectedElement = {...inspectedElement}; + + fillInPath(clonedInspectedElement, value, path, hydratedValue); + + return clonedInspectedElement; +} + +export function convertInspectedElementBackendToFrontend( + inspectedElementBackend: InspectedElementBackend, +): InspectedElementFrontend { + const { + canEditFunctionProps, + canEditFunctionPropsDeletePaths, + canEditFunctionPropsRenamePaths, + canEditHooks, + canEditHooksAndDeletePaths, + canEditHooksAndRenamePaths, + canToggleSuspense, + canViewSource, + hasLegacyContext, + id, + source, + type, + owners, + context, + hooks, + props, + rendererPackageName, + rendererVersion, + rootType, + state, + key, + errors, + warnings, + } = inspectedElementBackend; + + const inspectedElement: InspectedElementFrontend = { + canEditFunctionProps, + canEditFunctionPropsDeletePaths, + canEditFunctionPropsRenamePaths, + canEditHooks, + canEditHooksAndDeletePaths, + canEditHooksAndRenamePaths, + canToggleSuspense, + canViewSource, + hasLegacyContext, + id, + key, + rendererPackageName, + rendererVersion, + rootType, + source, + type, + owners: + owners === null + ? null + : owners.map(owner => { + const [displayName, hocDisplayNames] = separateDisplayNameAndHOCs( + owner.displayName, + owner.type, + ); + return { + ...owner, + displayName, + hocDisplayNames, + }; + }), + context: hydrateHelper(context), + hooks: hydrateHelper(hooks), + props: hydrateHelper(props), + state: hydrateHelper(state), + errors, + warnings, + }; + + return inspectedElement; +} + +function hydrateHelper( + dehydratedData: DehydratedData | null, + path?: Array, +): Object | null { + if (dehydratedData !== null) { + const {cleaned, data, unserializable} = dehydratedData; + + if (path) { + const {length} = path; + if (length > 0) { + // Hydration helper requires full paths, but inspection dehydrates with relative paths. + // In that event it's important that we adjust the "cleaned" paths to match. + return hydrate( + data, + cleaned.map(cleanedPath => cleanedPath.slice(length)), + unserializable.map(unserializablePath => + unserializablePath.slice(length), + ), + ); + } + } + + return hydrate(data, cleaned, unserializable); + } else { + return null; + } +} diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js index 1da4d8eb3e549..dad25e93258c1 100644 --- a/packages/react-devtools-shared/src/bridge.js +++ b/packages/react-devtools-shared/src/bridge.js @@ -89,7 +89,9 @@ type ViewAttributeSourceParams = {| type InspectElementParams = {| ...ElementAndRendererID, - path?: Array, + forceUpdate: boolean, + inspectedPaths: Object, + requestID: number, |}; type StoreAsGlobalParams = {| @@ -117,7 +119,7 @@ type UpdateConsolePatchSettingsParams = {| showInlineWarningsAndErrors: boolean, |}; -type BackendEvents = {| +export type BackendEvents = {| extensionBackendInitialized: [], inspectedElement: [InspectedElementPayload], isBackendStorageAPISupported: [boolean], diff --git a/packages/react-devtools-shared/src/devtools/cache.js b/packages/react-devtools-shared/src/devtools/cache.js index af6a02faa0cf2..573f666402717 100644 --- a/packages/react-devtools-shared/src/devtools/cache.js +++ b/packages/react-devtools-shared/src/devtools/cache.js @@ -12,6 +12,8 @@ import type {Thenable} from 'shared/ReactTypes'; import * as React from 'react'; import {createContext} from 'react'; +// TODO (cache) Remove this cache; it is outdated and will not work with newer APIs like startTransition. + // Cache implementation was forked from the React repo: // https://github.com/facebook/react/blob/master/packages/react-cache/src/ReactCache.js // diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index 6b1b83e524838..e5a6ac15fbde1 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -101,7 +101,7 @@ export default class Store extends EventEmitter<{| // Map of ID to (mutable) Element. // Elements are mutated to avoid excessive cloning during tree updates. - // The InspectedElementContext also relies on this mutability for its WeakMap usage. + // The InspectedElement Suspense cache also relies on this mutability for its WeakMap usage. _idToElement: Map = new Map(); // Should the React Native style editor panel be shown? @@ -378,42 +378,6 @@ export default class Store extends EventEmitter<{| return this._cachedWarningCount; } - clearErrorsAndWarnings(): void { - this._rootIDToRendererID.forEach(rendererID => { - this._bridge.send('clearErrorsAndWarnings', { - rendererID, - }); - }); - } - - clearErrorsForElement(id: number): void { - const rendererID = this.getRendererIDForElement(id); - if (rendererID === null) { - console.warn( - `Unable to find rendererID for element ${id} when clearing errors.`, - ); - } else { - this._bridge.send('clearErrorsForFiberID', { - rendererID, - id, - }); - } - } - - clearWarningsForElement(id: number): void { - const rendererID = this.getRendererIDForElement(id); - if (rendererID === null) { - console.warn( - `Unable to find rendererID for element ${id} when clearing warnings.`, - ); - } else { - this._bridge.send('clearWarningsForFiberID', { - rendererID, - id, - }); - } - } - containsElement(id: number): boolean { return this._idToElement.get(id) != null; } diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Components.js b/packages/react-devtools-shared/src/devtools/views/Components/Components.js index 91e31d4bf0294..712afdd987287 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/Components.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/Components.js @@ -17,7 +17,6 @@ import { useRef, } from 'react'; import Tree from './Tree'; -import {InspectedElementContextController} from './InspectedElementContext'; import {OwnersListContextController} from './OwnersListContext'; import portaledContent from '../portaledContent'; import {SettingsModalContextController} from 'react-devtools-shared/src/devtools/views/Settings/SettingsModalContext'; @@ -25,7 +24,9 @@ import { localStorageGetItem, localStorageSetItem, } from 'react-devtools-shared/src/storage'; +import InspectedElementErrorBoundary from './InspectedElementErrorBoundary'; import InspectedElement from './InspectedElement'; +import {InspectedElementContextController} from './InspectedElementContext'; import {ModalDialog} from '../ModalDialog'; import SettingsModal from 'react-devtools-shared/src/devtools/views/Settings/SettingsModal'; import {NativeStyleContextController} from './NativeStyleEditor/context'; @@ -151,32 +152,34 @@ function Components(_: {||}) { return ( - -
- -
- -
-
-
-
-
- +
+ +
+ +
+
+
+
+
+ + }> - + + + - -
- - - -
- + + +
+ + + +
); diff --git a/packages/react-devtools-shared/src/devtools/views/Components/ExpandCollapseToggle.js b/packages/react-devtools-shared/src/devtools/views/Components/ExpandCollapseToggle.js index 8950696b7e4fc..1851c75e0391e 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/ExpandCollapseToggle.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/ExpandCollapseToggle.js @@ -14,17 +14,20 @@ import ButtonIcon from '../ButtonIcon'; import styles from './ExpandCollapseToggle.css'; type ExpandCollapseToggleProps = {| + disabled: boolean, isOpen: boolean, setIsOpen: Function, |}; export default function ExpandCollapseToggle({ + disabled, isOpen, setIsOpen, }: ExpandCollapseToggleProps) { return (
diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js index 01b05a9fa33a1..48768b57aaac1 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js @@ -10,6 +10,8 @@ import * as React from 'react'; import { createContext, + unstable_startTransition as startTransition, + unstable_useCacheRefresh as useCacheRefresh, useCallback, useContext, useEffect, @@ -17,360 +19,134 @@ import { useRef, useState, } from 'react'; -import {unstable_batchedUpdates as batchedUpdates} from 'react-dom'; -import {createResource} from '../../cache'; -import {BridgeContext, StoreContext} from '../context'; -import {hydrate, fillInPath} from 'react-devtools-shared/src/hydration'; import {TreeStateContext} from './TreeContext'; -import {separateDisplayNameAndHOCs} from 'react-devtools-shared/src/utils'; +import {BridgeContext, StoreContext} from '../context'; +import { + checkForUpdate, + inspectElement, +} from 'react-devtools-shared/src/inspectedElementCache'; +import type {ReactNodeList} from 'shared/ReactTypes'; import type { - InspectedElement as InspectedElementBackend, - InspectedElementPayload, -} from 'react-devtools-shared/src/backend/types'; -import type { - DehydratedData, Element, - InspectedElement as InspectedElementFrontend, + InspectedElement, } from 'react-devtools-shared/src/devtools/views/Components/types'; -import type {Resource, Thenable} from '../../cache'; - -export type StoreAsGlobal = (id: number, path: Array) => void; -export type CopyInspectedElementPath = ( - id: number, - path: Array, -) => void; +type Path = Array; +type InspectPathFunction = (path: Path) => void; -export type GetInspectedElementPath = ( - id: number, - path: Array, -) => void; - -export type GetInspectedElement = ( - id: number, -) => InspectedElementFrontend | null; - -type RefreshInspectedElement = () => void; - -export type InspectedElementContextType = {| - copyInspectedElementPath: CopyInspectedElementPath, - getInspectedElementPath: GetInspectedElementPath, - getInspectedElement: GetInspectedElement, - refreshInspectedElement: RefreshInspectedElement, - storeAsGlobal: StoreAsGlobal, +type Context = {| + inspectedElement: InspectedElement | null, + inspectPaths: InspectPathFunction, |}; -const InspectedElementContext = createContext( - ((null: any): InspectedElementContextType), +export const InspectedElementContext = createContext( + ((null: any): Context), ); -InspectedElementContext.displayName = 'InspectedElementContext'; - -type ResolveFn = (inspectedElement: InspectedElementFrontend) => void; -type InProgressRequest = {| - promise: Thenable, - resolveFn: ResolveFn, -|}; - -const inProgressRequests: WeakMap = new WeakMap(); -const resource: Resource< - Element, - Element, - InspectedElementFrontend, -> = createResource( - (element: Element) => { - const request = inProgressRequests.get(element); - if (request != null) { - return request.promise; - } - - let resolveFn = ((null: any): ResolveFn); - const promise = new Promise(resolve => { - resolveFn = resolve; - }); - - inProgressRequests.set(element, {promise, resolveFn}); - return promise; - }, - (element: Element) => element, - {useWeakMap: true}, -); +const POLL_INTERVAL = 1000; -type Props = {| - children: React$Node, +export type Props = {| + children: ReactNodeList, |}; -function InspectedElementContextController({children}: Props) { +export function InspectedElementContextController({children}: Props) { + const {selectedElementID} = useContext(TreeStateContext); const bridge = useContext(BridgeContext); const store = useContext(StoreContext); - const storeAsGlobalCount = useRef(1); - - // Ask the backend to store the value at the specified path as a global variable. - const storeAsGlobal = useCallback( - (id: number, path: Array) => { - const rendererID = store.getRendererIDForElement(id); - if (rendererID !== null) { - bridge.send('storeAsGlobal', { - count: storeAsGlobalCount.current++, - id, - path, - rendererID, - }); - } - }, - [bridge, store], - ); - - // Ask the backend to copy the specified path to the clipboard. - const copyInspectedElementPath = useCallback( - (id: number, path: Array) => { - const rendererID = store.getRendererIDForElement(id); - if (rendererID !== null) { - bridge.send('copyElementPath', {id, path, rendererID}); - } - }, - [bridge, store], - ); - - // Ask the backend to fill in a "dehydrated" path; this will result in a "inspectedElement". - const getInspectedElementPath = useCallback( - (id: number, path: Array) => { - const rendererID = store.getRendererIDForElement(id); - if (rendererID !== null) { - bridge.send('inspectElement', {id, path, rendererID}); - } - }, - [bridge, store], - ); - - const getInspectedElement = useCallback( - (id: number) => { - const element = store.getElementByID(id); - if (element !== null) { - return resource.read(element); - } else { - return null; - } - }, - [store], - ); - - // It's very important that this context consumes selectedElementID and not inspectedElementID. - // Otherwise the effect that sends the "inspect" message across the bridge- - // would itself be blocked by the same render that suspends (waiting for the data). - const {selectedElementID} = useContext(TreeStateContext); - - const refreshInspectedElement = useCallback(() => { - if (selectedElementID !== null) { - const rendererID = store.getRendererIDForElement(selectedElementID); - if (rendererID !== null) { - bridge.send('inspectElement', {id: selectedElementID, rendererID}); - } - } - }, [bridge, selectedElementID]); - - const [ - currentlyInspectedElement, - setCurrentlyInspectedElement, - ] = useState(null); - - // This effect handler invalidates the suspense cache and schedules rendering updates with React. - useEffect(() => { - const onInspectedElement = (data: InspectedElementPayload) => { - const {id} = data; + const refresh = useCacheRefresh(); - let element; + // Track when insepected paths have changed; we need to force the backend to send an udpate then. + const forceUpdateRef = useRef(true); - switch (data.type) { - case 'no-change': - case 'not-found': - // No-op - break; - case 'hydrated-path': - // Merge new data into previous object and invalidate cache - element = store.getElementByID(id); - if (element !== null) { - if (currentlyInspectedElement != null) { - const value = hydrateHelper(data.value, data.path); - const inspectedElement = {...currentlyInspectedElement}; + // Track the paths insepected for the currently selected element. + const [state, setState] = useState<{| + element: Element | null, + inspectedPaths: Object, + |}>({ + element: null, + inspectedPaths: {}, + }); - fillInPath(inspectedElement, data.value, data.path, value); + const element = + selectedElementID !== null ? store.getElementByID(selectedElementID) : null; - resource.write(element, inspectedElement); + const elementHasChanged = element !== null && element !== state.element; - // Schedule update with React if the currently-selected element has been invalidated. - if (id === selectedElementID) { - setCurrentlyInspectedElement(inspectedElement); - } - } - } - break; - case 'full-data': - const { - canEditFunctionProps, - canEditFunctionPropsDeletePaths, - canEditFunctionPropsRenamePaths, - canEditHooks, - canEditHooksAndDeletePaths, - canEditHooksAndRenamePaths, - canToggleSuspense, - canViewSource, - hasLegacyContext, - source, - type, - owners, - context, - hooks, - props, - rendererPackageName, - rendererVersion, - rootType, - state, - key, - errors, - warnings, - } = ((data.value: any): InspectedElementBackend); - - const inspectedElement: InspectedElementFrontend = { - canEditFunctionProps, - canEditFunctionPropsDeletePaths, - canEditFunctionPropsRenamePaths, - canEditHooks, - canEditHooksAndDeletePaths, - canEditHooksAndRenamePaths, - canToggleSuspense, - canViewSource, - hasLegacyContext, - id, - key, - rendererPackageName, - rendererVersion, - rootType, - source, - type, - owners: - owners === null - ? null - : owners.map(owner => { - const [ - displayName, - hocDisplayNames, - ] = separateDisplayNameAndHOCs( - owner.displayName, - owner.type, - ); - return { - ...owner, - displayName, - hocDisplayNames, - }; - }), - context: hydrateHelper(context), - hooks: hydrateHelper(hooks), - props: hydrateHelper(props), - state: hydrateHelper(state), - errors, - warnings, - }; - - element = store.getElementByID(id); - if (element !== null) { - const request = inProgressRequests.get(element); - if (request != null) { - inProgressRequests.delete(element); - batchedUpdates(() => { - request.resolveFn(inspectedElement); - setCurrentlyInspectedElement(inspectedElement); - }); - } else { - resource.write(element, inspectedElement); + // Reset the cached inspected paths when a new element is selected. + if (elementHasChanged) { + setState({ + element, + inspectedPaths: {}, + }); + } - // Schedule update with React if the currently-selected element has been invalidated. - if (id === selectedElementID) { - setCurrentlyInspectedElement(inspectedElement); - } + // Don't load a stale element from the backend; it wastes bridge bandwidth. + const inspectedElement = + !elementHasChanged && element !== null + ? inspectElement( + element, + state.inspectedPaths, + forceUpdateRef.current, + store, + bridge, + ) + : null; + + const inspectPaths: InspectPathFunction = useCallback( + (path: Path) => { + startTransition(() => { + forceUpdateRef.current = true; + setState(prevState => { + const cloned = {...prevState}; + let current = cloned.inspectedPaths; + path.forEach(key => { + if (!current[key]) { + current[key] = {}; } - } - break; - default: - break; - } - }; - - bridge.addListener('inspectedElement', onInspectedElement); - return () => bridge.removeListener('inspectedElement', onInspectedElement); - }, [bridge, currentlyInspectedElement, selectedElementID, store]); + current = current[key]; + }); + return cloned; + }); + refresh(); + }); + }, + [setState], + ); - // This effect handler polls for updates on the currently selected element. + // Force backend update when inspected paths change. useEffect(() => { - if (selectedElementID === null) { - return () => {}; - } - - const rendererID = store.getRendererIDForElement(selectedElementID); - - let timeoutID: TimeoutID | null = null; + forceUpdateRef.current = false; + }, [element, state]); - const sendRequest = () => { - timeoutID = null; - - if (rendererID !== null) { - bridge.send('inspectElement', {id: selectedElementID, rendererID}); - } - }; - - // Send the initial inspection request. - // We'll poll for an update in the response handler below. - sendRequest(); - - const onInspectedElement = (data: InspectedElementPayload) => { - // If this is the element we requested, wait a little bit and then ask for another update. - if (data.id === selectedElementID) { - switch (data.type) { - case 'no-change': - case 'full-data': - case 'hydrated-path': - if (timeoutID !== null) { - clearTimeout(timeoutID); - } - timeoutID = setTimeout(sendRequest, 1000); - break; - default: - break; - } - } - }; - - bridge.addListener('inspectedElement', onInspectedElement); - - return () => { - bridge.removeListener('inspectedElement', onInspectedElement); - - if (timeoutID !== null) { + // Periodically poll the selected element for updates. + useEffect(() => { + if (element !== null) { + const inspectedPaths = state.inspectedPaths; + const checkForUpdateWrapper = () => { + checkForUpdate({bridge, element, inspectedPaths, refresh, store}); + timeoutID = setTimeout(checkForUpdateWrapper, POLL_INTERVAL); + }; + let timeoutID = setTimeout(checkForUpdateWrapper, POLL_INTERVAL); + return () => { clearTimeout(timeoutID); - } - }; - }, [bridge, selectedElementID, store]); - - const value = useMemo( + }; + } + }, [ + element, + // Reset this timer any time the element we're inspecting gets a new response. + // No sense to ping right away after e.g. inspecting/hydrating a path. + inspectedElement, + state, + ]); + + const value = useMemo( () => ({ - copyInspectedElementPath, - getInspectedElement, - getInspectedElementPath, - refreshInspectedElement, - storeAsGlobal, + inspectedElement, + inspectPaths, }), - // InspectedElement is used to invalidate the cache and schedule an update with React. - [ - copyInspectedElementPath, - currentlyInspectedElement, - getInspectedElement, - getInspectedElementPath, - refreshInspectedElement, - storeAsGlobal, - ], + [inspectedElement, inspectPaths], ); return ( @@ -379,33 +155,3 @@ function InspectedElementContextController({children}: Props) { ); } - -function hydrateHelper( - dehydratedData: DehydratedData | null, - path?: Array, -): Object | null { - if (dehydratedData !== null) { - const {cleaned, data, unserializable} = dehydratedData; - - if (path) { - const {length} = path; - if (length > 0) { - // Hydration helper requires full paths, but inspection dehydrates with relative paths. - // In that event it's important that we adjust the "cleaned" paths to match. - return hydrate( - data, - cleaned.map(cleanedPath => cleanedPath.slice(length)), - unserializable.map(unserializablePath => - unserializablePath.slice(length), - ), - ); - } - } - - return hydrate(data, cleaned, unserializable); - } else { - return null; - } -} - -export {InspectedElementContext, InspectedElementContextController}; diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContextTree.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContextTree.js index 22347cef07f63..8dfe6310a3609 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContextTree.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContextTree.js @@ -20,20 +20,20 @@ import { ElementTypeFunction, } from 'react-devtools-shared/src/types'; -import type {GetInspectedElementPath} from './InspectedElementContext'; import type {InspectedElement} from './types'; import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; +import type {Element} from 'react-devtools-shared/src/devtools/views/Components/types'; type Props = {| bridge: FrontendBridge, - getInspectedElementPath: GetInspectedElementPath, + element: Element, inspectedElement: InspectedElement, store: Store, |}; export default function InspectedElementContextTree({ bridge, - getInspectedElementPath, + element, inspectedElement, store, }: Props) { @@ -81,15 +81,15 @@ export default function InspectedElementContextTree({ canEditValues={!isReadOnly} canRenamePaths={!isReadOnly} canRenamePathsAtDepth={canRenamePathsAtDepth} - type="context" depth={1} - getInspectedElementPath={getInspectedElementPath} + element={element} hidden={false} inspectedElement={inspectedElement} name={name} path={[name]} pathRoot="context" store={store} + type="context" value={value} /> ))} diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.css b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.css new file mode 100644 index 0000000000000..399f83cec45b0 --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.css @@ -0,0 +1,21 @@ +.Error { + justify-content: center; + align-items: center; + display: flex; + flex-direction: column; + height: 100%; + font-size: var(--font-size-sans-large); + font-weight: bold; + text-align: center; + background-color: var(--color-error-background); + color: var(--color-error-text); + border: 1px solid var(--color-error-border); + padding: 1rem; +} + +.Message { + margin-bottom: 1rem; +} + +.RetryButton { +} \ No newline at end of file diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.js new file mode 100644 index 0000000000000..09c11664dcf77 --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.js @@ -0,0 +1,93 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import * as React from 'react'; +import {Component, useContext} from 'react'; +import {TreeDispatcherContext} from './TreeContext'; +import Button from 'react-devtools-shared/src/devtools/views/Button'; +import styles from './InspectedElementErrorBoundary.css'; + +import type {DispatcherContext} from './InspectedElementErrorBoundary.css'; + +type WrapperProps = {| + children: React$Node, +|}; + +export default function InspectedElementErrorBoundaryWrapper({ + children, +}: WrapperProps) { + const dispatch = useContext(TreeDispatcherContext); + + return ( + + ); +} + +type Props = {| + children: React$Node, + dispatch: DispatcherContext, +|}; + +type State = {| + errorMessage: string | null, + hasError: boolean, +|}; + +const InitialState: State = { + errorMessage: null, + hasError: false, +}; + +class InspectedElementErrorBoundary extends Component { + state: State = InitialState; + + static getDerivedStateFromError(error: any) { + const errorMessage = + typeof error === 'object' && + error !== null && + error.hasOwnProperty('message') + ? error.message + : error; + + return { + errorMessage, + hasError: true, + }; + } + + render() { + const {children} = this.props; + const {errorMessage, hasError} = this.state; + + if (hasError) { + return ( +
+
{errorMessage || 'Error'}
+ +
+ ); + } + + return children; + } + + _retry = () => { + const {dispatch} = this.props; + dispatch({ + type: 'SELECT_ELEMENT_BY_ID', + payload: null, + }); + this.setState({ + errorMessage: null, + hasError: false, + }); + }; +} diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorsAndWarningsTree.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorsAndWarningsTree.js index 1f161c98836b6..3b7aaca3216d5 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorsAndWarningsTree.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorsAndWarningsTree.js @@ -8,14 +8,21 @@ */ import * as React from 'react'; -import {useContext} from 'react'; +import { + useContext, + unstable_useCacheRefresh as useCacheRefresh, + unstable_useTransition as useTransition, +} from 'react'; import Button from '../Button'; import ButtonIcon from '../ButtonIcon'; import Store from '../../store'; import sharedStyles from './InspectedElementSharedStyles.css'; import styles from './InspectedElementErrorsAndWarningsTree.css'; import {SettingsContext} from '../Settings/SettingsContext'; -import {InspectedElementContext} from './InspectedElementContext'; +import { + clearErrorsForElement as clearErrorsForElementAPI, + clearWarningsForElement as clearWarningsForElementAPI, +} from 'react-devtools-shared/src/backendAPI'; import type {InspectedElement} from './types'; import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; @@ -31,7 +38,45 @@ export default function InspectedElementErrorsAndWarningsTree({ inspectedElement, store, }: Props) { - const {refreshInspectedElement} = useContext(InspectedElementContext); + const refresh = useCacheRefresh(); + + const [ + startClearErrorsTransition, + isErrorsTransitionPending, + ] = useTransition(); + const clearErrorsForInspectedElement = () => { + const {id} = inspectedElement; + const rendererID = store.getRendererIDForElement(id); + if (rendererID !== null) { + startClearErrorsTransition(() => { + clearErrorsForElementAPI({ + bridge, + id, + rendererID, + }); + refresh(); + }); + } + }; + + const [ + startClearWarningsTransition, + isWarningsTransitionPending, + ] = useTransition(); + const clearWarningsForInspectedElement = () => { + const {id} = inspectedElement; + const rendererID = store.getRendererIDForElement(id); + if (rendererID !== null) { + startClearWarningsTransition(() => { + clearWarningsForElementAPI({ + bridge, + id, + rendererID, + }); + refresh(); + }); + } + }; const {showInlineWarningsAndErrors} = useContext(SettingsContext); if (!showInlineWarningsAndErrors) { @@ -40,26 +85,6 @@ export default function InspectedElementErrorsAndWarningsTree({ const {errors, warnings} = inspectedElement; - const clearErrors = () => { - const {id} = inspectedElement; - store.clearErrorsForElement(id); - - // Immediately poll for updated data. - // This avoids a delay between clicking the clear button and refreshing errors. - // Ideally this would be done with useTranstion but that requires updating to a newer Cache strategy. - refreshInspectedElement(); - }; - - const clearWarnings = () => { - const {id} = inspectedElement; - store.clearWarningsForElement(id); - - // Immediately poll for updated data. - // This avoids a delay between clicking the clear button and refreshing warnings. - // Ideally this would be done with useTranstion but that requires updating to a newer Cache strategy. - refreshInspectedElement(); - }; - return ( {errors.length > 0 && ( @@ -67,8 +92,9 @@ export default function InspectedElementErrorsAndWarningsTree({ badgeClassName={styles.ErrorBadge} bridge={bridge} className={styles.ErrorTree} - clearMessages={clearErrors} + clearMessages={clearErrorsForInspectedElement} entries={errors} + isTransitionPending={isErrorsTransitionPending} label="errors" messageClassName={styles.Error} /> @@ -78,8 +104,9 @@ export default function InspectedElementErrorsAndWarningsTree({ badgeClassName={styles.WarningBadge} bridge={bridge} className={styles.WarningTree} - clearMessages={clearWarnings} + clearMessages={clearWarningsForInspectedElement} entries={warnings} + isTransitionPending={isWarningsTransitionPending} label="warnings" messageClassName={styles.Warning} /> @@ -94,6 +121,7 @@ type TreeProps = {| className: string, clearMessages: () => {}, entries: Array<[string, number]>, + isTransitionPending: boolean, label: string, messageClassName: string, |}; @@ -104,6 +132,7 @@ function Tree({ className, clearMessages, entries, + isTransitionPending, label, messageClassName, }: TreeProps) { @@ -115,6 +144,7 @@ function Tree({
{label}
diff --git a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary.js b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary.js index bd932cff395f9..7b777115d3b5e 100644 --- a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary.js +++ b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary.js @@ -38,14 +38,25 @@ const InitialState: State = { export default class ErrorBoundary extends Component { state: State = InitialState; - componentDidCatch(error: any, {componentStack}: any) { + static getDerivedStateFromError(error: any) { const errorMessage = - typeof error === 'object' && error.hasOwnProperty('message') + typeof error === 'object' && + error !== null && + error.hasOwnProperty('message') ? error.message : error; + return { + errorMessage, + hasError: true, + }; + } + + componentDidCatch(error: any, {componentStack}: any) { const callStack = - typeof error === 'object' && error.hasOwnProperty('stack') + typeof error === 'object' && + error !== null && + error.hasOwnProperty('stack') ? error.stack .split('\n') .slice(1) @@ -55,8 +66,6 @@ export default class ErrorBoundary extends Component { this.setState({ callStack, componentStack, - errorMessage, - hasError: true, }); } diff --git a/packages/react-devtools-shared/src/inspectedElementCache.js b/packages/react-devtools-shared/src/inspectedElementCache.js new file mode 100644 index 0000000000000..6169207df0dbe --- /dev/null +++ b/packages/react-devtools-shared/src/inspectedElementCache.js @@ -0,0 +1,220 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import { + unstable_getCacheForType, + unstable_startTransition as startTransition, +} from 'react'; +import Store from './devtools/store'; +import { + convertInspectedElementBackendToFrontend, + inspectElement as inspectElementAPI, +} from './backendAPI'; + +import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; +import type {Wakeable} from 'shared/ReactTypes'; +import type { + InspectedElement as InspectedElementBackend, + InspectedElementPayload, +} from 'react-devtools-shared/src/backend/types'; +import type { + Element, + InspectedElement as InspectedElementFrontend, +} from 'react-devtools-shared/src/devtools/views/Components/types'; + +const Pending = 0; +const Resolved = 1; +const Rejected = 2; + +type PendingRecord = {| + status: 0, + value: Wakeable, +|}; + +type ResolvedRecord = {| + status: 1, + value: T, +|}; + +type RejectedRecord = {| + status: 2, + value: string, +|}; + +type Record = PendingRecord | ResolvedRecord | RejectedRecord; + +function readRecord(record: Record): ResolvedRecord { + if (record.status === Resolved) { + // This is just a type refinement. + return record; + } else { + throw record.value; + } +} + +type InspectedElementMap = WeakMap>; +type CacheSeedKey = () => InspectedElementMap; + +function createMap(): InspectedElementMap { + return new WeakMap(); +} + +function getRecordMap(): WeakMap> { + return unstable_getCacheForType(createMap); +} + +function createCacheSeed( + element: Element, + inspectedElement: InspectedElementFrontend, +): [CacheSeedKey, InspectedElementMap] { + const newRecord: Record = { + status: Resolved, + value: inspectedElement, + }; + const map = createMap(); + map.set(element, newRecord); + return [createMap, map]; +} + +/** + * Fetches element props and state from the backend for inspection. + * This method should be called during render; it will suspend if data has not yet been fetched. + */ +export function inspectElement( + element: Element, + inspectedPaths: Object, + forceUpdate: boolean, + store: Store, + bridge: FrontendBridge, +): InspectedElementFrontend | null { + const map = getRecordMap(); + let record = map.get(element); + if (!record) { + const callbacks = new Set(); + const wakeable: Wakeable = { + then(callback) { + callbacks.add(callback); + }, + }; + const wake = () => { + // This assumes they won't throw. + callbacks.forEach(callback => callback()); + callbacks.clear(); + }; + const newRecord: Record = (record = { + status: Pending, + value: wakeable, + }); + + const rendererID = store.getRendererIDForElement(element.id); + if (rendererID == null) { + const rejectedRecord = ((newRecord: any): RejectedRecord); + rejectedRecord.status = Rejected; + rejectedRecord.value = 'Inspected element not found.'; + return null; + } + + inspectElementAPI({ + bridge, + forceUpdate: true, + id: element.id, + inspectedPaths, + rendererID: ((rendererID: any): number), + }).then( + (data: InspectedElementPayload) => { + if (newRecord.status === Pending) { + switch (data.type) { + case 'no-change': + // This response type should never be received. + // We always send forceUpdate:true when we have a cache miss. + break; + + case 'not-found': + const notFoundRecord = ((newRecord: any): RejectedRecord); + notFoundRecord.status = Rejected; + notFoundRecord.value = 'Inspected element not found.'; + wake(); + break; + + case 'full-data': + const resolvedRecord = ((newRecord: any): ResolvedRecord); + resolvedRecord.status = Resolved; + resolvedRecord.value = convertInspectedElementBackendToFrontend( + ((data.value: any): InspectedElementBackend), + ); + wake(); + break; + } + } + }, + + () => { + // Timed out without receiving a response. + if (newRecord.status === Pending) { + const timedOutRecord = ((newRecord: any): RejectedRecord); + timedOutRecord.status = Rejected; + timedOutRecord.value = 'Inspected element timed out.'; + wake(); + } + }, + ); + map.set(element, record); + } + + const response = readRecord(record).value; + return response; +} + +type RefreshFunction = ( + seedKey: CacheSeedKey, + cacheMap: InspectedElementMap, +) => void; + +/** + * Asks the backend for updated props and state from an expected element. + * This method should never be called during render; call it from an effect or event handler. + * This method will schedule an update if updated information is returned. + */ +export function checkForUpdate({ + bridge, + element, + inspectedPaths, + refresh, + store, +}: { + bridge: FrontendBridge, + element: Element, + inspectedPaths: Object, + refresh: RefreshFunction, + store: Store, +}): void { + const {id} = element; + const rendererID = store.getRendererIDForElement(id); + if (rendererID != null) { + inspectElementAPI({ + bridge, + forceUpdate: false, + id, + inspectedPaths, + rendererID, + }).then((data: InspectedElementPayload) => { + switch (data.type) { + case 'full-data': + const inspectedElement = convertInspectedElementBackendToFrontend( + ((data.value: any): InspectedElementBackend), + ); + startTransition(() => { + const [key, value] = createCacheSeed(element, inspectedElement); + refresh(key, value); + }); + break; + } + }); + } +} diff --git a/packages/react-devtools-shell/src/app/ElementTypes/index.js b/packages/react-devtools-shell/src/app/ElementTypes/index.js index cc3905457a996..4b44a50a6653e 100644 --- a/packages/react-devtools-shell/src/app/ElementTypes/index.js +++ b/packages/react-devtools-shell/src/app/ElementTypes/index.js @@ -19,6 +19,7 @@ import { Profiler, StrictMode, Suspense, + unstable_Cache as Cache, } from 'react'; const Context = createContext('abc'); @@ -61,15 +62,17 @@ export default function ElementTypes() { {value => null} - Loading...
}> - - - - - - - - + + Loading...
}> + + + + + + + + + diff --git a/packages/react-devtools/package.json b/packages/react-devtools/package.json index 3a22f5c622f20..ad5d429fd1841 100644 --- a/packages/react-devtools/package.json +++ b/packages/react-devtools/package.json @@ -24,7 +24,7 @@ }, "dependencies": { "cross-spawn": "^5.0.1", - "electron": "^9.1.0", + "electron": "^11.1.0", "ip": "^1.1.4", "minimist": "^1.2.3", "react-devtools-core": "4.10.1", diff --git a/packages/react-reconciler/src/ReactChildFiber.new.js b/packages/react-reconciler/src/ReactChildFiber.new.js index 7163df34d8568..83362b5e15411 100644 --- a/packages/react-reconciler/src/ReactChildFiber.new.js +++ b/packages/react-reconciler/src/ReactChildFiber.new.js @@ -282,22 +282,13 @@ function ChildReconciler(shouldTrackSideEffects) { childToDelete.nextEffect = null; childToDelete.flags = (childToDelete.flags & StaticMask) | Deletion; - let deletions = returnFiber.deletions; + const deletions = returnFiber.deletions; if (deletions === null) { - deletions = returnFiber.deletions = [childToDelete]; + returnFiber.deletions = [childToDelete]; returnFiber.flags |= ChildDeletion; } else { deletions.push(childToDelete); } - // Stash a reference to the return fiber's deletion array on each of the - // deleted children. This is really weird, but it's a temporary workaround - // while we're still using the effect list to traverse effect fibers. A - // better workaround would be to follow the `.return` pointer in the commit - // phase, but unfortunately we can't assume that `.return` points to the - // correct fiber, even in the commit phase, because `findDOMNode` might - // mutate it. - // TODO: Remove this line. - childToDelete.deletions = deletions; } function deleteRemainingChildren( diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 52e6216d442eb..9684efafc409c 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -2203,14 +2203,13 @@ function updateSuspensePrimaryChildren( currentFallbackChildFragment.flags = (currentFallbackChildFragment.flags & StaticMask) | Deletion; workInProgress.firstEffect = workInProgress.lastEffect = currentFallbackChildFragment; - let deletions = workInProgress.deletions; + const deletions = workInProgress.deletions; if (deletions === null) { - deletions = workInProgress.deletions = [currentFallbackChildFragment]; + workInProgress.deletions = [currentFallbackChildFragment]; workInProgress.flags |= ChildDeletion; } else { deletions.push(currentFallbackChildFragment); } - currentFallbackChildFragment.deletions = deletions; } workInProgress.child = primaryChildFragment; @@ -3194,14 +3193,13 @@ function remountFiber( current.nextEffect = null; current.flags = (current.flags & StaticMask) | Deletion; - let deletions = returnFiber.deletions; + const deletions = returnFiber.deletions; if (deletions === null) { - deletions = returnFiber.deletions = [current]; + returnFiber.deletions = [current]; returnFiber.flags |= ChildDeletion; } else { deletions.push(current); } - current.deletions = deletions; newWorkInProgress.flags |= Placement; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index d3082d5f785b8..35115140041c7 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -66,11 +66,18 @@ import { NoFlags, ContentReset, Placement, + PlacementAndUpdate, ChildDeletion, Snapshot, Update, + Callback, + Ref, + Hydrating, + HydratingAndUpdate, Passive, + MutationMask, PassiveMask, + LayoutMask, PassiveUnmountPendingDev, } from './ReactFiberFlags'; import getComponentName from 'shared/getComponentName'; @@ -490,95 +497,154 @@ export function commitPassiveEffectDurations( } } -function commitLifeCycles( +function commitLayoutEffectOnFiber( finishedRoot: FiberRoot, current: Fiber | null, finishedWork: Fiber, committedLanes: Lanes, ): void { - switch (finishedWork.tag) { - case FunctionComponent: - case ForwardRef: - case SimpleMemoComponent: { - // At this point layout effects have already been destroyed (during mutation phase). - // This is done to prevent sibling component effects from interfering with each other, - // e.g. a destroy function in one component should never override a ref set - // by a create function in another component during the same commit. - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { - try { - startLayoutEffectTimer(); + if ((finishedWork.flags & (Update | Callback)) !== NoFlags) { + switch (finishedWork.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + // At this point layout effects have already been destroyed (during mutation phase). + // This is done to prevent sibling component effects from interfering with each other, + // e.g. a destroy function in one component should never override a ref set + // by a create function in another component during the same commit. + if ( + enableProfilerTimer && + enableProfilerCommitHooks && + finishedWork.mode & ProfileMode + ) { + try { + startLayoutEffectTimer(); + commitHookEffectListMount(HookLayout | HookHasEffect, finishedWork); + } finally { + recordLayoutEffectDuration(finishedWork); + } + } else { commitHookEffectListMount(HookLayout | HookHasEffect, finishedWork); - } finally { - recordLayoutEffectDuration(finishedWork); } - } else { - commitHookEffectListMount(HookLayout | HookHasEffect, finishedWork); - } - schedulePassiveEffects(finishedWork); - return; - } - case ClassComponent: { - const instance = finishedWork.stateNode; - if (finishedWork.flags & Update) { - if (current === null) { - // We could update instance props and state here, - // but instead we rely on them being set during last render. - // TODO: revisit this when we implement resuming. - if (__DEV__) { + schedulePassiveEffects(finishedWork); + break; + } + case ClassComponent: { + const instance = finishedWork.stateNode; + if (finishedWork.flags & Update) { + if (current === null) { + // We could update instance props and state here, + // but instead we rely on them being set during last render. + // TODO: revisit this when we implement resuming. + if (__DEV__) { + if ( + finishedWork.type === finishedWork.elementType && + !didWarnAboutReassigningProps + ) { + if (instance.props !== finishedWork.memoizedProps) { + console.error( + 'Expected %s props to match memoized props before ' + + 'componentDidMount. ' + + 'This might either be because of a bug in React, or because ' + + 'a component reassigns its own `this.props`. ' + + 'Please file an issue.', + getComponentName(finishedWork.type) || 'instance', + ); + } + if (instance.state !== finishedWork.memoizedState) { + console.error( + 'Expected %s state to match memoized state before ' + + 'componentDidMount. ' + + 'This might either be because of a bug in React, or because ' + + 'a component reassigns its own `this.state`. ' + + 'Please file an issue.', + getComponentName(finishedWork.type) || 'instance', + ); + } + } + } if ( - finishedWork.type === finishedWork.elementType && - !didWarnAboutReassigningProps + enableProfilerTimer && + enableProfilerCommitHooks && + finishedWork.mode & ProfileMode ) { - if (instance.props !== finishedWork.memoizedProps) { - console.error( - 'Expected %s props to match memoized props before ' + - 'componentDidMount. ' + - 'This might either be because of a bug in React, or because ' + - 'a component reassigns its own `this.props`. ' + - 'Please file an issue.', - getComponentName(finishedWork.type) || 'instance', - ); - } - if (instance.state !== finishedWork.memoizedState) { - console.error( - 'Expected %s state to match memoized state before ' + - 'componentDidMount. ' + - 'This might either be because of a bug in React, or because ' + - 'a component reassigns its own `this.state`. ' + - 'Please file an issue.', - getComponentName(finishedWork.type) || 'instance', - ); + try { + startLayoutEffectTimer(); + instance.componentDidMount(); + } finally { + recordLayoutEffectDuration(finishedWork); } - } - } - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { - try { - startLayoutEffectTimer(); + } else { instance.componentDidMount(); - } finally { - recordLayoutEffectDuration(finishedWork); } } else { - instance.componentDidMount(); + const prevProps = + finishedWork.elementType === finishedWork.type + ? current.memoizedProps + : resolveDefaultProps(finishedWork.type, current.memoizedProps); + const prevState = current.memoizedState; + // We could update instance props and state here, + // but instead we rely on them being set during last render. + // TODO: revisit this when we implement resuming. + if (__DEV__) { + if ( + finishedWork.type === finishedWork.elementType && + !didWarnAboutReassigningProps + ) { + if (instance.props !== finishedWork.memoizedProps) { + console.error( + 'Expected %s props to match memoized props before ' + + 'componentDidUpdate. ' + + 'This might either be because of a bug in React, or because ' + + 'a component reassigns its own `this.props`. ' + + 'Please file an issue.', + getComponentName(finishedWork.type) || 'instance', + ); + } + if (instance.state !== finishedWork.memoizedState) { + console.error( + 'Expected %s state to match memoized state before ' + + 'componentDidUpdate. ' + + 'This might either be because of a bug in React, or because ' + + 'a component reassigns its own `this.state`. ' + + 'Please file an issue.', + getComponentName(finishedWork.type) || 'instance', + ); + } + } + } + if ( + enableProfilerTimer && + enableProfilerCommitHooks && + finishedWork.mode & ProfileMode + ) { + try { + startLayoutEffectTimer(); + instance.componentDidUpdate( + prevProps, + prevState, + instance.__reactInternalSnapshotBeforeUpdate, + ); + } finally { + recordLayoutEffectDuration(finishedWork); + } + } else { + instance.componentDidUpdate( + prevProps, + prevState, + instance.__reactInternalSnapshotBeforeUpdate, + ); + } } - } else { - const prevProps = - finishedWork.elementType === finishedWork.type - ? current.memoizedProps - : resolveDefaultProps(finishedWork.type, current.memoizedProps); - const prevState = current.memoizedState; - // We could update instance props and state here, - // but instead we rely on them being set during last render. - // TODO: revisit this when we implement resuming. + } + + // TODO: I think this is now always non-null by the time it reaches the + // commit phase. Consider removing the type check. + const updateQueue: UpdateQueue< + *, + > | null = (finishedWork.updateQueue: any); + if (updateQueue !== null) { if (__DEV__) { if ( finishedWork.type === finishedWork.elementType && @@ -587,7 +653,7 @@ function commitLifeCycles( if (instance.props !== finishedWork.memoizedProps) { console.error( 'Expected %s props to match memoized props before ' + - 'componentDidUpdate. ' + + 'processing the update queue. ' + 'This might either be because of a bug in React, or because ' + 'a component reassigns its own `this.props`. ' + 'Please file an issue.', @@ -597,7 +663,7 @@ function commitLifeCycles( if (instance.state !== finishedWork.memoizedState) { console.error( 'Expected %s state to match memoized state before ' + - 'componentDidUpdate. ' + + 'processing the update queue. ' + 'This might either be because of a bug in React, or because ' + 'a component reassigns its own `this.state`. ' + 'Please file an issue.', @@ -606,210 +672,166 @@ function commitLifeCycles( } } } - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { - try { - startLayoutEffectTimer(); - instance.componentDidUpdate( - prevProps, - prevState, - instance.__reactInternalSnapshotBeforeUpdate, - ); - } finally { - recordLayoutEffectDuration(finishedWork); - } - } else { - instance.componentDidUpdate( - prevProps, - prevState, - instance.__reactInternalSnapshotBeforeUpdate, - ); - } + // We could update instance props and state here, + // but instead we rely on them being set during last render. + // TODO: revisit this when we implement resuming. + commitUpdateQueue(finishedWork, updateQueue, instance); } + break; } - - // TODO: I think this is now always non-null by the time it reaches the - // commit phase. Consider removing the type check. - const updateQueue: UpdateQueue< - *, - > | null = (finishedWork.updateQueue: any); - if (updateQueue !== null) { - if (__DEV__) { - if ( - finishedWork.type === finishedWork.elementType && - !didWarnAboutReassigningProps - ) { - if (instance.props !== finishedWork.memoizedProps) { - console.error( - 'Expected %s props to match memoized props before ' + - 'processing the update queue. ' + - 'This might either be because of a bug in React, or because ' + - 'a component reassigns its own `this.props`. ' + - 'Please file an issue.', - getComponentName(finishedWork.type) || 'instance', - ); - } - if (instance.state !== finishedWork.memoizedState) { - console.error( - 'Expected %s state to match memoized state before ' + - 'processing the update queue. ' + - 'This might either be because of a bug in React, or because ' + - 'a component reassigns its own `this.state`. ' + - 'Please file an issue.', - getComponentName(finishedWork.type) || 'instance', - ); + case HostRoot: { + // TODO: I think this is now always non-null by the time it reaches the + // commit phase. Consider removing the type check. + const updateQueue: UpdateQueue< + *, + > | null = (finishedWork.updateQueue: any); + if (updateQueue !== null) { + let instance = null; + if (finishedWork.child !== null) { + switch (finishedWork.child.tag) { + case HostComponent: + instance = getPublicInstance(finishedWork.child.stateNode); + break; + case ClassComponent: + instance = finishedWork.child.stateNode; + break; } } + commitUpdateQueue(finishedWork, updateQueue, instance); } - // We could update instance props and state here, - // but instead we rely on them being set during last render. - // TODO: revisit this when we implement resuming. - commitUpdateQueue(finishedWork, updateQueue, instance); + break; } - return; - } - case HostRoot: { - // TODO: I think this is now always non-null by the time it reaches the - // commit phase. Consider removing the type check. - const updateQueue: UpdateQueue< - *, - > | null = (finishedWork.updateQueue: any); - if (updateQueue !== null) { - let instance = null; - if (finishedWork.child !== null) { - switch (finishedWork.child.tag) { - case HostComponent: - instance = getPublicInstance(finishedWork.child.stateNode); - break; - case ClassComponent: - instance = finishedWork.child.stateNode; - break; - } + case HostComponent: { + const instance: Instance = finishedWork.stateNode; + + // Renderers may schedule work to be done after host components are mounted + // (eg DOM renderer may schedule auto-focus for inputs and form controls). + // These effects should only be committed when components are first mounted, + // aka when there is no current/alternate. + if (current === null && finishedWork.flags & Update) { + const type = finishedWork.type; + const props = finishedWork.memoizedProps; + commitMount(instance, type, props, finishedWork); } - commitUpdateQueue(finishedWork, updateQueue, instance); - } - return; - } - case HostComponent: { - const instance: Instance = finishedWork.stateNode; - // Renderers may schedule work to be done after host components are mounted - // (eg DOM renderer may schedule auto-focus for inputs and form controls). - // These effects should only be committed when components are first mounted, - // aka when there is no current/alternate. - if (current === null && finishedWork.flags & Update) { - const type = finishedWork.type; - const props = finishedWork.memoizedProps; - commitMount(instance, type, props, finishedWork); + break; } + case HostText: { + // We have no life-cycles associated with text. + break; + } + case HostPortal: { + // We have no life-cycles associated with portals. + break; + } + case Profiler: { + if (enableProfilerTimer) { + const {onCommit, onRender} = finishedWork.memoizedProps; + const {effectDuration} = finishedWork.stateNode; - return; - } - case HostText: { - // We have no life-cycles associated with text. - return; - } - case HostPortal: { - // We have no life-cycles associated with portals. - return; - } - case Profiler: { - if (enableProfilerTimer) { - const {onCommit, onRender} = finishedWork.memoizedProps; - const {effectDuration} = finishedWork.stateNode; - - const commitTime = getCommitTime(); - - let phase = current === null ? 'mount' : 'update'; - if (enableProfilerNestedUpdatePhase) { - if (isCurrentUpdateNested()) { - phase = 'nested-update'; - } - } + const commitTime = getCommitTime(); - if (typeof onRender === 'function') { - if (enableSchedulerTracing) { - onRender( - finishedWork.memoizedProps.id, - phase, - finishedWork.actualDuration, - finishedWork.treeBaseDuration, - finishedWork.actualStartTime, - commitTime, - finishedRoot.memoizedInteractions, - ); - } else { - onRender( - finishedWork.memoizedProps.id, - phase, - finishedWork.actualDuration, - finishedWork.treeBaseDuration, - finishedWork.actualStartTime, - commitTime, - ); + let phase = current === null ? 'mount' : 'update'; + if (enableProfilerNestedUpdatePhase) { + if (isCurrentUpdateNested()) { + phase = 'nested-update'; + } } - } - if (enableProfilerCommitHooks) { - if (typeof onCommit === 'function') { + if (typeof onRender === 'function') { if (enableSchedulerTracing) { - onCommit( + onRender( finishedWork.memoizedProps.id, phase, - effectDuration, + finishedWork.actualDuration, + finishedWork.treeBaseDuration, + finishedWork.actualStartTime, commitTime, finishedRoot.memoizedInteractions, ); } else { - onCommit( + onRender( finishedWork.memoizedProps.id, phase, - effectDuration, + finishedWork.actualDuration, + finishedWork.treeBaseDuration, + finishedWork.actualStartTime, commitTime, ); } } - // Schedule a passive effect for this Profiler to call onPostCommit hooks. - // This effect should be scheduled even if there is no onPostCommit callback for this Profiler, - // because the effect is also where times bubble to parent Profilers. - enqueuePendingPassiveProfilerEffect(finishedWork); + if (enableProfilerCommitHooks) { + if (typeof onCommit === 'function') { + if (enableSchedulerTracing) { + onCommit( + finishedWork.memoizedProps.id, + phase, + effectDuration, + commitTime, + finishedRoot.memoizedInteractions, + ); + } else { + onCommit( + finishedWork.memoizedProps.id, + phase, + effectDuration, + commitTime, + ); + } + } - // Propagate layout effect durations to the next nearest Profiler ancestor. - // Do not reset these values until the next render so DevTools has a chance to read them first. - let parentFiber = finishedWork.return; - while (parentFiber !== null) { - if (parentFiber.tag === Profiler) { - const parentStateNode = parentFiber.stateNode; - parentStateNode.effectDuration += effectDuration; - break; + // Schedule a passive effect for this Profiler to call onPostCommit hooks. + // This effect should be scheduled even if there is no onPostCommit callback for this Profiler, + // because the effect is also where times bubble to parent Profilers. + enqueuePendingPassiveProfilerEffect(finishedWork); + + // Propagate layout effect durations to the next nearest Profiler ancestor. + // Do not reset these values until the next render so DevTools has a chance to read them first. + let parentFiber = finishedWork.return; + while (parentFiber !== null) { + if (parentFiber.tag === Profiler) { + const parentStateNode = parentFiber.stateNode; + parentStateNode.effectDuration += effectDuration; + break; + } + parentFiber = parentFiber.return; } - parentFiber = parentFiber.return; } } + break; } - return; + case SuspenseComponent: { + commitSuspenseHydrationCallbacks(finishedRoot, finishedWork); + break; + } + case SuspenseListComponent: + case IncompleteClassComponent: + case FundamentalComponent: + case ScopeComponent: + case OffscreenComponent: + case LegacyHiddenComponent: + break; + default: + invariant( + false, + 'This unit of work tag should not have side-effects. This error is ' + + 'likely caused by a bug in React. Please file an issue.', + ); } - case SuspenseComponent: { - commitSuspenseHydrationCallbacks(finishedRoot, finishedWork); - return; + } + + if (enableScopeAPI) { + // TODO: This is a temporary solution that allowed us to transition away + // from React Flare on www. + if (finishedWork.flags & Ref && finishedWork.tag !== ScopeComponent) { + commitAttachRef(finishedWork); + } + } else { + if (finishedWork.flags & Ref) { + commitAttachRef(finishedWork); } - case SuspenseListComponent: - case IncompleteClassComponent: - case FundamentalComponent: - case ScopeComponent: - case OffscreenComponent: - case LegacyHiddenComponent: - return; } - invariant( - false, - 'This unit of work tag should not have side-effects. This error is ' + - 'likely caused by a bug in React. Please file an issue.', - ); } function hideOrUnhideAllChildren(finishedWork, isHidden) { @@ -1084,17 +1106,17 @@ function detachFiberMutation(fiber: Fiber) { // Note that we can't clear child or sibling pointers yet. // They're needed for passive effects and for findDOMNode. // We defer those fields, and all other cleanup, to the passive phase (see detachFiberAfterEffects). - const alternate = fiber.alternate; - if (alternate !== null) { - alternate.return = null; - fiber.alternate = null; - } + // + // Don't reset the alternate yet, either. We need that so we can detach the + // alternate's fields in the passive phase. Clearing the return pointer is + // sufficient for findDOMNode semantics. fiber.return = null; } export function detachFiberAfterEffects(fiber: Fiber): void { // Null out fields to improve GC for references that may be lingering (e.g. DevTools). // Note that we already cleared the return pointer in detachFiberMutation(). + fiber.alternate = null; fiber.child = null; fiber.deletions = null; fiber.dependencies = null; @@ -1823,12 +1845,254 @@ function commitResetTextContent(current: Fiber) { resetTextContent(current.stateNode); } -export function commitPassiveMountEffects( +export function commitMutationEffects( root: FiberRoot, + renderPriorityLevel: ReactPriorityLevel, firstChild: Fiber, -): void { +) { nextEffect = firstChild; - commitPassiveMountEffects_begin(firstChild, root); + commitMutationEffects_begin(root, renderPriorityLevel); +} + +function commitMutationEffects_begin( + root: FiberRoot, + renderPriorityLevel: ReactPriorityLevel, +) { + while (nextEffect !== null) { + const fiber = nextEffect; + + // TODO: Should wrap this in flags check, too, as optimization + const deletions = fiber.deletions; + if (deletions !== null) { + for (let i = 0; i < deletions.length; i++) { + const childToDelete = deletions[i]; + if (__DEV__) { + invokeGuardedCallback( + null, + commitDeletion, + null, + root, + childToDelete, + renderPriorityLevel, + ); + if (hasCaughtError()) { + const error = clearCaughtError(); + captureCommitPhaseError(childToDelete, error); + } + } else { + try { + commitDeletion(root, childToDelete, renderPriorityLevel); + } catch (error) { + captureCommitPhaseError(childToDelete, error); + } + } + } + } + + const child = fiber.child; + if ((fiber.subtreeFlags & MutationMask) !== NoFlags && child !== null) { + ensureCorrectReturnPointer(child, fiber); + nextEffect = child; + } else { + commitMutationEffects_complete(root, renderPriorityLevel); + } + } +} + +function commitMutationEffects_complete( + root: FiberRoot, + renderPriorityLevel: ReactPriorityLevel, +) { + while (nextEffect !== null) { + const fiber = nextEffect; + if (__DEV__) { + setCurrentDebugFiberInDEV(fiber); + invokeGuardedCallback( + null, + commitMutationEffectsOnFiber, + null, + fiber, + root, + renderPriorityLevel, + ); + if (hasCaughtError()) { + const error = clearCaughtError(); + captureCommitPhaseError(fiber, error); + } + resetCurrentDebugFiberInDEV(); + } else { + try { + commitMutationEffectsOnFiber(fiber, root, renderPriorityLevel); + } catch (error) { + captureCommitPhaseError(fiber, error); + } + } + + const sibling = fiber.sibling; + if (sibling !== null) { + ensureCorrectReturnPointer(sibling, fiber.return); + nextEffect = sibling; + return; + } + + nextEffect = fiber.return; + } +} + +function commitMutationEffectsOnFiber( + finishedWork: Fiber, + root: FiberRoot, + renderPriorityLevel: ReactPriorityLevel, +) { + const flags = finishedWork.flags; + + if (flags & ContentReset) { + commitResetTextContent(finishedWork); + } + + if (flags & Ref) { + const current = finishedWork.alternate; + if (current !== null) { + commitDetachRef(current); + } + if (enableScopeAPI) { + // TODO: This is a temporary solution that allowed us to transition away + // from React Flare on www. + if (finishedWork.tag === ScopeComponent) { + commitAttachRef(finishedWork); + } + } + } + + // The following switch statement is only concerned about placement, + // updates, and deletions. To avoid needing to add a case for every possible + // bitmap value, we remove the secondary effects from the effect tag and + // switch on that value. + const primaryFlags = flags & (Placement | Update | Hydrating); + outer: switch (primaryFlags) { + case Placement: { + commitPlacement(finishedWork); + // Clear the "placement" from effect tag so that we know that this is + // inserted, before any life-cycles like componentDidMount gets called. + // TODO: findDOMNode doesn't rely on this any more but isMounted does + // and isMounted is deprecated anyway so we should be able to kill this. + finishedWork.flags &= ~Placement; + break; + } + case PlacementAndUpdate: { + // Placement + commitPlacement(finishedWork); + // Clear the "placement" from effect tag so that we know that this is + // inserted, before any life-cycles like componentDidMount gets called. + finishedWork.flags &= ~Placement; + + // Update + const current = finishedWork.alternate; + commitWork(current, finishedWork); + break; + } + case Hydrating: { + finishedWork.flags &= ~Hydrating; + break; + } + case HydratingAndUpdate: { + finishedWork.flags &= ~Hydrating; + + // Update + const current = finishedWork.alternate; + commitWork(current, finishedWork); + break; + } + case Update: { + const current = finishedWork.alternate; + commitWork(current, finishedWork); + break; + } + } +} + +export function commitLayoutEffects( + finishedWork: Fiber, + root: FiberRoot, + committedLanes: Lanes, +): void { + nextEffect = finishedWork; + commitLayoutEffects_begin(finishedWork, root, committedLanes); +} + +function commitLayoutEffects_begin( + subtreeRoot: Fiber, + root: FiberRoot, + committedLanes: Lanes, +) { + while (nextEffect !== null) { + const fiber = nextEffect; + const firstChild = fiber.child; + if ((fiber.subtreeFlags & LayoutMask) !== NoFlags && firstChild !== null) { + ensureCorrectReturnPointer(firstChild, fiber); + nextEffect = firstChild; + } else { + commitLayoutMountEffects_complete(subtreeRoot, root, committedLanes); + } + } +} + +function commitLayoutMountEffects_complete( + subtreeRoot: Fiber, + root: FiberRoot, + committedLanes: Lanes, +) { + while (nextEffect !== null) { + const fiber = nextEffect; + if ((fiber.flags & LayoutMask) !== NoFlags) { + const current = fiber.alternate; + if (__DEV__) { + setCurrentDebugFiberInDEV(fiber); + invokeGuardedCallback( + null, + commitLayoutEffectOnFiber, + null, + root, + current, + fiber, + committedLanes, + ); + if (hasCaughtError()) { + const error = clearCaughtError(); + captureCommitPhaseError(fiber, error); + } + resetCurrentDebugFiberInDEV(); + } else { + try { + commitLayoutEffectOnFiber(root, current, fiber, committedLanes); + } catch (error) { + captureCommitPhaseError(fiber, error); + } + } + } + + if (fiber === subtreeRoot) { + nextEffect = null; + return; + } + + const sibling = fiber.sibling; + if (sibling !== null) { + ensureCorrectReturnPointer(sibling, fiber.return); + nextEffect = sibling; + return; + } + + nextEffect = fiber.return; + } +} + +export function commitPassiveMountEffects( + root: FiberRoot, + finishedWork: Fiber, +): void { + nextEffect = finishedWork; + commitPassiveMountEffects_begin(finishedWork, root); } function commitPassiveMountEffects_begin(subtreeRoot: Fiber, root: FiberRoot) { @@ -1936,7 +2200,11 @@ function commitPassiveUnmountEffects_begin() { commitPassiveUnmountEffectsInsideOfDeletedTree_begin(fiberToDelete); // Now that passive effects have been processed, it's safe to detach lingering pointers. + const alternate = fiberToDelete.alternate; detachFiberAfterEffects(fiberToDelete); + if (alternate !== null) { + detachFiberAfterEffects(alternate); + } } nextEffect = fiber; } @@ -2005,6 +2273,13 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_begin( ) { while (nextEffect !== null) { const fiber = nextEffect; + + // Deletion effects fire in parent -> child order + // TODO: Check if fiber has a PassiveStatic flag + setCurrentDebugFiberInDEV(fiber); + commitPassiveUnmountInsideDeletedTreeOnFiber(fiber); + resetCurrentDebugFiberInDEV(); + const child = fiber.child; // TODO: Only traverse subtree if it has a PassiveStatic flag if (child !== null) { @@ -2023,11 +2298,6 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_complete( ) { while (nextEffect !== null) { const fiber = nextEffect; - // TODO: Check if fiber has a PassiveStatic flag - setCurrentDebugFiberInDEV(fiber); - commitPassiveUnmountInsideDeletedTreeOnFiber(fiber); - resetCurrentDebugFiberInDEV(); - if (fiber === deletedSubtreeRoot) { nextEffect = null; return; @@ -2088,7 +2358,6 @@ export { commitPlacement, commitDeletion, commitWork, - commitLifeCycles, commitAttachRef, commitDetachRef, }; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 95e48b1e4ae4f..d3bfe78e39479 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -1084,17 +1084,17 @@ function detachFiberMutation(fiber: Fiber) { // Note that we can't clear child or sibling pointers yet. // They're needed for passive effects and for findDOMNode. // We defer those fields, and all other cleanup, to the passive phase (see detachFiberAfterEffects). - const alternate = fiber.alternate; - if (alternate !== null) { - alternate.return = null; - fiber.alternate = null; - } + // + // Don't reset the alternate yet, either. We need that so we can detach the + // alternate's fields in the passive phase. Clearing the return pointer is + // sufficient for findDOMNode semantics. fiber.return = null; } export function detachFiberAfterEffects(fiber: Fiber): void { // Null out fields to improve GC for references that may be lingering (e.g. DevTools). // Note that we already cleared the return pointer in detachFiberMutation(). + fiber.alternate = null; fiber.child = null; fiber.deletions = null; fiber.dependencies = null; @@ -1936,7 +1936,11 @@ function commitPassiveUnmountEffects_begin() { commitPassiveUnmountEffectsInsideOfDeletedTree_begin(fiberToDelete); // Now that passive effects have been processed, it's safe to detach lingering pointers. + const alternate = fiberToDelete.alternate; detachFiberAfterEffects(fiberToDelete); + if (alternate !== null) { + detachFiberAfterEffects(alternate); + } } nextEffect = fiber; } @@ -2005,6 +2009,13 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_begin( ) { while (nextEffect !== null) { const fiber = nextEffect; + + // Deletion effects fire in parent -> child order + // TODO: Check if fiber has a PassiveStatic flag + setCurrentDebugFiberInDEV(fiber); + commitPassiveUnmountInsideDeletedTreeOnFiber(fiber); + resetCurrentDebugFiberInDEV(); + const child = fiber.child; // TODO: Only traverse subtree if it has a PassiveStatic flag if (child !== null) { @@ -2023,11 +2034,6 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_complete( ) { while (nextEffect !== null) { const fiber = nextEffect; - // TODO: Check if fiber has a PassiveStatic flag - setCurrentDebugFiberInDEV(fiber); - commitPassiveUnmountInsideDeletedTreeOnFiber(fiber); - resetCurrentDebugFiberInDEV(); - if (fiber === deletedSubtreeRoot) { nextEffect = null; return; diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js index 37be28be6628c..f416c33b21f1b 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js @@ -144,14 +144,13 @@ function deleteHydratableInstance( returnFiber.firstEffect = returnFiber.lastEffect = childToDelete; } - let deletions = returnFiber.deletions; + const deletions = returnFiber.deletions; if (deletions === null) { - deletions = returnFiber.deletions = [childToDelete]; + returnFiber.deletions = [childToDelete]; returnFiber.flags |= ChildDeletion; } else { deletions.push(childToDelete); } - childToDelete.deletions = deletions; } function insertNonHydratedInstance(returnFiber: Fiber, fiber: Fiber) { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 03831988e2b2b..4eafe42585043 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -31,7 +31,6 @@ import { decoupleUpdatePriorityFromScheduler, enableDebugTracing, enableSchedulingProfiler, - enableScopeAPI, disableSchedulerTimeoutInWorkLoop, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; @@ -116,7 +115,6 @@ import { ForwardRef, MemoComponent, SimpleMemoComponent, - ScopeComponent, Profiler, } from './ReactWorkTags'; import {LegacyRoot} from './ReactRootTags'; @@ -124,20 +122,14 @@ import { NoFlags, PerformedWork, Placement, - Update, - PlacementAndUpdate, Deletion, ChildDeletion, - Ref, - ContentReset, Snapshot, - Callback, Passive, PassiveStatic, Incomplete, HostEffectMask, Hydrating, - HydratingAndUpdate, StaticMask, } from './ReactFiberFlags'; import { @@ -190,14 +182,9 @@ import { } from './ReactFiberThrow.new'; import { commitBeforeMutationLifeCycles as commitBeforeMutationEffectOnFiber, - commitLifeCycles as commitLayoutEffectOnFiber, - commitPlacement, - commitWork, - commitDeletion, - commitDetachRef, - commitAttachRef, + commitLayoutEffects, + commitMutationEffects, commitPassiveEffectDurations, - commitResetTextContent, isSuspenseBoundaryBeingHidden, commitPassiveMountEffects, commitPassiveUnmountEffects, @@ -2032,32 +2019,7 @@ function commitRootImpl(root, renderPriorityLevel) { } // The next phase is the mutation phase, where we mutate the host tree. - nextEffect = firstEffect; - do { - if (__DEV__) { - invokeGuardedCallback( - null, - commitMutationEffects, - null, - root, - renderPriorityLevel, - ); - if (hasCaughtError()) { - invariant(nextEffect !== null, 'Should be working on an effect.'); - const error = clearCaughtError(); - captureCommitPhaseError(nextEffect, error); - nextEffect = nextEffect.nextEffect; - } - } else { - try { - commitMutationEffects(root, renderPriorityLevel); - } catch (error) { - invariant(nextEffect !== null, 'Should be working on an effect.'); - captureCommitPhaseError(nextEffect, error); - nextEffect = nextEffect.nextEffect; - } - } - } while (nextEffect !== null); + commitMutationEffects(root, renderPriorityLevel, finishedWork); if (shouldFireAfterActiveInstanceBlur) { afterActiveInstanceBlur(); @@ -2073,28 +2035,24 @@ function commitRootImpl(root, renderPriorityLevel) { // The next phase is the layout phase, where we call effects that read // the host tree after it's been mutated. The idiomatic use case for this is // layout, but class component lifecycles also fire here for legacy reasons. - nextEffect = firstEffect; - do { - if (__DEV__) { - invokeGuardedCallback(null, commitLayoutEffects, null, root, lanes); - if (hasCaughtError()) { - invariant(nextEffect !== null, 'Should be working on an effect.'); - const error = clearCaughtError(); - captureCommitPhaseError(nextEffect, error); - nextEffect = nextEffect.nextEffect; - } - } else { - try { - commitLayoutEffects(root, lanes); - } catch (error) { - invariant(nextEffect !== null, 'Should be working on an effect.'); - captureCommitPhaseError(nextEffect, error); - nextEffect = nextEffect.nextEffect; - } + if (__DEV__) { + if (enableDebugTracing) { + logLayoutEffectsStarted(lanes); } - } while (nextEffect !== null); + } + if (enableSchedulingProfiler) { + markLayoutEffectsStarted(lanes); + } + commitLayoutEffects(finishedWork, root, lanes); + if (__DEV__) { + if (enableDebugTracing) { + logLayoutEffectsStopped(); + } + } - nextEffect = null; + if (enableSchedulingProfiler) { + markLayoutEffectsStopped(); + } if (enableProfilerTimer && enableProfilerNestedUpdateScheduledHook) { rootCommittingMutationOrLayoutEffects = null; @@ -2148,7 +2106,11 @@ function commitRootImpl(root, renderPriorityLevel) { if (deletions !== null) { for (let i = 0; i < deletions.length; i++) { const deletion = deletions[i]; + const alternate = deletion.alternate; detachFiberAfterEffects(deletion); + if (alternate !== null) { + detachFiberAfterEffects(alternate); + } } } } @@ -2305,166 +2267,6 @@ function commitBeforeMutationEffects() { } } -function commitMutationEffects( - root: FiberRoot, - renderPriorityLevel: ReactPriorityLevel, -) { - // TODO: Should probably move the bulk of this function to commitWork. - while (nextEffect !== null) { - setCurrentDebugFiberInDEV(nextEffect); - - const flags = nextEffect.flags; - - if (flags & ContentReset) { - commitResetTextContent(nextEffect); - } - - if (flags & Ref) { - const current = nextEffect.alternate; - if (current !== null) { - commitDetachRef(current); - } - if (enableScopeAPI) { - // TODO: This is a temporary solution that allowed us to transition away - // from React Flare on www. - if (nextEffect.tag === ScopeComponent) { - commitAttachRef(nextEffect); - } - } - } - - // The following switch statement is only concerned about placement, - // updates, and deletions. To avoid needing to add a case for every possible - // bitmap value, we remove the secondary effects from the effect tag and - // switch on that value. - const primaryFlags = flags & (Placement | Update | Deletion | Hydrating); - outer: switch (primaryFlags) { - case Placement: { - commitPlacement(nextEffect); - // Clear the "placement" from effect tag so that we know that this is - // inserted, before any life-cycles like componentDidMount gets called. - // TODO: findDOMNode doesn't rely on this any more but isMounted does - // and isMounted is deprecated anyway so we should be able to kill this. - nextEffect.flags &= ~Placement; - break; - } - case PlacementAndUpdate: { - // Placement - commitPlacement(nextEffect); - // Clear the "placement" from effect tag so that we know that this is - // inserted, before any life-cycles like componentDidMount gets called. - nextEffect.flags &= ~Placement; - - // Update - const current = nextEffect.alternate; - commitWork(current, nextEffect); - break; - } - case Hydrating: { - nextEffect.flags &= ~Hydrating; - break; - } - case HydratingAndUpdate: { - nextEffect.flags &= ~Hydrating; - - // Update - const current = nextEffect.alternate; - commitWork(current, nextEffect); - break; - } - case Update: { - const current = nextEffect.alternate; - commitWork(current, nextEffect); - break; - } - case Deletion: { - // Reached a deletion effect. Instead of commit this effect like we - // normally do, we're going to use the `deletions` array of the parent. - // However, because the effect list is sorted in depth-first order, we - // can't wait until we reach the parent node, because the child effects - // will have run in the meantime. - // - // So instead, we use a trick where the first time we hit a deletion - // effect, we commit all the deletion effects that belong to that parent. - // - // This is an incremental step away from using the effect list and - // toward a DFS + subtreeFlags traversal. - // - // A reference to the deletion array of the parent is also stored on - // each of the deletions. This is really weird. It would be better to - // follow the `.return` pointer, but unfortunately we can't assume that - // `.return` points to the correct fiber, even in the commit phase, - // because `findDOMNode` might mutate it. - const deletedChild = nextEffect; - const deletions = deletedChild.deletions; - if (deletions !== null) { - for (let i = 0; i < deletions.length; i++) { - const deletion = deletions[i]; - // Clear the deletion effect so that we don't delete this node more - // than once. - deletion.flags &= ~Deletion; - deletion.deletions = null; - commitDeletion(root, deletion, renderPriorityLevel); - } - } - break; - } - } - - resetCurrentDebugFiberInDEV(); - nextEffect = nextEffect.nextEffect; - } -} - -function commitLayoutEffects(root: FiberRoot, committedLanes: Lanes) { - if (__DEV__) { - if (enableDebugTracing) { - logLayoutEffectsStarted(committedLanes); - } - } - - if (enableSchedulingProfiler) { - markLayoutEffectsStarted(committedLanes); - } - - // TODO: Should probably move the bulk of this function to commitWork. - while (nextEffect !== null) { - setCurrentDebugFiberInDEV(nextEffect); - - const flags = nextEffect.flags; - - if (flags & (Update | Callback)) { - const current = nextEffect.alternate; - commitLayoutEffectOnFiber(root, current, nextEffect, committedLanes); - } - - if (enableScopeAPI) { - // TODO: This is a temporary solution that allowed us to transition away - // from React Flare on www. - if (flags & Ref && nextEffect.tag !== ScopeComponent) { - commitAttachRef(nextEffect); - } - } else { - if (flags & Ref) { - commitAttachRef(nextEffect); - } - } - - resetCurrentDebugFiberInDEV(); - nextEffect = nextEffect.nextEffect; - } - - if (__DEV__) { - if (enableDebugTracing) { - logLayoutEffectsStopped(); - } - } - - if (enableSchedulingProfiler) { - markLayoutEffectsStopped(); - } -} - export function flushPassiveEffects(): boolean { // Returns whether passive effects were flushed. if (pendingPassiveEffectsRenderPriority !== NoSchedulerPriority) { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 6966472e0d16f..caeab7ce67330 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -2148,7 +2148,11 @@ function commitRootImpl(root, renderPriorityLevel) { if (deletions !== null) { for (let i = 0; i < deletions.length; i++) { const deletion = deletions[i]; + const alternate = deletion.alternate; detachFiberAfterEffects(deletion); + if (alternate !== null) { + detachFiberAfterEffects(alternate); + } } } } diff --git a/packages/react-reconciler/src/__tests__/ReactEffectOrdering-test.js b/packages/react-reconciler/src/__tests__/ReactEffectOrdering-test.js new file mode 100644 index 0000000000000..6e9ffcfb894f5 --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactEffectOrdering-test.js @@ -0,0 +1,86 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + * @jest-environment node + */ + +/* eslint-disable no-func-assign */ + +'use strict'; + +let React; +let ReactNoop; +let Scheduler; +let useEffect; +let useLayoutEffect; + +describe('ReactHooksWithNoopRenderer', () => { + beforeEach(() => { + jest.resetModules(); + jest.useFakeTimers(); + + React = require('react'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + useEffect = React.useEffect; + useLayoutEffect = React.useLayoutEffect; + }); + + test('layout unmmouts on deletion are fired in parent -> child order', async () => { + const root = ReactNoop.createRoot(); + + function Parent() { + useLayoutEffect(() => { + return () => Scheduler.unstable_yieldValue('Unmount parent'); + }); + return ; + } + + function Child() { + useLayoutEffect(() => { + return () => Scheduler.unstable_yieldValue('Unmount child'); + }); + return 'Child'; + } + + await ReactNoop.act(async () => { + root.render(); + }); + expect(root).toMatchRenderedOutput('Child'); + await ReactNoop.act(async () => { + root.render(null); + }); + expect(Scheduler).toHaveYielded(['Unmount parent', 'Unmount child']); + }); + + test('passive unmmouts on deletion are fired in parent -> child order', async () => { + const root = ReactNoop.createRoot(); + + function Parent() { + useEffect(() => { + return () => Scheduler.unstable_yieldValue('Unmount parent'); + }); + return ; + } + + function Child() { + useEffect(() => { + return () => Scheduler.unstable_yieldValue('Unmount child'); + }); + return 'Child'; + } + + await ReactNoop.act(async () => { + root.render(); + }); + expect(root).toMatchRenderedOutput('Child'); + await ReactNoop.act(async () => { + root.render(null); + }); + expect(Scheduler).toHaveYielded(['Unmount parent', 'Unmount child']); + }); +}); diff --git a/packages/react/src/__tests__/ReactDOMTracing-test.internal.js b/packages/react/src/__tests__/ReactDOMTracing-test.internal.js index 5026605672fcb..bd097fea76548 100644 --- a/packages/react/src/__tests__/ReactDOMTracing-test.internal.js +++ b/packages/react/src/__tests__/ReactDOMTracing-test.internal.js @@ -152,7 +152,7 @@ describe('ReactDOMTracing', () => { onInteractionScheduledWorkCompleted, ).toHaveBeenLastNotifiedOfInteraction(interaction); - if (gate(flags => flags.dfsEffectsRefactor)) { + if (gate(flags => flags.enableUseJSStackToTrackPassiveDurations)) { expect(onRender).toHaveBeenCalledTimes(3); } else { // TODO: This is 4 instead of 3 because this update was scheduled at @@ -310,7 +310,7 @@ describe('ReactDOMTracing', () => { expect( onInteractionScheduledWorkCompleted, ).toHaveBeenLastNotifiedOfInteraction(interaction); - if (gate(flags => flags.dfsEffectsRefactor)) { + if (gate(flags => flags.enableUseJSStackToTrackPassiveDurations)) { expect(onRender).toHaveBeenCalledTimes(3); } else { // TODO: This is 4 instead of 3 because this update was scheduled at diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index eb0c69537df4b..8387319511b55 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -368,7 +368,7 @@ describe('Profiler', () => { renderer.update(); - if (gate(flags => flags.dfsEffectsRefactor)) { + if (gate(flags => flags.enableUseJSStackToTrackPassiveDurations)) { // None of the Profiler's subtree was rendered because App bailed out before the Profiler. // So we expect onRender not to be called. expect(callback).not.toHaveBeenCalled(); @@ -4383,7 +4383,7 @@ describe('Profiler', () => { // because the resolved suspended subtree doesn't contain any passive effects. // If or its decendents had a passive effect, // onPostCommit would be called again. - if (gate(flags => flags.dfsEffectsRefactor)) { + if (gate(flags => flags.enableUseJSStackToTrackPassiveDurations)) { expect(Scheduler).toFlushAndYield([]); } else { expect(Scheduler).toFlushAndYield(['onPostCommit']); @@ -4874,7 +4874,7 @@ describe('Profiler', () => { }); if (__DEV__) { - // @gate dfsEffectsRefactor + // @gate enableUseJSStackToTrackPassiveDurations // @gate enableDoubleInvokingEffects it('double invoking does not disconnect wrapped async work', () => { ReactFeatureFlags.enableDoubleInvokingEffects = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 16e488409e0c4..c6ffacb27bab2 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -23,7 +23,7 @@ export const enableSchedulerTracing = __PROFILE__; export const enableSuspenseServerRenderer = false; export const enableSelectiveHydration = false; export const enableLazyElements = false; -export const enableCache = false; +export const enableCache = __EXPERIMENTAL__; export const disableJavaScriptURLs = false; export const disableInputAttributeSyncing = false; export const enableSchedulerDebugging = false; diff --git a/scripts/jest/TestFlags.js b/scripts/jest/TestFlags.js index f869620f03290..d7e82312dd2a5 100644 --- a/scripts/jest/TestFlags.js +++ b/scripts/jest/TestFlags.js @@ -45,9 +45,10 @@ const environmentFlags = { // Use this for tests that are known to be broken. FIXME: false, - // Turn this flag back on (or delete) once the effect list is removed in favor - // of a depth-first traversal using `subtreeTags`. - dfsEffectsRefactor: false, + // Turn these flags back on (or delete) once the effect list is removed in + // favor of a depth-first traversal using `subtreeTags`. + dfsEffectsRefactor: __VARIANT__, + enableUseJSStackToTrackPassiveDurations: false, }; function getTestFlags() { diff --git a/scripts/jest/config.build-devtools.js b/scripts/jest/config.build-devtools.js index 39985e7416e18..761479ce45c50 100644 --- a/scripts/jest/config.build-devtools.js +++ b/scripts/jest/config.build-devtools.js @@ -4,6 +4,9 @@ const {readdirSync, statSync} = require('fs'); const {join} = require('path'); const baseConfig = require('./config.base'); +const NODE_MODULES_DIR = + process.env.RELEASE_CHANNEL === 'stable' ? 'oss-stable' : 'oss-experimental'; + // Find all folders in packages/* with package.json const packagesRoot = join(__dirname, '..', '..', 'packages'); const packages = readdirSync(packagesRoot).filter(dir => { @@ -32,11 +35,13 @@ moduleNameMapper['react-devtools-feature-flags'] = // Map packages to bundles packages.forEach(name => { // Root entry point - moduleNameMapper[`^${name}$`] = `/build/node_modules/${name}`; + moduleNameMapper[ + `^${name}$` + ] = `/build2/${NODE_MODULES_DIR}/${name}`; // Named entry points moduleNameMapper[ `^${name}\/([^\/]+)$` - ] = `/build/node_modules/${name}/$1`; + ] = `/build2/${NODE_MODULES_DIR}/${name}/$1`; }); // Allow tests to import shared code (e.g. feature flags, getStackByFiberInDevAndProd) @@ -50,9 +55,12 @@ module.exports = Object.assign({}, baseConfig, { // Don't run bundle tests on -test.internal.* files testPathIgnorePatterns: ['/node_modules/', '-test.internal.js$'], // Exclude the build output from transforms - transformIgnorePatterns: ['/node_modules/', '/build/'], + transformIgnorePatterns: ['/node_modules/', '/build2/'], testRegex: 'packages/react-devtools-shared/src/__tests__/[^]+.test.js$', snapshotSerializers: [ + require.resolve( + '../../packages/react-devtools-shared/src/__tests__/dehydratedValueSerializer.js' + ), require.resolve( '../../packages/react-devtools-shared/src/__tests__/inspectedElementSerializer.js' ), diff --git a/scripts/jest/config.build.js b/scripts/jest/config.build.js index fe83740fa4fe7..95bc0e633ce5e 100644 --- a/scripts/jest/config.build.js +++ b/scripts/jest/config.build.js @@ -6,6 +6,9 @@ const baseConfig = require('./config.base'); process.env.IS_BUILD = true; +const NODE_MODULES_DIR = + process.env.RELEASE_CHANNEL === 'stable' ? 'oss-stable' : 'oss-experimental'; + // Find all folders in packages/* with package.json const packagesRoot = join(__dirname, '..', '..', 'packages'); const packages = readdirSync(packagesRoot).filter(dir => { @@ -35,11 +38,13 @@ moduleNameMapper[ // Map packages to bundles packages.forEach(name => { // Root entry point - moduleNameMapper[`^${name}$`] = `/build/node_modules/${name}`; + moduleNameMapper[ + `^${name}$` + ] = `/build2/${NODE_MODULES_DIR}/${name}`; // Named entry points moduleNameMapper[ `^${name}\/([^\/]+)$` - ] = `/build/node_modules/${name}/$1`; + ] = `/build2/${NODE_MODULES_DIR}/${name}/$1`; }); module.exports = Object.assign({}, baseConfig, { @@ -52,7 +57,7 @@ module.exports = Object.assign({}, baseConfig, { // Don't run bundle tests on -test.internal.* files testPathIgnorePatterns: ['/node_modules/', '-test.internal.js$'], // Exclude the build output from transforms - transformIgnorePatterns: ['/node_modules/', '/build/'], + transformIgnorePatterns: ['/node_modules/', '/build2/'], setupFiles: [ ...baseConfig.setupFiles, require.resolve('./setupTests.build.js'), diff --git a/scripts/jest/jest-cli.js b/scripts/jest/jest-cli.js index 19e3879c776a8..76fdd7e67581a 100644 --- a/scripts/jest/jest-cli.js +++ b/scripts/jest/jest-cli.js @@ -212,10 +212,10 @@ function validateOptions() { if (argv.build) { // TODO: We could build this if it hasn't been built yet. - const buildDir = path.resolve('./build'); + const buildDir = path.resolve('./build2'); if (!fs.existsSync(buildDir)) { logError( - 'Build directory does not exist, please run `yarn build` or remove the --build option.' + 'Build directory does not exist, please run `yarn build-combined` or remove the --build option.' ); success = false; } else if (Date.now() - fs.statSync(buildDir).mtimeMs > 1000 * 60 * 15) { diff --git a/scripts/jest/preprocessor.js b/scripts/jest/preprocessor.js index f57005c9407fa..072071be07fbd 100644 --- a/scripts/jest/preprocessor.js +++ b/scripts/jest/preprocessor.js @@ -59,6 +59,10 @@ const babelOptions = { module.exports = { process: function(src, filePath) { + if (filePath.match(/\.css$/)) { + // Don't try to parse CSS modules; they aren't needed for tests anyway. + return ''; + } if (filePath.match(/\.coffee$/)) { return coffee.compile(src, {bare: true}); } diff --git a/scripts/release/prepare-release-from-ci.js b/scripts/release/prepare-release-from-ci.js index f7222ba8ca8a0..dd8e143453f79 100755 --- a/scripts/release/prepare-release-from-ci.js +++ b/scripts/release/prepare-release-from-ci.js @@ -3,8 +3,7 @@ 'use strict'; const {join} = require('path'); -const {readJsonSync} = require('fs-extra'); -const {getPublicPackages, handleError} = require('./utils'); +const {handleError} = require('./utils'); const checkEnvironmentVariables = require('./shared-commands/check-environment-variables'); const downloadBuildArtifacts = require('./shared-commands/download-build-artifacts'); @@ -26,11 +25,6 @@ const run = async () => { await checkEnvironmentVariables(params); await downloadBuildArtifacts(params); - const version = readJsonSync('./build/node_modules/react/package.json') - .version; - const isExperimental = version.includes('experimental'); - params.packages = await getPublicPackages(isExperimental); - if (!params.skipTests) { await testPackagingFixture(params); await testTracingFixture(params); diff --git a/scripts/release/publish.js b/scripts/release/publish.js index e51df5c45f8ab..9ec9330a0973e 100755 --- a/scripts/release/publish.js +++ b/scripts/release/publish.js @@ -48,8 +48,22 @@ const run = async () => { await confirmVersionAndTags(params); await validateSkipPackages(params); await checkNPMPermissions(params); - const otp = await promptForOTP(params); - await publishToNPM(params, otp); + + while (true) { + try { + const otp = await promptForOTP(params); + await publishToNPM(params, otp); + break; + } catch (error) { + console.error(error.message); + console.log(); + console.log( + theme.error`Publish failed. Enter a fresh otp code to retry.` + ); + continue; + } + } + await updateStableVersionNumbers(params); await printFollowUpInstructions(params); } catch (error) { diff --git a/scripts/release/shared-commands/download-build-artifacts.js b/scripts/release/shared-commands/download-build-artifacts.js index 7158341934407..4d5259e24d1c7 100644 --- a/scripts/release/shared-commands/download-build-artifacts.js +++ b/scripts/release/shared-commands/download-build-artifacts.js @@ -3,65 +3,56 @@ 'use strict'; const {exec} = require('child-process-promise'); -const {existsSync, readdirSync} = require('fs'); -const {readJsonSync} = require('fs-extra'); +const {existsSync} = require('fs'); const {join} = require('path'); const {getArtifactsList, logPromise} = require('../utils'); const theme = require('../theme'); -const run = async ({build, cwd}) => { +const run = async ({build, cwd, releaseChannel}) => { const artifacts = await getArtifactsList(build); - const nodeModulesArtifact = artifacts.find(entry => - entry.path.endsWith('node_modules.tgz') + const buildArtifacts = artifacts.find(entry => + entry.path.endsWith('build2.tgz') ); - if (!nodeModulesArtifact) { + if (!buildArtifacts) { console.log( theme`{error The specified build (${build}) does not contain any build artifacts.}` ); process.exit(1); } - const nodeModulesURL = nodeModulesArtifact.url; + // Download and extract artifact + await exec(`rm -rf ./build2`, {cwd}); + await exec( + `curl -L $(fwdproxy-config curl) ${buildArtifacts.url} | tar -xvz`, + { + cwd, + } + ); + // Copy to staging directory + // TODO: Consider staging the release in a different directory from the CI + // build artifacts: `./build/node_modules` -> `./staged-releases` if (!existsSync(join(cwd, 'build'))) { await exec(`mkdir ./build`, {cwd}); + } else { + await exec(`rm -rf ./build/node_modules`, {cwd}); } - - // Download and extract artifact - await exec(`rm -rf ./build/node_modules*`, {cwd}); - await exec(`curl -L ${nodeModulesURL} --output ./build/node_modules.tgz`, { - cwd, - }); - await exec(`mkdir ./build/node_modules`, {cwd}); - await exec(`tar zxvf ./build/node_modules.tgz -C ./build/node_modules/`, { - cwd, - }); - - // Unpack packages and prepare to publish - const compressedPackages = readdirSync(join(cwd, 'build/node_modules/')); - for (let i = 0; i < compressedPackages.length; i++) { - await exec( - `tar zxvf ./build/node_modules/${compressedPackages[i]} -C ./build/node_modules/`, - {cwd} - ); - const packageJSON = readJsonSync( - join(cwd, `/build/node_modules/package/package.json`) - ); - await exec( - `mv ./build/node_modules/package ./build/node_modules/${packageJSON.name}`, - {cwd} - ); + let sourceDir; + if (releaseChannel === 'stable') { + sourceDir = 'oss-stable'; + } else if (releaseChannel === 'experimental') { + sourceDir = 'oss-experimental'; + } else { + console.error('Internal error: Invalid release channel: ' + releaseChannel); + process.exit(releaseChannel); } - - // Cleanup - await exec(`rm ./build/node_modules.tgz`, {cwd}); - await exec(`rm ./build/node_modules/*.tgz`, {cwd}); + await exec(`cp -r ./build2/${sourceDir} ./build/node_modules`, {cwd}); }; -module.exports = async ({build, cwd}) => { +module.exports = async ({build, cwd, releaseChannel}) => { return logPromise( - run({build, cwd}), + run({build, cwd, releaseChannel}), theme`Downloading artifacts from Circle CI for build {build ${build}}` ); }; diff --git a/scripts/release/shared-commands/get-latest-master-build-number.js b/scripts/release/shared-commands/get-latest-master-build-number.js index 1d1787fad8f04..d0bcd56e2e042 100644 --- a/scripts/release/shared-commands/get-latest-master-build-number.js +++ b/scripts/release/shared-commands/get-latest-master-build-number.js @@ -5,11 +5,7 @@ const http = require('request-promise-json'); const {logPromise} = require('../utils'); -const run = async useExperimentalBuild => { - const targetJobName = useExperimentalBuild - ? 'process_artifacts_experimental' - : 'process_artifacts'; - +const run = async () => { // https://circleci.com/docs/api/#recent-builds-for-a-project-branch const metadataURL = `https://circleci.com/api/v1.1/project/github/facebook/react/tree/master`; const metadata = await http.get(metadataURL, true); @@ -17,7 +13,7 @@ const run = async useExperimentalBuild => { entry => entry.branch === 'master' && entry.status === 'success' && - entry.workflows.job_name === targetJobName + entry.workflows.job_name === 'process_artifacts_combined' ).build_num; return build; diff --git a/scripts/release/shared-commands/parse-params.js b/scripts/release/shared-commands/parse-params.js index 703fc0fb11b40..2ec21020282fa 100644 --- a/scripts/release/shared-commands/parse-params.js +++ b/scripts/release/shared-commands/parse-params.js @@ -17,10 +17,24 @@ const paramDefinitions = [ description: 'Skip automated fixture tests.', defaultValue: false, }, + { + name: 'releaseChannel', + alias: 'r', + type: String, + description: 'Release channel (stable or experimental)', + }, ]; module.exports = () => { const params = commandLineArgs(paramDefinitions); + const channel = params.releaseChannel; + if (channel !== 'experimental' && channel !== 'stable') { + console.error( + `Invalid release channel (-r) "${channel}". Must be "stable" or "experimental".` + ); + process.exit(1); + } + return params; }; diff --git a/scripts/release/utils.js b/scripts/release/utils.js index be191978df3f4..52731d4376e0f 100644 --- a/scripts/release/utils.js +++ b/scripts/release/utils.js @@ -41,11 +41,7 @@ const getArtifactsList = async buildID => { ); process.exit(1); } - const artifactsJobName = buildMetadata.workflows.job_name.endsWith( - '_experimental' - ) - ? 'process_artifacts_experimental' - : 'process_artifacts'; + const artifactsJobName = 'process_artifacts_combined'; const workflowID = buildMetadata.workflows.workflow_id; const workflowMetadataURL = `https://circleci.com/api/v2/workflow/${workflowID}/job?circle-token=${process.env.CIRCLE_CI_API_TOKEN}`; const workflowMetadata = await http.get(workflowMetadataURL, true); diff --git a/scripts/rollup/build-all-release-channels.js b/scripts/rollup/build-all-release-channels.js new file mode 100644 index 0000000000000..0b74e4c42d8e9 --- /dev/null +++ b/scripts/rollup/build-all-release-channels.js @@ -0,0 +1,183 @@ +'use strict'; + +/* eslint-disable no-for-of-loops/no-for-of-loops */ + +const fs = require('fs'); +const {spawnSync} = require('child_process'); +const path = require('path'); +const tmp = require('tmp'); + +// Runs the build script for both stable and experimental release channels, +// by configuring an environment variable. + +const sha = ( + spawnSync('git', ['show', '-s', '--format=%h']).stdout + '' +).trim(); +const ReactVersion = JSON.parse(fs.readFileSync('packages/react/package.json')) + .version; + +if (process.env.CIRCLE_NODE_TOTAL) { + // In CI, we use multiple concurrent processes. Allocate half the processes to + // build the stable channel, and the other half for experimental. Override + // the environment variables to "trick" the underlying build script. + const total = parseInt(process.env.CIRCLE_NODE_TOTAL, 10); + const halfTotal = Math.floor(total / 2); + const index = parseInt(process.env.CIRCLE_NODE_INDEX, 10); + if (index < halfTotal) { + const nodeTotal = halfTotal; + const nodeIndex = index; + const version = '0.0.0-' + sha; + updateTheReactVersionThatDevToolsReads(ReactVersion + '-' + sha); + buildForChannel('stable', nodeTotal, nodeIndex); + processStable('./build', version); + } else { + const nodeTotal = total - halfTotal; + const nodeIndex = index - halfTotal; + const version = '0.0.0-experimental-' + sha; + updateTheReactVersionThatDevToolsReads( + ReactVersion + '-experimental-' + sha + ); + buildForChannel('experimental', nodeTotal, nodeIndex); + processExperimental('./build', version); + } + + // TODO: Currently storing artifacts as `./build2` so that it doesn't conflict + // with old build job. Remove once we migrate rest of build/test pipeline. + fs.renameSync('./build', './build2'); +} else { + // Running locally, no concurrency. Move each channel's build artifacts into + // a temporary directory so that they don't conflict. + const stableVersion = '0.0.0-' + sha; + buildForChannel('stable', '', ''); + const stableDir = tmp.dirSync().name; + fs.renameSync('./build', stableDir); + processStable(stableDir, stableVersion); + + const experimentalVersion = '0.0.0-experimental-' + sha; + buildForChannel('experimental', '', ''); + const experimentalDir = tmp.dirSync().name; + fs.renameSync('./build', experimentalDir); + processExperimental(experimentalDir, experimentalVersion); + + // Then merge the experimental folder into the stable one. processExperimental + // will have already removed conflicting files. + // + // In CI, merging is handled automatically by CircleCI's workspace feature. + spawnSync('rsync', ['-ar', experimentalDir + '/', stableDir + '/']); + + // Now restore the combined directory back to its original name + // TODO: Currently storing artifacts as `./build2` so that it doesn't conflict + // with old build job. Remove once we migrate rest of build/test pipeline. + fs.renameSync(stableDir, './build2'); +} + +function buildForChannel(channel, nodeTotal, nodeIndex) { + spawnSync('node', ['./scripts/rollup/build.js', ...process.argv.slice(2)], { + stdio: ['pipe', process.stdout, process.stderr], + env: { + ...process.env, + RELEASE_CHANNEL: channel, + CIRCLE_NODE_TOTAL: nodeTotal, + CIRCLE_NODE_INDEX: nodeIndex, + }, + }); +} + +function processStable(buildDir, version) { + if (fs.existsSync(buildDir + '/node_modules')) { + updatePackageVersions(buildDir + '/node_modules', version); + fs.renameSync(buildDir + '/node_modules', buildDir + '/oss-stable'); + } + + if (fs.existsSync(buildDir + '/facebook-www')) { + for (const fileName of fs.readdirSync(buildDir + '/facebook-www')) { + const filePath = buildDir + '/facebook-www/' + fileName; + const stats = fs.statSync(filePath); + if (!stats.isDirectory()) { + fs.renameSync(filePath, filePath.replace('.js', '.classic.js')); + } + } + } + + if (fs.existsSync(buildDir + '/sizes')) { + fs.renameSync(buildDir + '/sizes', buildDir + '/sizes-stable'); + } +} + +function processExperimental(buildDir, version) { + if (fs.existsSync(buildDir + '/node_modules')) { + updatePackageVersions(buildDir + '/node_modules', version); + fs.renameSync(buildDir + '/node_modules', buildDir + '/oss-experimental'); + } + + if (fs.existsSync(buildDir + '/facebook-www')) { + for (const fileName of fs.readdirSync(buildDir + '/facebook-www')) { + const filePath = buildDir + '/facebook-www/' + fileName; + const stats = fs.statSync(filePath); + if (!stats.isDirectory()) { + fs.renameSync(filePath, filePath.replace('.js', '.modern.js')); + } + } + } + + if (fs.existsSync(buildDir + '/sizes')) { + fs.renameSync(buildDir + '/sizes', buildDir + '/sizes-experimental'); + } + + // Delete all other artifacts that weren't handled above. We assume they are + // duplicates of the corresponding artifacts in the stable channel. Ideally, + // the underlying build script should not have produced these files in the + // first place. + for (const pathName of fs.readdirSync(buildDir)) { + if ( + pathName !== 'oss-experimental' && + pathName !== 'facebook-www' && + pathName !== 'sizes-experimental' + ) { + spawnSync('rm', ['-rm', buildDir + '/' + pathName]); + } + } +} + +function updatePackageVersions(modulesDir, version) { + const allReactModuleNames = fs.readdirSync('packages'); + for (const moduleName of fs.readdirSync(modulesDir)) { + const packageJSONPath = path.join(modulesDir, moduleName, 'package.json'); + const stats = fs.statSync(packageJSONPath); + if (stats.isFile()) { + const packageInfo = JSON.parse(fs.readFileSync(packageJSONPath)); + + // Update version + packageInfo.version = version; + + // Update dependency versions + if (packageInfo.dependencies) { + for (const dep of Object.keys(packageInfo.dependencies)) { + if (allReactModuleNames.includes(dep)) { + packageInfo.dependencies[dep] = version; + } + } + } + if (packageInfo.peerDependencies) { + for (const dep of Object.keys(packageInfo.peerDependencies)) { + if (allReactModuleNames.includes(dep)) { + packageInfo.peerDependencies[dep] = version; + } + } + } + + // Write out updated package.json + fs.writeFileSync(packageJSONPath, JSON.stringify(packageInfo, null, 2)); + } + } +} + +function updateTheReactVersionThatDevToolsReads(version) { + // Overwrite the ReactVersion module before the build script runs so that it + // is included in the final bundles. This only runs in CI, so it's fine to + // edit the source file. + fs.writeFileSync( + './packages/shared/ReactVersion.js', + `export default '${version}';\n` + ); +} diff --git a/yarn.lock b/yarn.lock index 7b6cb08909628..9daccaf78bb36 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5413,10 +5413,10 @@ electron-to-chromium@^1.3.523: resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.3.538.tgz#15226638ee9db5d8e74f4c860cef6078d8e1e871" integrity sha512-rlyYXLlOoZkJuvY4AJXUpP7CHRVtwZz311HPVoEO1UHo/kqDCsP1pNas0A9paZuPEiYGdLwrjllF2hs69NEaTw== -electron@^9.1.0: - version "9.1.0" - resolved "https://registry.yarnpkg.com/electron/-/electron-9.1.0.tgz#ca77600c9e4cd591298c340e013384114d3d8d05" - integrity sha512-VRAF8KX1m0py9I9sf0kw1kWfeC87mlscfFcbcRdLBsNJ44/GrJhi3+E8rKbpHUeZNQxsPaVA5Zu5Lxb6dV/scQ== +electron@^11.1.0: + version "11.1.1" + resolved "https://registry.yarnpkg.com/electron/-/electron-11.1.1.tgz#188f036f8282798398dca9513e9bb3b10213e3aa" + integrity sha512-tlbex3xosJgfileN6BAQRotevPRXB/wQIq48QeQ08tUJJrXwE72c8smsM/hbHx5eDgnbfJ2G3a60PmRjHU2NhA== dependencies: "@electron/get" "^1.0.1" "@types/node" "^12.0.12"