From bb77ec5aad776d8cea4e42bb277ed32e2af6b721 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 17 May 2021 23:52:23 -0400 Subject: [PATCH] Alternate fix for DevTools+Refres bug --- .../FastRefreshDevToolsIntegration-test.js | 74 ++- .../__snapshots__/profilingCache-test.js.snap | 510 +++++++++--------- .../profilingCommitTreeBuilder-test.js.snap | 12 +- .../src/__tests__/setupTests.js | 39 ++ .../src/backend/legacy/renderer.js | 6 - .../src/backend/renderer.js | 255 ++++----- .../src/backend/types.js | 8 - .../views/ErrorBoundary/ErrorBoundary.js | 20 +- .../devtools/views/ErrorBoundary/ErrorView.js | 12 +- .../devtools/views/ErrorBoundary/shared.css | 18 +- packages/react-devtools-shared/src/hook.js | 8 - .../src/ReactFiberBeginWork.new.js | 30 +- .../src/ReactFiberBeginWork.old.js | 30 +- .../src/ReactFiberDevToolsHook.new.js | 25 - .../src/ReactFiberDevToolsHook.old.js | 25 - .../src/ReactFiberHotReloading.new.js | 1 - .../src/ReactFiberHotReloading.old.js | 1 - 17 files changed, 552 insertions(+), 522 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js b/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js index 279390c5fca8b..4e2cdecfb9e67 100644 --- a/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js +++ b/packages/react-devtools-shared/src/__tests__/FastRefreshDevToolsIntegration-test.js @@ -131,25 +131,48 @@ describe('Fast Refresh', () => { }; function Child() { - return null; + return
; }; export default Parent; `); + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + + `); + + let element = container.firstChild; + expect(container.firstChild).not.toBe(null); + + patch(` + function Parent() { + return ; + }; + + function Child() { + return
; + }; + export default Parent; + `); expect(store).toMatchInlineSnapshot(` [root] ▾ `); + // State is preserved; this verifies that Fast Refresh is wired up. + expect(container.firstChild).toBe(element); + element = container.firstChild; + patch(` function Parent() { return ; }; function Child() { - return null; + return
; }; export default Parent; @@ -159,6 +182,9 @@ describe('Fast Refresh', () => { ▾ `); + + // State is reset because hooks changed. + expect(container.firstChild).not.toBe(element); }); it('should not break when there are warnings in between patching', () => { @@ -168,10 +194,8 @@ describe('Fast Refresh', () => { export default function Component() { const [state, setState] = useState(1); - console.warn("Expected warning during render"); - - return
; + return null; } `); }); @@ -181,56 +205,56 @@ describe('Fast Refresh', () => { ⚠ `); - let element = container.firstChild; - withErrorsOrWarningsIgnored(['Expected warning during render'], () => { patch(` - const {useState} = React; + const {useEffect, useState} = React; export default function Component() { const [state, setState] = useState(1); - console.warn("Expected warning during render"); - - return
; + return null; } `); }); - - // This is the same component type, so the warning count carries over. expect(store).toMatchInlineSnapshot(` ✕ 0, ⚠ 2 [root] ⚠ `); - // State is preserved; this verifies that Fast Refresh is wired up. - expect(container.firstChild).toBe(element); - element = container.firstChild; - withErrorsOrWarningsIgnored(['Expected warning during render'], () => { patch(` const {useEffect, useState} = React; export default function Component() { - const [state, setState] = useState(3); + const [state, setState] = useState(1); useEffect(() => {}); - console.warn("Expected warning during render"); - - return
; + return null; } `); }); - - // This is a new component type, so the warning count has been reset. expect(store).toMatchInlineSnapshot(` ✕ 0, ⚠ 1 [root] ⚠ `); - // State is reset because hooks changed. - expect(container.firstChild).not.toBe(element); + withErrorsOrWarningsIgnored(['Expected warning during render'], () => { + patch(` + const {useEffect, useState} = React; + + export default function Component() { + const [state, setState] = useState(1); + console.warn("Expected warning during render"); + return null; + } + `); + }); + expect(store).toMatchInlineSnapshot(` + ✕ 0, ⚠ 1 + [root] + ⚠ + `); }); }); diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap index ad1d487ea790c..8f07adaaf4dae 100644 --- a/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap +++ b/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap @@ -10,14 +10,14 @@ Object { "props": null, "state": null, }, - 3 => Object { + 4 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, "props": null, "state": null, }, - 5 => Object { + 6 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, @@ -30,14 +30,14 @@ Object { "fiberActualDurations": Map { 1 => 16, 2 => 16, - 3 => 1, - 5 => 1, + 4 => 1, + 6 => 1, }, "fiberSelfDurations": Map { 1 => 0, 2 => 10, - 3 => 1, - 5 => 1, + 4 => 1, + 6 => 1, }, "passiveEffectDuration": null, "priorityLevel": "Normal", @@ -104,7 +104,7 @@ Object { exports[`ProfilingCache should calculate self duration correctly for suspended views: CommitDetails with filtered self durations 2`] = ` Object { "changeDescriptions": Map { - 5 => Object { + 7 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, @@ -115,11 +115,11 @@ Object { "duration": 3, "effectDuration": null, "fiberActualDurations": Map { - 5 => 3, + 7 => 3, 3 => 3, }, "fiberSelfDurations": Map { - 5 => 3, + 7 => 3, 3 => 0, }, "passiveEffectDuration": null, @@ -147,21 +147,21 @@ Object { "props": null, "state": null, }, - 3 => Object { + 4 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, "props": null, "state": null, }, - 4 => Object { + 5 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, "props": null, "state": null, }, - 5 => Object { + 6 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, @@ -174,16 +174,16 @@ Object { "fiberActualDurations": Map { 1 => 12, 2 => 12, - 3 => 0, - 4 => 1, + 4 => 0, 5 => 1, + 6 => 1, }, "fiberSelfDurations": Map { 1 => 0, 2 => 10, - 3 => 0, - 4 => 1, + 4 => 0, 5 => 1, + 6 => 1, }, "passiveEffectDuration": null, "priorityLevel": "Normal", @@ -203,21 +203,21 @@ Object { exports[`ProfilingCache should collect data for each commit: CommitDetails commitIndex: 1 1`] = ` Object { "changeDescriptions": Map { - 3 => Object { + 4 => Object { "context": null, "didHooksChange": false, "isFirstMount": false, "props": Array [], "state": null, }, - 4 => Object { + 5 => Object { "context": null, "didHooksChange": false, "isFirstMount": false, "props": Array [], "state": null, }, - 6 => Object { + 7 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, @@ -237,16 +237,16 @@ Object { "duration": 13, "effectDuration": null, "fiberActualDurations": Map { - 3 => 0, - 4 => 1, - 6 => 2, + 4 => 0, + 5 => 1, + 7 => 2, 2 => 13, 1 => 13, }, "fiberSelfDurations": Map { - 3 => 0, - 4 => 1, - 6 => 2, + 4 => 0, + 5 => 1, + 7 => 2, 2 => 10, 1 => 0, }, @@ -268,7 +268,7 @@ Object { exports[`ProfilingCache should collect data for each commit: CommitDetails commitIndex: 2 1`] = ` Object { "changeDescriptions": Map { - 3 => Object { + 4 => Object { "context": null, "didHooksChange": false, "isFirstMount": false, @@ -288,12 +288,12 @@ Object { "duration": 10, "effectDuration": null, "fiberActualDurations": Map { - 3 => 0, + 4 => 0, 2 => 10, 1 => 10, }, "fiberSelfDurations": Map { - 3 => 0, + 4 => 0, 2 => 10, 1 => 0, }, @@ -368,7 +368,7 @@ Object { }, ], Array [ - 3, + 4, Object { "context": null, "didHooksChange": false, @@ -378,7 +378,7 @@ Object { }, ], Array [ - 4, + 5, Object { "context": null, "didHooksChange": false, @@ -388,7 +388,7 @@ Object { }, ], Array [ - 5, + 6, Object { "context": null, "didHooksChange": false, @@ -410,15 +410,15 @@ Object { 12, ], Array [ - 3, + 4, 0, ], Array [ - 4, + 5, 1, ], Array [ - 5, + 6, 1, ], ], @@ -432,15 +432,15 @@ Object { 10, ], Array [ - 3, + 4, 0, ], Array [ - 4, + 5, 1, ], Array [ - 5, + 6, 1, ], ], @@ -460,7 +460,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 3, + 4, Object { "context": null, "didHooksChange": false, @@ -470,7 +470,7 @@ Object { }, ], Array [ - 4, + 5, Object { "context": null, "didHooksChange": false, @@ -480,7 +480,7 @@ Object { }, ], Array [ - 6, + 7, Object { "context": null, "didHooksChange": false, @@ -506,15 +506,15 @@ Object { "effectDuration": null, "fiberActualDurations": Array [ Array [ - 3, + 4, 0, ], Array [ - 4, + 5, 1, ], Array [ - 6, + 7, 2, ], Array [ @@ -528,15 +528,15 @@ Object { ], "fiberSelfDurations": Array [ Array [ - 3, + 4, 0, ], Array [ - 4, + 5, 1, ], Array [ - 6, + 7, 2, ], Array [ @@ -564,7 +564,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 3, + 4, Object { "context": null, "didHooksChange": false, @@ -590,7 +590,7 @@ Object { "effectDuration": null, "fiberActualDurations": Array [ Array [ - 3, + 4, 0, ], Array [ @@ -604,7 +604,7 @@ Object { ], "fiberSelfDurations": Array [ Array [ - 3, + 4, 0, ], Array [ @@ -723,34 +723,34 @@ Object { 2, 12000, 1, - 3, + 4, 5, 2, 2, 2, 3, 4, - 3, + 4, 0, 1, - 4, + 5, 5, 2, 2, 2, 4, 4, - 4, + 5, 1000, 1, - 5, + 6, 8, 2, 2, 2, 0, 4, - 5, + 6, 1000, ], Array [ @@ -766,14 +766,14 @@ Object { 1, 50, 1, - 6, + 7, 5, 2, 2, 1, 2, 4, - 6, + 7, 2000, 4, 2, @@ -781,10 +781,10 @@ Object { 3, 2, 4, - 3, 4, - 6, 5, + 7, + 6, 4, 1, 14000, @@ -795,16 +795,16 @@ Object { 0, 2, 2, - 6, - 4, + 7, + 5, 4, 2, 11000, 3, 2, 2, - 3, - 5, + 4, + 6, 4, 1, 11000, @@ -815,7 +815,7 @@ Object { 0, 2, 1, - 3, + 4, ], ], "rootID": 1, @@ -834,7 +834,7 @@ Array [ ] `; -exports[`ProfilingCache should collect data for each rendered fiber: FiberCommits: element 3 1`] = ` +exports[`ProfilingCache should collect data for each rendered fiber: FiberCommits: element 4 1`] = ` Array [ 0, 1, @@ -842,20 +842,20 @@ Array [ ] `; -exports[`ProfilingCache should collect data for each rendered fiber: FiberCommits: element 4 1`] = ` +exports[`ProfilingCache should collect data for each rendered fiber: FiberCommits: element 5 1`] = ` Array [ 0, ] `; -exports[`ProfilingCache should collect data for each rendered fiber: FiberCommits: element 5 1`] = ` +exports[`ProfilingCache should collect data for each rendered fiber: FiberCommits: element 6 1`] = ` Array [ 1, 2, ] `; -exports[`ProfilingCache should collect data for each rendered fiber: FiberCommits: element 6 1`] = ` +exports[`ProfilingCache should collect data for each rendered fiber: FiberCommits: element 7 1`] = ` Array [ 2, ] @@ -879,7 +879,7 @@ Object { }, ], Array [ - 3, + 4, Object { "context": null, "didHooksChange": false, @@ -889,7 +889,7 @@ Object { }, ], Array [ - 4, + 5, Object { "context": null, "didHooksChange": false, @@ -911,11 +911,11 @@ Object { 11, ], Array [ - 3, + 4, 0, ], Array [ - 4, + 5, 1, ], ], @@ -929,11 +929,11 @@ Object { 10, ], Array [ - 3, + 4, 0, ], Array [ - 4, + 5, 1, ], ], @@ -953,7 +953,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 3, + 4, Object { "context": null, "didHooksChange": false, @@ -963,7 +963,7 @@ Object { }, ], Array [ - 5, + 6, Object { "context": null, "didHooksChange": false, @@ -989,11 +989,11 @@ Object { "effectDuration": null, "fiberActualDurations": Array [ Array [ - 3, + 4, 0, ], Array [ - 5, + 6, 1, ], Array [ @@ -1007,11 +1007,11 @@ Object { ], "fiberSelfDurations": Array [ Array [ - 3, + 4, 0, ], Array [ - 5, + 6, 1, ], Array [ @@ -1039,7 +1039,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 3, + 4, Object { "context": null, "didHooksChange": false, @@ -1049,7 +1049,7 @@ Object { }, ], Array [ - 5, + 6, Object { "context": null, "didHooksChange": false, @@ -1059,7 +1059,7 @@ Object { }, ], Array [ - 6, + 7, Object { "context": null, "didHooksChange": false, @@ -1085,15 +1085,15 @@ Object { "effectDuration": null, "fiberActualDurations": Array [ Array [ - 3, + 4, 0, ], Array [ - 5, + 6, 1, ], Array [ - 6, + 7, 2, ], Array [ @@ -1107,15 +1107,15 @@ Object { ], "fiberSelfDurations": Array [ Array [ - 3, + 4, 0, ], Array [ - 5, + 6, 1, ], Array [ - 6, + 7, 2, ], Array [ @@ -1182,24 +1182,24 @@ Object { 2, 11000, 1, - 3, + 4, 5, 2, 2, 2, 3, 4, - 3, + 4, 0, 1, - 4, + 5, 8, 2, 2, 2, 0, 4, - 4, + 5, 1000, ], Array [ @@ -1215,14 +1215,14 @@ Object { 1, 49, 1, - 5, + 6, 5, 2, 2, 1, 2, 4, - 5, + 6, 1000, 4, 2, @@ -1230,9 +1230,9 @@ Object { 3, 2, 3, - 3, - 5, 4, + 6, + 5, 4, 1, 12000, @@ -1250,14 +1250,14 @@ Object { 1, 50, 1, - 6, + 7, 5, 2, 2, 1, 2, 4, - 6, + 7, 2000, 4, 2, @@ -1265,10 +1265,10 @@ Object { 3, 2, 4, - 3, - 5, - 6, 4, + 6, + 7, + 5, 4, 1, 14000, @@ -1287,21 +1287,21 @@ Object { "commitData": Array [ Object { "changeDescriptions": Map { - 3 => Object { + 4 => Object { "context": null, "didHooksChange": false, "isFirstMount": false, "props": Array [], "state": null, }, - 4 => Object { + 5 => Object { "context": null, "didHooksChange": false, "isFirstMount": false, "props": Array [], "state": null, }, - 10 => Object { + 12 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, @@ -1321,16 +1321,16 @@ Object { "duration": 13, "effectDuration": null, "fiberActualDurations": Map { - 3 => 0, - 4 => 1, - 10 => 2, + 4 => 0, + 5 => 1, + 12 => 2, 2 => 13, 1 => 13, }, "fiberSelfDurations": Map { - 3 => 0, - 4 => 1, - 10 => 2, + 4 => 0, + 5 => 1, + 12 => 2, 2 => 10, 1 => 0, }, @@ -1349,7 +1349,7 @@ Object { }, Object { "changeDescriptions": Map { - 3 => Object { + 4 => Object { "context": null, "didHooksChange": false, "isFirstMount": false, @@ -1369,12 +1369,12 @@ Object { "duration": 10, "effectDuration": null, "fiberActualDurations": Map { - 3 => 0, + 4 => 0, 2 => 10, 1 => 10, }, "fiberSelfDurations": Map { - 3 => 0, + 4 => 0, 2 => 10, 1 => 0, }, @@ -1431,9 +1431,9 @@ Object { "initialTreeBaseDurations": Map { 1 => 12, 2 => 12, - 3 => 0, - 4 => 1, + 4 => 0, 5 => 1, + 6 => 1, }, "operations": Array [ Array [ @@ -1449,14 +1449,14 @@ Object { 1, 50, 1, - 10, + 12, 5, 2, 2, 1, 2, 4, - 10, + 12, 2000, 4, 2, @@ -1464,10 +1464,10 @@ Object { 3, 2, 4, - 3, 4, - 10, 5, + 12, + 6, 4, 1, 14000, @@ -1478,16 +1478,16 @@ Object { 0, 2, 2, - 10, - 4, + 12, + 5, 4, 2, 11000, 3, 2, 2, - 3, - 5, + 4, + 6, 4, 1, 11000, @@ -1498,7 +1498,7 @@ Object { 0, 2, 1, - 3, + 4, ], ], "rootID": 1, @@ -1515,9 +1515,9 @@ Object { }, 2 => Object { "children": Array [ - 3, 4, 5, + 6, ], "displayName": "Parent", "hocDisplayNames": null, @@ -1525,29 +1525,29 @@ Object { "key": null, "type": 5, }, - 3 => Object { + 4 => Object { "children": Array [], "displayName": "Child", "hocDisplayNames": null, - "id": 3, + "id": 4, "key": "0", "type": 5, }, - 4 => Object { + 5 => Object { "children": Array [], "displayName": "Child", "hocDisplayNames": null, - "id": 4, + "id": 5, "key": "1", "type": 5, }, - 5 => Object { + 6 => Object { "children": Array [], "displayName": "Child", "hocDisplayNames": Array [ "Memo", ], - "id": 5, + "id": 6, "key": null, "type": 8, }, @@ -1560,21 +1560,21 @@ Object { "commitData": Array [ Object { "changeDescriptions": Map { - 12 => Object { + 14 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, "props": null, "state": null, }, - 13 => Object { + 16 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, "props": null, "state": null, }, - 14 => Object { + 17 => Object { "context": null, "didHooksChange": false, "isFirstMount": true, @@ -1585,16 +1585,16 @@ Object { "duration": 11, "effectDuration": null, "fiberActualDurations": Map { - 11 => 11, - 12 => 11, - 13 => 0, - 14 => 1, + 13 => 11, + 14 => 11, + 16 => 0, + 17 => 1, }, "fiberSelfDurations": Map { - 11 => 0, - 12 => 10, 13 => 0, - 14 => 1, + 14 => 10, + 16 => 0, + 17 => 1, }, "passiveEffectDuration": null, "priorityLevel": "Normal", @@ -1603,7 +1603,7 @@ Object { Object { "displayName": "Anonymous", "hocDisplayNames": null, - "id": 11, + "id": 13, "key": null, "type": 11, }, @@ -1615,7 +1615,7 @@ Object { "operations": Array [ Array [ 1, - 11, + 13, 15, 6, 80, @@ -1633,46 +1633,46 @@ Object { 1, 48, 1, - 11, + 13, 11, 1, 1, 4, - 11, + 13, 11000, 1, - 12, + 14, 5, - 11, + 13, 0, 1, 0, 4, - 12, + 14, 11000, 1, - 13, + 16, 5, - 12, - 12, + 14, + 14, 2, 3, 4, - 13, + 16, 0, 1, - 14, + 17, 8, - 12, - 12, + 14, + 14, 2, 0, 4, - 14, + 17, 1000, ], ], - "rootID": 11, + "rootID": 13, "snapshots": Map {}, } `; @@ -1693,7 +1693,7 @@ Object { Object { "displayName": "Anonymous", "hocDisplayNames": null, - "id": 6, + "id": 7, "key": null, "type": 11, }, @@ -1702,62 +1702,62 @@ Object { ], "displayName": "Parent", "initialTreeBaseDurations": Map { - 6 => 11, 7 => 11, - 8 => 0, - 9 => 1, + 8 => 11, + 10 => 0, + 11 => 1, }, "operations": Array [ Array [ 1, - 6, + 7, 0, 2, 4, - 9, + 11, + 10, 8, 7, - 6, ], ], - "rootID": 6, + "rootID": 7, "snapshots": Map { - 6 => Object { + 7 => Object { "children": Array [ - 7, + 8, ], "displayName": null, "hocDisplayNames": null, - "id": 6, + "id": 7, "key": null, "type": 11, }, - 7 => Object { + 8 => Object { "children": Array [ - 8, - 9, + 10, + 11, ], "displayName": "Parent", "hocDisplayNames": null, - "id": 7, + "id": 8, "key": null, "type": 5, }, - 8 => Object { + 10 => Object { "children": Array [], "displayName": "Child", "hocDisplayNames": null, - "id": 8, + "id": 10, "key": "0", "type": 5, }, - 9 => Object { + 11 => Object { "children": Array [], "displayName": "Child", "hocDisplayNames": Array [ "Memo", ], - "id": 9, + "id": 11, "key": null, "type": 8, }, @@ -1773,7 +1773,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 3, + 4, Object { "context": null, "didHooksChange": false, @@ -1783,7 +1783,7 @@ Object { }, ], Array [ - 4, + 5, Object { "context": null, "didHooksChange": false, @@ -1793,7 +1793,7 @@ Object { }, ], Array [ - 10, + 12, Object { "context": null, "didHooksChange": false, @@ -1819,15 +1819,15 @@ Object { "effectDuration": null, "fiberActualDurations": Array [ Array [ - 3, + 4, 0, ], Array [ - 4, + 5, 1, ], Array [ - 10, + 12, 2, ], Array [ @@ -1841,15 +1841,15 @@ Object { ], "fiberSelfDurations": Array [ Array [ - 3, + 4, 0, ], Array [ - 4, + 5, 1, ], Array [ - 10, + 12, 2, ], Array [ @@ -1877,7 +1877,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 3, + 4, Object { "context": null, "didHooksChange": false, @@ -1903,7 +1903,7 @@ Object { "effectDuration": null, "fiberActualDurations": Array [ Array [ - 3, + 4, 0, ], Array [ @@ -1917,7 +1917,7 @@ Object { ], "fiberSelfDurations": Array [ Array [ - 3, + 4, 0, ], Array [ @@ -2004,15 +2004,15 @@ Object { 12, ], Array [ - 3, + 4, 0, ], Array [ - 4, + 5, 1, ], Array [ - 5, + 6, 1, ], ], @@ -2030,14 +2030,14 @@ Object { 1, 50, 1, - 10, + 12, 5, 2, 2, 1, 2, 4, - 10, + 12, 2000, 4, 2, @@ -2045,10 +2045,10 @@ Object { 3, 2, 4, - 3, 4, - 10, 5, + 12, + 6, 4, 1, 14000, @@ -2059,16 +2059,16 @@ Object { 0, 2, 2, - 10, - 4, + 12, + 5, 4, 2, 11000, 3, 2, 2, - 3, - 5, + 4, + 6, 4, 1, 11000, @@ -2079,7 +2079,7 @@ Object { 0, 2, 1, - 3, + 4, ], ], "rootID": 1, @@ -2101,9 +2101,9 @@ Object { 2, Object { "children": Array [ - 3, 4, 5, + 6, ], "displayName": "Parent", "hocDisplayNames": null, @@ -2113,36 +2113,36 @@ Object { }, ], Array [ - 3, + 4, Object { "children": Array [], "displayName": "Child", "hocDisplayNames": null, - "id": 3, + "id": 4, "key": "0", "type": 5, }, ], Array [ - 4, + 5, Object { "children": Array [], "displayName": "Child", "hocDisplayNames": null, - "id": 4, + "id": 5, "key": "1", "type": 5, }, ], Array [ - 5, + 6, Object { "children": Array [], "displayName": "Child", "hocDisplayNames": Array [ "Memo", ], - "id": 5, + "id": 6, "key": null, "type": 8, }, @@ -2154,7 +2154,7 @@ Object { Object { "changeDescriptions": Array [ Array [ - 12, + 14, Object { "context": null, "didHooksChange": false, @@ -2164,7 +2164,7 @@ Object { }, ], Array [ - 13, + 16, Object { "context": null, "didHooksChange": false, @@ -2174,7 +2174,7 @@ Object { }, ], Array [ - 14, + 17, Object { "context": null, "didHooksChange": false, @@ -2188,37 +2188,37 @@ Object { "effectDuration": null, "fiberActualDurations": Array [ Array [ - 11, + 13, 11, ], Array [ - 12, + 14, 11, ], Array [ - 13, + 16, 0, ], Array [ - 14, + 17, 1, ], ], "fiberSelfDurations": Array [ Array [ - 11, + 13, 0, ], Array [ - 12, + 14, 10, ], Array [ - 13, + 16, 0, ], Array [ - 14, + 17, 1, ], ], @@ -2229,7 +2229,7 @@ Object { Object { "displayName": "Anonymous", "hocDisplayNames": null, - "id": 11, + "id": 13, "key": null, "type": 11, }, @@ -2241,7 +2241,7 @@ Object { "operations": Array [ Array [ 1, - 11, + 13, 15, 6, 80, @@ -2259,46 +2259,46 @@ Object { 1, 48, 1, - 11, + 13, 11, 1, 1, 4, - 11, + 13, 11000, 1, - 12, + 14, 5, - 11, + 13, 0, 1, 0, 4, - 12, + 14, 11000, 1, - 13, + 16, 5, - 12, - 12, + 14, + 14, 2, 3, 4, - 13, + 16, 0, 1, - 14, + 17, 8, - 12, - 12, + 14, + 14, 2, 0, 4, - 14, + 17, 1000, ], ], - "rootID": 11, + "rootID": 13, "snapshots": Array [], }, Object { @@ -2316,7 +2316,7 @@ Object { Object { "displayName": "Anonymous", "hocDisplayNames": null, - "id": 6, + "id": 7, "key": null, "type": 11, }, @@ -2326,84 +2326,84 @@ Object { "displayName": "Parent", "initialTreeBaseDurations": Array [ Array [ - 6, + 7, 11, ], Array [ - 7, + 8, 11, ], Array [ - 8, + 10, 0, ], Array [ - 9, + 11, 1, ], ], "operations": Array [ Array [ 1, - 6, + 7, 0, 2, 4, - 9, + 11, + 10, 8, 7, - 6, ], ], - "rootID": 6, + "rootID": 7, "snapshots": Array [ Array [ - 6, + 7, Object { "children": Array [ - 7, + 8, ], "displayName": null, "hocDisplayNames": null, - "id": 6, + "id": 7, "key": null, "type": 11, }, ], Array [ - 7, + 8, Object { "children": Array [ - 8, - 9, + 10, + 11, ], "displayName": "Parent", "hocDisplayNames": null, - "id": 7, + "id": 8, "key": null, "type": 5, }, ], Array [ - 8, + 10, Object { "children": Array [], "displayName": "Child", "hocDisplayNames": null, - "id": 8, + "id": 10, "key": "0", "type": 5, }, ], Array [ - 9, + 11, Object { "children": Array [], "displayName": "Child", "hocDisplayNames": Array [ "Memo", ], - "id": 9, + "id": 11, "key": null, "type": 8, }, diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCommitTreeBuilder-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCommitTreeBuilder-test.js.snap index 1fa419013d98f..43f3f91c7a24f 100644 --- a/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCommitTreeBuilder-test.js.snap +++ b/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCommitTreeBuilder-test.js.snap @@ -71,7 +71,7 @@ Object { }, 3 => Object { "children": Array [ - 4, + 6, ], "displayName": "Suspense", "hocDisplayNames": null, @@ -81,11 +81,11 @@ Object { "treeBaseDuration": 0, "type": 12, }, - 4 => Object { + 6 => Object { "children": Array [], "displayName": "LazyInnerComponent", "hocDisplayNames": null, - "id": 4, + "id": 6, "key": null, "parentID": 3, "treeBaseDuration": 0, @@ -167,7 +167,7 @@ Object { }, 3 => Object { "children": Array [ - 4, + 6, ], "displayName": "Suspense", "hocDisplayNames": null, @@ -177,11 +177,11 @@ Object { "treeBaseDuration": 0, "type": 12, }, - 4 => Object { + 6 => Object { "children": Array [], "displayName": "LazyInnerComponent", "hocDisplayNames": null, - "id": 4, + "id": 6, "key": null, "parentID": 3, "treeBaseDuration": 0, diff --git a/packages/react-devtools-shared/src/__tests__/setupTests.js b/packages/react-devtools-shared/src/__tests__/setupTests.js index b244c7e0cf17e..811f5661dc135 100644 --- a/packages/react-devtools-shared/src/__tests__/setupTests.js +++ b/packages/react-devtools-shared/src/__tests__/setupTests.js @@ -12,8 +12,47 @@ import type { FrontendBridge, } from 'react-devtools-shared/src/bridge'; +import {Console} from 'console'; + +// Override Jest's default behavior of prepending location (file & line number) for logs. +function patchConsole() { + const customConsole = new Console({ + stdout: process.stdout, + stderr: process.stderr, + colorMode: false, + }); + + const {error, info, warn} = customConsole; + + const colorStrings = (method, color) => (...args) => { + let stringFound = false; + for (let i = 0; i < args.length; i++) { + const arg = args[i]; + if (typeof arg === 'string') { + stringFound = true; + args[i] = color + arg; + break; + } + } + + if (stringFound) { + args.push('\x1b[0m'); + } + + method(...args); + }; + + customConsole.error = colorStrings(error, '\x1b[31m'); + customConsole.info = colorStrings(info, '\x1b[34m'); + customConsole.warn = colorStrings(warn, '\x1b[33m'); + + //global.console = customConsole; +} + const env = jasmine.getEnv(); env.beforeEach(() => { + patchConsole(); + global.mockClipboardCopy = jest.fn(); // Test environment doesn't support document methods like execCommand() diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index f4d3def1766b6..091d755009b42 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -1007,11 +1007,6 @@ export function attach( const getProfilingData = () => { throw new Error('getProfilingData not supported by this renderer'); }; - const handleClonedForForceRemount = () => { - throw new Error( - 'handleClonedForForceRemount not supported by this renderer', - ); - }; const handleCommitFiberRoot = () => { throw new Error('handleCommitFiberRoot not supported by this renderer'); }; @@ -1089,7 +1084,6 @@ export function attach( getOwnersList, getPathForElement, getProfilingData, - handleClonedForForceRemount, handleCommitFiberRoot, handleCommitFiberUnmount, handlePostCommitFiberRoot, diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 476a1781e0791..13a8e1b34a8ec 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -636,7 +636,7 @@ export function attach( // Fortunately we would only leak Fibers that have errors/warnings associated with them, // which is hopefully only a small set and only in DEV mode– but this is still not great. // We should clean up Fibers like this when flushing; see recordPendingErrorsAndWarnings(). - const fiberID = getFiberID(getPrimaryFiber(fiber)); + const fiberID = getOrGenerateFiberID(fiber); if (__DEBUG__) { debug('onErrorOrWarning', fiber, null, `${type}: "${message}"`); @@ -707,17 +707,19 @@ export function attach( const displayName = fiber.tag + ':' + (getDisplayNameForFiber(fiber) || 'null'); - const id = getFiberIDSafe(fiber) || '-'; + const maybeID = getFiberIDUnsafe(fiber) || ''; const parentDisplayName = parentFiber ? parentFiber.tag + ':' + (getDisplayNameForFiber(parentFiber) || 'null') : ''; - const parentID = parentFiber ? getFiberIDSafe(parentFiber) || '-' : ''; + const maybeParentID = parentFiber + ? getFiberIDUnsafe(parentFiber) || '' + : ''; console.log( - `[renderer] %c${name} %c${displayName} (${id}) %c${ - parentFiber ? `${parentDisplayName} (${parentID})` : '' + `[renderer] %c${name} %c${displayName} (${maybeID}) %c${ + parentFiber ? `${parentDisplayName} (${maybeParentID})` : '' } %c${extraString}`, 'color: red; font-weight: bold;', 'color: blue;', @@ -802,7 +804,7 @@ export function attach( // Recursively unmount all roots. hook.getFiberRoots(rendererID).forEach(root => { - currentRootID = getFiberID(getPrimaryFiber(root.current)); + currentRootID = getOrGenerateFiberID(root.current); // The TREE_OPERATION_REMOVE_ROOT operation serves two purposes: // 1. It avoids sending unnecessary bridge traffic to clear a root. // 2. It preserves Fiber IDs when remounting (below) which in turn ID to error/warning mapping. @@ -818,7 +820,7 @@ export function attach( // Recursively re-mount all roots with new filter criteria applied. hook.getFiberRoots(rendererID).forEach(root => { - currentRootID = getFiberID(getPrimaryFiber(root.current)); + currentRootID = getOrGenerateFiberID(root.current); setRootPseudoKey(currentRootID, root.current); mountFiberRecursively(root.current, null, false, false); flushPendingEvents(root); @@ -949,25 +951,16 @@ export function attach( } } - // This is a slightly annoying indirection. - // It is currently necessary because DevTools wants to use unique objects as keys for instances. - // However fibers have two versions. - // We use this set to remember first encountered fiber for each conceptual instance. - function getPrimaryFiber(fiber: Fiber): Fiber { - if (primaryFibers.has(fiber)) { - return fiber; - } - const {alternate} = fiber; - if (alternate != null && primaryFibers.has(alternate)) { - return alternate; - } - primaryFibers.add(fiber); - return fiber; - } - + // Map of one or more Fibers in a pair to their unique id number. + // We track both Fibers to support Fast Refresh, + // which may forcefully replace one of the pair as part of hot reloading. + // In that case it's still important to be able to locate the previous ID during subsequent renders. const fiberToIDMap: Map = new Map(); - const idToFiberMap: Map = new Map(); - const primaryFibers: Set = new Set(); + + // Map of id to one (arbitrary) Fiber in a pair. + // This Map is used to e.g. get the display name for a Fiber or schedule an update, + // operations that should be the same whether the current and work-in-progress Fiber is used. + const idToArbitraryFiberMap: Map = new Map(); // When profiling is supported, we store the latest tree base durations for each Fiber. // This is so that we can quickly capture a snapshot of those values if profiling starts. @@ -982,30 +975,77 @@ export function attach( // When a mount or update is in progress, this value tracks the root that is being operated on. let currentRootID: number = -1; - function getFiberIDSafe(fiber: Fiber): number | null { - let primaryFiber = null; - if (primaryFibers.has(fiber)) { - primaryFiber = fiber; + function getOrGenerateFiberID(fiber: Fiber): number { + let id = null; + if (fiberToIDMap.has(fiber)) { + id = fiberToIDMap.get(fiber); + } else { + const {alternate} = fiber; + if (alternate !== null && fiberToIDMap.has(alternate)) { + id = fiberToIDMap.get(alternate); + } + } + + if (id === null) { + id = getUID(); + } + + // This refinement is for Flow purposes only. + const refinedID = ((id: any): number); + + // Make sure we're tracking this Fiber + // e.g. if it just mounted or an error was logged during initial render. + if (!fiberToIDMap.has(fiber)) { + fiberToIDMap.set(fiber, refinedID); + idToArbitraryFiberMap.set(refinedID, fiber); } + + // Also make sure we're tracking its alternate, + // e.g. in case this is the first update after mount. const {alternate} = fiber; - if (alternate != null && primaryFibers.has(alternate)) { - primaryFiber = alternate; + if (alternate !== null) { + if (!fiberToIDMap.has(alternate)) { + fiberToIDMap.set(alternate, refinedID); + } } - if (primaryFiber && fiberToIDMap.has(primaryFiber)) { - return ((fiberToIDMap.get(primaryFiber): any): number); + return refinedID; + } + + function getFiberIDThrows(fiber: Fiber): number { + const maybeID = getFiberIDUnsafe(fiber); + if (maybeID !== null) { + return maybeID; } + throw Error( + `Could not find ID for Fiber "${getDisplayNameForFiber(fiber) || ''}"`, + ); + } + function getFiberIDUnsafe(fiber: Fiber): number | null { + if (fiberToIDMap.has(fiber)) { + return ((fiberToIDMap.get(fiber): any): number); + } else { + const {alternate} = fiber; + if (alternate !== null && fiberToIDMap.has(alternate)) { + return ((fiberToIDMap.get(alternate): any): number); + } + } return null; } - function getFiberID(primaryFiber: Fiber): number { - if (!fiberToIDMap.has(primaryFiber)) { - const id = getUID(); - fiberToIDMap.set(primaryFiber, id); - idToFiberMap.set(id, primaryFiber); + function untrackFiberID(fiber: Fiber) { + const fiberID = getFiberIDUnsafe(fiber); + if (fiberID !== null) { + idToArbitraryFiberMap.delete(fiberID); + } + + fiberToIDMap.delete(fiber); + + const {alternate} = fiber; + if (alternate !== null) { + fiberToIDMap.delete(alternate); } - return ((fiberToIDMap.get(primaryFiber): any): number); } function getChangeDescription( @@ -1066,7 +1106,7 @@ export function attach( switch (getElementTypeForFiber(fiber)) { case ElementTypeClass: if (idToContextsMap !== null) { - const id = getFiberID(getPrimaryFiber(fiber)); + const id = getFiberIDThrows(fiber); const contexts = getContextsForFiber(fiber); if (contexts !== null) { idToContextsMap.set(id, contexts); @@ -1122,7 +1162,7 @@ export function attach( switch (getElementTypeForFiber(fiber)) { case ElementTypeClass: if (idToContextsMap !== null) { - const id = getFiberID(getPrimaryFiber(fiber)); + const id = getFiberIDThrows(fiber); const prevContexts = idToContextsMap.has(id) ? idToContextsMap.get(id) : null; @@ -1390,15 +1430,13 @@ export function attach( clearPendingErrorsAndWarningsAfterDelay(); fibersWithChangedErrorOrWarningCounts.forEach(fiberID => { - const fiber = idToFiberMap.get(fiberID); + const fiber = idToArbitraryFiberMap.get(fiberID); if (fiber != null) { // Don't send updates for Fibers that didn't mount due to e.g. Suspense or an error boundary. // We may also need to clean up after ourselves to avoid leaks. // See inline comments in onErrorOrWarning() for more info. if (isFiberMountedImpl(fiber) !== MOUNTED) { - fiberToIDMap.delete(fiber); - idToFiberMap.delete(fiberID); - primaryFibers.delete(fiber); + untrackFiberID(fiber); return; } @@ -1559,7 +1597,7 @@ export function attach( } const isRoot = fiber.tag === HostRoot; - const id = getFiberID(getPrimaryFiber(fiber)); + const id = getOrGenerateFiberID(fiber); const hasOwnerMetadata = fiber.hasOwnProperty('_debugOwner'); const isProfilingSupported = fiber.hasOwnProperty('treeBaseDuration'); @@ -1582,11 +1620,8 @@ export function attach( const elementType = getElementTypeForFiber(fiber); const {_debugOwner} = fiber; - const ownerID = - _debugOwner != null ? getFiberID(getPrimaryFiber(_debugOwner)) : 0; - const parentID = parentFiber - ? getFiberID(getPrimaryFiber(parentFiber)) - : 0; + const ownerID = _debugOwner != null ? getFiberIDThrows(_debugOwner) : 0; + const parentID = parentFiber ? getFiberIDThrows(parentFiber) : 0; const displayNameStringID = getStringID(displayName); @@ -1621,6 +1656,18 @@ export function attach( ); } + const unsafeID = getFiberIDUnsafe(fiber); + if (fiber._debugNeedsRemount) { + if (unsafeID === null) { + // This is a case we can't recover from. + setTimeout(() => { + // TODO (bvaughn) Document this + updateComponentFilters([]); + }, 0); + return; + } + } + if (trackedPathMatchFiber !== null) { // We're in the process of trying to restore previous selection. // If this fiber matched but is being unmounted, there's no use trying. @@ -1633,9 +1680,7 @@ export function attach( } } - const isRoot = fiber.tag === HostRoot; - const primaryFiber = getPrimaryFiber(fiber); - if (!fiberToIDMap.has(primaryFiber)) { + if (unsafeID === null) { // If we've never seen this Fiber, it might be inside of a legacy render Suspense fragment (so the store is not even aware of it). // In that case we can just ignore it or it will cause errors later on. // One example of this is a Lazy component that never resolves before being unmounted. @@ -1643,10 +1688,12 @@ export function attach( // TODO: This is fragile and can obscure actual bugs. // // Calling getPrimaryFiber() lazily adds fibers to the Map, so clean up after ourselves before returning. - primaryFibers.delete(primaryFiber); return; } - const id = getFiberID(primaryFiber); + + // Flow refinement. + const id = ((unsafeID: any): number); + const isRoot = fiber.tag === HostRoot; if (isRoot) { // Roots must be removed only after all children (pending and simulated) have been removed. // So we track it separately. @@ -1662,14 +1709,14 @@ export function attach( } } - fiberToIDMap.delete(primaryFiber); - idToFiberMap.delete(id); - primaryFibers.delete(primaryFiber); + if (!fiber._debugNeedsRemount) { + untrackFiberID(fiber); - const isProfilingSupported = fiber.hasOwnProperty('treeBaseDuration'); - if (isProfilingSupported) { - idToRootMap.delete(id); - idToTreeBaseDurationMap.delete(id); + const isProfilingSupported = fiber.hasOwnProperty('treeBaseDuration'); + if (isProfilingSupported) { + idToRootMap.delete(id); + idToTreeBaseDurationMap.delete(id); + } } } @@ -1696,6 +1743,9 @@ export function attach( const shouldIncludeInTree = !shouldFilterFiber(fiber); if (shouldIncludeInTree) { recordMount(fiber, parentFiber); + } else { + // Generate an ID even for filtered Fibers, in case it's needed later (e.g. for Profiling). + getOrGenerateFiberID(fiber); } if (traceUpdatesEnabled) { @@ -1806,7 +1856,7 @@ export function attach( } function recordProfilingDurations(fiber: Fiber) { - const id = getFiberID(getPrimaryFiber(fiber)); + const id = getFiberIDThrows(fiber); const {actualDuration, treeBaseDuration} = fiber; idToTreeBaseDurationMap.set(id, treeBaseDuration || 0); @@ -1894,7 +1944,7 @@ export function attach( return; } pushOperation(TREE_OPERATION_REORDER_CHILDREN); - pushOperation(getFiberID(getPrimaryFiber(fiber))); + pushOperation(getFiberIDThrows(fiber)); pushOperation(numChildren); for (let i = 0; i < nextChildren.length; i++) { pushOperation(nextChildren[i]); @@ -1906,7 +1956,7 @@ export function attach( nextChildren: Array, ) { if (!shouldFilterFiber(fiber)) { - nextChildren.push(getFiberID(getPrimaryFiber(fiber))); + nextChildren.push(getFiberIDThrows(fiber)); } else { let child = fiber.child; const isTimedOutSuspense = @@ -1944,6 +1994,8 @@ export function attach( debug('updateFiberRecursively()', nextFiber, parentFiber); } + const id = getOrGenerateFiberID(nextFiber); + if (traceUpdatesEnabled) { const elementType = getElementTypeForFiber(nextFiber); if (traceNearestHostComponentUpdate) { @@ -1969,8 +2021,7 @@ export function attach( if ( mostRecentlyInspectedElement !== null && - mostRecentlyInspectedElement.id === - getFiberID(getPrimaryFiber(nextFiber)) && + mostRecentlyInspectedElement.id === id && didFiberRender(prevFiber, nextFiber) ) { // If this Fiber has updated, clear cached inspected data. @@ -2114,7 +2165,7 @@ export function attach( // we should fall back to recursively marking the nearest host descendants for highlight. if (traceNearestHostComponentUpdate) { const hostFibers = findAllCurrentHostFibers( - getFiberID(getPrimaryFiber(nextFiber)), + getFiberIDThrows(nextFiber), ); hostFibers.forEach(hostFiber => { traceUpdatesForNodes.add(hostFiber.stateNode); @@ -2198,7 +2249,7 @@ export function attach( } // If we have not been profiling, then we can just walk the tree and build up its current state as-is. hook.getFiberRoots(rendererID).forEach(root => { - currentRootID = getFiberID(getPrimaryFiber(root.current)); + currentRootID = getOrGenerateFiberID(root.current); setRootPseudoKey(currentRootID, root.current); // Handle multi-renderer edge-case where only some v16 renderers support profiling. @@ -2253,7 +2304,7 @@ export function attach( const current = root.current; const alternate = current.alternate; - currentRootID = getFiberID(getPrimaryFiber(current)); + currentRootID = getOrGenerateFiberID(current); // Before the traversals, remember to start tracking // our path in case we have selection to restore. @@ -2399,7 +2450,7 @@ export function attach( } function getDisplayNameForFiberID(id) { - const fiber = idToFiberMap.get(id); + const fiber = idToArbitraryFiberMap.get(id); return fiber != null ? getDisplayNameForFiber(((fiber: any): Fiber)) : null; } @@ -2414,7 +2465,7 @@ export function attach( fiber = fiber.return; } } - return getFiberID(getPrimaryFiber(((fiber: any): Fiber))); + return getFiberIDThrows(((fiber: any): Fiber)); } return null; } @@ -2484,7 +2535,7 @@ export function attach( // It would be nice if we updated React to inject this function directly (vs just indirectly via findDOMNode). // BEGIN copied code function findCurrentFiberUsingSlowPathById(id: number): Fiber | null { - const fiber = idToFiberMap.get(id); + const fiber = idToArbitraryFiberMap.get(id); if (fiber == null) { console.warn(`Could not find Fiber with id "${id}"`); return null; @@ -2646,7 +2697,7 @@ export function attach( } function prepareViewElementSource(id: number): void { - const fiber = idToFiberMap.get(id); + const fiber = idToArbitraryFiberMap.get(id); if (fiber == null) { console.warn(`Could not find Fiber with id "${id}"`); return; @@ -2680,7 +2731,7 @@ export function attach( function fiberToSerializedElement(fiber: Fiber): SerializedElement { return { displayName: getDisplayNameForFiber(fiber) || 'Anonymous', - id: getFiberID(getPrimaryFiber(fiber)), + id: getFiberIDThrows(fiber), key: fiber.key, type: getElementTypeForFiber(fiber), }; @@ -3011,7 +3062,7 @@ export function attach( function updateSelectedElement(inspectedElement: InspectedElement): void { const {hooks, id, props} = inspectedElement; - const fiber = idToFiberMap.get(id); + const fiber = idToArbitraryFiberMap.get(id); if (fiber == null) { console.warn(`Could not find Fiber with id "${id}"`); return; @@ -3529,7 +3580,7 @@ export function attach( idToContextsMap = new Map(); hook.getFiberRoots(rendererID).forEach(root => { - const rootID = getFiberID(getPrimaryFiber(root.current)); + const rootID = getFiberIDThrows(root.current); ((displayNamesByRootID: any): DisplayNamesByRootID).set( rootID, getDisplayNameForRoot(root.current), @@ -3572,8 +3623,8 @@ export function attach( const forceFallbackForSuspenseIDs = new Set(); function shouldSuspendFiberAccordingToSet(fiber) { - const id = getFiberID(getPrimaryFiber(((fiber: any): Fiber))); - return forceFallbackForSuspenseIDs.has(id); + const maybeID = getFiberIDUnsafe(((fiber: any): Fiber)); + return maybeID !== null && forceFallbackForSuspenseIDs.has(maybeID); } function overrideSuspense(id, forceFallback) { @@ -3598,7 +3649,7 @@ export function attach( setSuspenseHandler(shouldSuspendFiberAlwaysFalse); } } - const fiber = idToFiberMap.get(id); + const fiber = idToArbitraryFiberMap.get(id); if (fiber != null) { scheduleUpdate(fiber); } @@ -3749,7 +3800,7 @@ export function attach( case HostRoot: // Roots don't have a real displayName, index, or key. // Instead, we'll use the pseudo key (childDisplayName:indexWithThatName). - const id = getFiberID(getPrimaryFiber(fiber)); + const id = getFiberIDThrows(fiber); const pseudoKey = rootPseudoKeys.get(id); if (pseudoKey === undefined) { throw new Error('Expected mounted root to have known pseudo key.'); @@ -3774,7 +3825,7 @@ export function attach( // The return path will contain Fibers that are "invisible" to the store // because their keys and indexes are important to restoring the selection. function getPathForElement(id: number): Array | null { - let fiber = idToFiberMap.get(id); + let fiber = idToArbitraryFiberMap.get(id); if (fiber == null) { return null; } @@ -3805,7 +3856,7 @@ export function attach( return null; } return { - id: getFiberID(getPrimaryFiber(fiber)), + id: getFiberIDThrows(fiber), isFullMatch: trackedPathMatchDepth === trackedPath.length - 1, }; } @@ -3836,41 +3887,6 @@ export function attach( traceUpdatesEnabled = isEnabled; } - function handleClonedForForceRemount(oldFiber: Fiber, newFiber: Fiber): void { - if (__DEBUG__) { - console.log( - '[renderer] handleClonedForForceRemount', - getDisplayNameForFiber(oldFiber), - '(' + getFiberID(getPrimaryFiber(oldFiber)), - '->', - getFiberID(getPrimaryFiber(newFiber)) + ')', - ); - } - - let primaryFiber = null; - if (primaryFibers.has(oldFiber)) { - primaryFiber = oldFiber; - } - const {alternate} = oldFiber; - if (alternate != null && primaryFibers.has(alternate)) { - primaryFiber = alternate; - } - - // Fast Refresh is about to (synchronously) clone and replace this part of the tree. - // In order to avoid these Fibers from leaking on the DevTools side, - // and possibly remaining visible in the Components tab as well, - // queue up unmount operations to send on the next commit. - if (primaryFiber) { - recordUnmount(primaryFiber, false); - unmountFiberChildrenRecursively(primaryFiber); - - const fiberID = ((fiberToIDMap.get(primaryFiber): any): number); - fiberToIDMap.delete(primaryFiber); - idToFiberMap.delete(fiberID); - primaryFibers.delete(primaryFiber); - } - } - return { cleanup, clearErrorsAndWarnings, @@ -3887,7 +3903,6 @@ export function attach( getOwnersList, getPathForElement, getProfilingData, - handleClonedForForceRemount, handleCommitFiberRoot, handleCommitFiberUnmount, handlePostCommitFiberRoot, diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 5fc0950791e46..342931c250eab 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -323,7 +323,6 @@ export type RendererInterface = { getProfilingData(): ProfilingDataBackend, getOwnersList: (id: number) => Array | null, getPathForElement: (id: number) => Array | null, - handleClonedForForceRemount: (oldFiber: Fiber, newFiber: Fiber) => void, handleCommitFiberRoot: (fiber: Object, commitPriority?: number) => void, handleCommitFiberUnmount: (fiber: Object) => void, handlePostCommitFiberRoot: (fiber: Object) => void, @@ -397,12 +396,5 @@ export type DevToolsHook = { // Added in v16.9 to support Fast Refresh didError?: boolean, ) => void, - - // Added in v17.x to improve Fast Refresh + DevTools integration - onClonedForForceRemount: ( - rendererID: RendererID, - oldFiber: Fiber, - newFiber: Fiber, - ) => void, ... }; diff --git a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js index 45441a925df65..63c67afdd9bc2 100644 --- a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js +++ b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js @@ -21,6 +21,7 @@ type Props = {| type State = {| callStack: string | null, + canDismiss: boolean, componentStack: string | null, errorMessage: string | null, hasError: boolean, @@ -28,6 +29,7 @@ type State = {| const InitialState: State = { callStack: null, + canDismiss: false, componentStack: null, errorMessage: null, hasError: false, @@ -77,13 +79,20 @@ export default class ErrorBoundary extends Component { render() { const {children} = this.props; - const {callStack, componentStack, errorMessage, hasError} = this.state; + const { + callStack, + canDismiss, + componentStack, + errorMessage, + hasError, + } = this.state; if (hasError) { return ( }> { return children; } + _dismissError = () => { + this.setState(InitialState); + }; + _onStoreError = (error: Error) => { if (!this.state.hasError) { - this.setState(ErrorBoundary.getDerivedStateFromError(error)); + this.setState({ + ...ErrorBoundary.getDerivedStateFromError(error), + canDismiss: true, + }); } }; } diff --git a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorView.js b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorView.js index 6ec792408fe9c..65898c5e553d7 100644 --- a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorView.js +++ b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorView.js @@ -8,12 +8,14 @@ */ import * as React from 'react'; +import Button from 'react-devtools-shared/src/devtools/views/Button'; import styles from './shared.css'; type Props = {| callStack: string | null, children: React$Node, componentStack: string | null, + dismissError: Function | null, errorMessage: string | null, |}; @@ -21,14 +23,20 @@ export default function ErrorView({ callStack, children, componentStack, + dismissError = null, errorMessage, }: Props) { return (
{children}
-
- Uncaught Error: {errorMessage || ''} +
+
+ Uncaught Error: {errorMessage || ''} +
+ {dismissError !== null && ( + + )}
{!!callStack && (
diff --git a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/shared.css b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/shared.css index 267cfad7b7e88..05988649e01cf 100644 --- a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/shared.css +++ b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/shared.css @@ -37,12 +37,22 @@ overflow: auto; } -.Header { +.HeaderRow { + display: flex; + flex-direction: row; font-size: var(--font-size-sans-large); font-weight: bold; color: var(--color-error-text); } +.Header { + flex: 1 1 auto; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + min-width: 0; +} + .Stack { margin-top: 0.5rem; white-space: pre-wrap; @@ -75,9 +85,13 @@ .ReproSteps { margin-left: 0.25rem; color: var(--color-console-warning-text); + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + min-width: 0; } .UpdateExistingIssuePrompt { margin-right: 0.25rem; color: var(--color-console-warning-text); -} \ No newline at end of file +} diff --git a/packages/react-devtools-shared/src/hook.js b/packages/react-devtools-shared/src/hook.js index d636a683d526b..a4724f6f6b92b 100644 --- a/packages/react-devtools-shared/src/hook.js +++ b/packages/react-devtools-shared/src/hook.js @@ -261,13 +261,6 @@ export function installHook(target: any): DevToolsHook | null { return roots[rendererID]; } - function onClonedForForceRemount(rendererID, oldFiber, newFiber) { - const rendererInterface = rendererInterfaces.get(rendererID); - if (rendererInterface != null) { - rendererInterface.handleClonedForForceRemount(oldFiber, newFiber); - } - } - function onCommitFiberUnmount(rendererID, fiber) { const rendererInterface = rendererInterfaces.get(rendererID); if (rendererInterface != null) { @@ -313,7 +306,6 @@ export function installHook(target: any): DevToolsHook | null { // Fast Refresh for web relies on this. renderers, - onClonedForForceRemount, emit, getFiberRoots, diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 44dc1caefeeb6..2dfc6766577f9 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -32,10 +32,6 @@ import type {UpdateQueue} from './ReactUpdateQueue.new'; import checkPropTypes from 'shared/checkPropTypes'; -import { - isDevToolsPresent, - onClonedForForceRemount, -} from './ReactFiberDevToolsHook.new'; import { IndeterminateComponent, FunctionComponent, @@ -3198,21 +3194,19 @@ function beginWork( if (__DEV__) { if (workInProgress._debugNeedsRemount && current !== null) { - const clonedWorkInProgress = createFiberFromTypeAndProps( - workInProgress.type, - workInProgress.key, - workInProgress.pendingProps, - workInProgress._debugOwner || null, - workInProgress.mode, - workInProgress.lanes, - ); - - if (isDevToolsPresent) { - onClonedForForceRemount(workInProgress, clonedWorkInProgress); - } - // This will restart the begin phase with a new fiber. - return remountFiber(current, workInProgress, clonedWorkInProgress); + return remountFiber( + current, + workInProgress, + createFiberFromTypeAndProps( + workInProgress.type, + workInProgress.key, + workInProgress.pendingProps, + workInProgress._debugOwner || null, + workInProgress.mode, + workInProgress.lanes, + ), + ); } } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index a3293218e9ea5..7c9df820b98c5 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -32,10 +32,6 @@ import type {UpdateQueue} from './ReactUpdateQueue.old'; import checkPropTypes from 'shared/checkPropTypes'; -import { - isDevToolsPresent, - onClonedForForceRemount, -} from './ReactFiberDevToolsHook.old'; import { IndeterminateComponent, FunctionComponent, @@ -3198,21 +3194,19 @@ function beginWork( if (__DEV__) { if (workInProgress._debugNeedsRemount && current !== null) { - const clonedWorkInProgress = createFiberFromTypeAndProps( - workInProgress.type, - workInProgress.key, - workInProgress.pendingProps, - workInProgress._debugOwner || null, - workInProgress.mode, - workInProgress.lanes, - ); - - if (isDevToolsPresent) { - onClonedForForceRemount(workInProgress, clonedWorkInProgress); - } - // This will restart the begin phase with a new fiber. - return remountFiber(current, workInProgress, clonedWorkInProgress); + return remountFiber( + current, + workInProgress, + createFiberFromTypeAndProps( + workInProgress.type, + workInProgress.key, + workInProgress.pendingProps, + workInProgress._debugOwner || null, + workInProgress.mode, + workInProgress.lanes, + ), + ); } } diff --git a/packages/react-reconciler/src/ReactFiberDevToolsHook.new.js b/packages/react-reconciler/src/ReactFiberDevToolsHook.new.js index 9d2f9243a2752..e7bde59e20cba 100644 --- a/packages/react-reconciler/src/ReactFiberDevToolsHook.new.js +++ b/packages/react-reconciler/src/ReactFiberDevToolsHook.new.js @@ -166,28 +166,3 @@ export function onCommitUnmount(fiber: Fiber) { } } } - -export function onClonedForForceRemount( - oldWorkInProgress: Fiber, - newWorkInProgress: Fiber, -) { - if ( - injectedHook && - typeof injectedHook.onClonedForForceRemount === 'function' - ) { - try { - injectedHook.onClonedForForceRemount( - rendererID, - oldWorkInProgress, - newWorkInProgress, - ); - } catch (err) { - if (__DEV__) { - if (!hasLoggedError) { - hasLoggedError = true; - console.error('React instrumentation encountered an error: %s', err); - } - } - } - } -} diff --git a/packages/react-reconciler/src/ReactFiberDevToolsHook.old.js b/packages/react-reconciler/src/ReactFiberDevToolsHook.old.js index 198e6233c0b9d..494138685e104 100644 --- a/packages/react-reconciler/src/ReactFiberDevToolsHook.old.js +++ b/packages/react-reconciler/src/ReactFiberDevToolsHook.old.js @@ -166,28 +166,3 @@ export function onCommitUnmount(fiber: Fiber) { } } } - -export function onClonedForForceRemount( - oldWorkInProgress: Fiber, - newWorkInProgress: Fiber, -) { - if ( - injectedHook && - typeof injectedHook.onClonedForForceRemount === 'function' - ) { - try { - injectedHook.onClonedForForceRemount( - rendererID, - oldWorkInProgress, - newWorkInProgress, - ); - } catch (err) { - if (__DEV__) { - if (!hasLoggedError) { - hasLoggedError = true; - console.error('React instrumentation encountered an error: %s', err); - } - } - } - } -} diff --git a/packages/react-reconciler/src/ReactFiberHotReloading.new.js b/packages/react-reconciler/src/ReactFiberHotReloading.new.js index 61ce24224c3e3..4c9eaf010125c 100644 --- a/packages/react-reconciler/src/ReactFiberHotReloading.new.js +++ b/packages/react-reconciler/src/ReactFiberHotReloading.new.js @@ -318,7 +318,6 @@ function scheduleFibersWithFamiliesRecursively( if (needsRemount) { fiber._debugNeedsRemount = true; } - if (needsRemount || needsRender) { scheduleUpdateOnFiber(fiber, SyncLane, NoTimestamp); } diff --git a/packages/react-reconciler/src/ReactFiberHotReloading.old.js b/packages/react-reconciler/src/ReactFiberHotReloading.old.js index 475b449541d16..ee0616fae79c0 100644 --- a/packages/react-reconciler/src/ReactFiberHotReloading.old.js +++ b/packages/react-reconciler/src/ReactFiberHotReloading.old.js @@ -318,7 +318,6 @@ function scheduleFibersWithFamiliesRecursively( if (needsRemount) { fiber._debugNeedsRemount = true; } - if (needsRemount || needsRender) { scheduleUpdateOnFiber(fiber, SyncLane, NoTimestamp); }