{"commit":{"authoredAt":"2021-08-20T12:53:28-04:00","revision":"75a3e9fa4074b4b18ee95398ceabc361a6c9f82e","shortRevision":"75a3e9fa40","authorEmail":"bvaughn@fb.com","authorName":"Brian Vaughn","subject":"DevTools: Reset cached indices in Store after elements reordered (#22147)"},"parents":["b6ff9ad1630b4f1d7cb98e9b9ec46bca315bb302"],"body":"","patch":"diff --git a\/packages\/react-devtools-shared\/src\/__tests__\/treeContext-test.js b\/packages\/react-devtools-shared\/src\/__tests__\/treeContext-test.js\nindex 30d21c0335..36f14d8c7d 100644\n--- a\/packages\/react-devtools-shared\/src\/__tests__\/treeContext-test.js\n+++ b\/packages\/react-devtools-shared\/src\/__tests__\/treeContext-test.js\n@@ -2177,6 +2177,111 @@ describe('TreeListContext', () => {\n `);\n });\n \n+ it('should update correctly when elements are re-ordered', () => {\n+ const container = document.createElement('div');\n+ function ErrorOnce() {\n+ const didErroRef = React.useRef(false);\n+ if (!didErroRef.current) {\n+ didErroRef.current = true;\n+ console.error('test-only:one-time-error');\n+ }\n+ return null;\n+ }\n+ withErrorsOrWarningsIgnored(['test-only:'], () =>\n+ utils.act(() =>\n+ legacyRender(\n+ \n+ \n+ \n+ \n+ \n+ <\/React.Fragment>,\n+ container,\n+ ),\n+ ),\n+ );\n+\n+ let renderer;\n+ utils.act(() => (renderer = TestRenderer.create()));\n+ expect(state).toMatchInlineSnapshot(`\n+ ✕ 2, ⚠ 0\n+ [root]\n+ \n+ ✕\n+ \n+ ✕\n+ `);\n+\n+ \/\/ Select a child\n+ selectNextErrorOrWarning();\n+ utils.act(() => renderer.update());\n+ expect(state).toMatchInlineSnapshot(`\n+ ✕ 2, ⚠ 0\n+ [root]\n+ \n+ → ✕\n+ \n+ ✕\n+ `);\n+\n+ \/\/ Re-order the tree and ensure indices are updated.\n+ withErrorsOrWarningsIgnored(['test-only:'], () =>\n+ utils.act(() =>\n+ legacyRender(\n+ \n+ \n+ \n+ \n+ \n+ <\/React.Fragment>,\n+ container,\n+ ),\n+ ),\n+ );\n+ expect(state).toMatchInlineSnapshot(`\n+ ✕ 2, ⚠ 0\n+ [root]\n+ → ✕\n+ \n+ ✕\n+ \n+ `);\n+\n+ \/\/ Select the next child and ensure the index doesn't break.\n+ selectNextErrorOrWarning();\n+ expect(state).toMatchInlineSnapshot(`\n+ ✕ 2, ⚠ 0\n+ [root]\n+ ✕\n+ \n+ → ✕\n+ \n+ `);\n+\n+ \/\/ Re-order the tree and ensure indices are updated.\n+ withErrorsOrWarningsIgnored(['test-only:'], () =>\n+ utils.act(() =>\n+ legacyRender(\n+ \n+ \n+ \n+ \n+ \n+ <\/React.Fragment>,\n+ container,\n+ ),\n+ ),\n+ );\n+ expect(state).toMatchInlineSnapshot(`\n+ ✕ 2, ⚠ 0\n+ [root]\n+ → ✕\n+ ✕\n+ \n+ \n+ `);\n+ });\n+\n it('should update select and auto-expand parts components within hidden parts of the tree', () => {\n const Wrapper = ({children}) => children;\n \ndiff --git a\/packages\/react-devtools-shared\/src\/devtools\/store.js b\/packages\/react-devtools-shared\/src\/devtools\/store.js\nindex 7cd5c41e8b..217abe7f72 100644\n--- a\/packages\/react-devtools-shared\/src\/devtools\/store.js\n+++ b\/packages\/react-devtools-shared\/src\/devtools\/store.js\n@@ -57,6 +57,8 @@ const LOCAL_STORAGE_COLLAPSE_ROOTS_BY_DEFAULT_KEY =\n const LOCAL_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY =\n 'React::DevTools::recordChangeDescriptions';\n \n+type ErrorAndWarningTuples = Array<{|id: number, index: number|}>;\n+\n type Config = {|\n checkBridgeProtocolCompatibility?: boolean,\n isProfiling?: boolean,\n@@ -94,7 +96,7 @@ export default class Store extends EventEmitter<{|\n \/\/ Computed whenever _errorsAndWarnings Map changes.\n _cachedErrorCount: number = 0;\n _cachedWarningCount: number = 0;\n- _cachedErrorAndWarningTuples: Array<{|id: number, index: number|}> = [];\n+ _cachedErrorAndWarningTuples: ErrorAndWarningTuples | null = null;\n \n \/\/ Should new nodes be collapsed by default when added to the tree?\n _collapseNodesByDefault: boolean = true;\n@@ -510,7 +512,34 @@ export default class Store extends EventEmitter<{|\n \n \/\/ Returns a tuple of [id, index]\n getElementsWithErrorsAndWarnings(): Array<{|id: number, index: number|}> {\n- return this._cachedErrorAndWarningTuples;\n+ if (this._cachedErrorAndWarningTuples !== null) {\n+ return this._cachedErrorAndWarningTuples;\n+ } else {\n+ const errorAndWarningTuples: ErrorAndWarningTuples = [];\n+\n+ this._errorsAndWarnings.forEach((_, id) => {\n+ const index = this.getIndexOfElementID(id);\n+ if (index !== null) {\n+ let low = 0;\n+ let high = errorAndWarningTuples.length;\n+ while (low < high) {\n+ const mid = (low + high) >> 1;\n+ if (errorAndWarningTuples[mid].index > index) {\n+ high = mid;\n+ } else {\n+ low = mid + 1;\n+ }\n+ }\n+\n+ errorAndWarningTuples.splice(low, 0, {id, index});\n+ }\n+ });\n+\n+ \/\/ Cache for later (at least until the tree changes again).\n+ this._cachedErrorAndWarningTuples = errorAndWarningTuples;\n+\n+ return errorAndWarningTuples;\n+ }\n }\n \n getErrorAndWarningCountForElementID(\n@@ -1124,6 +1153,9 @@ export default class Store extends EventEmitter<{|\n \n this._revision++;\n \n+ \/\/ Any time the tree changes (e.g. elements added, removed, or reordered) cached inidices may be invalid.\n+ this._cachedErrorAndWarningTuples = null;\n+\n if (haveErrorsOrWarningsChanged) {\n let errorCount = 0;\n let warningCount = 0;\n@@ -1135,28 +1167,6 @@ export default class Store extends EventEmitter<{|\n \n this._cachedErrorCount = errorCount;\n this._cachedWarningCount = warningCount;\n-\n- const errorAndWarningTuples: Array<{|id: number, index: number|}> = [];\n-\n- this._errorsAndWarnings.forEach((_, id) => {\n- const index = this.getIndexOfElementID(id);\n- if (index !== null) {\n- let low = 0;\n- let high = errorAndWarningTuples.length;\n- while (low < high) {\n- const mid = (low + high) >> 1;\n- if (errorAndWarningTuples[mid].index > index) {\n- high = mid;\n- } else {\n- low = mid + 1;\n- }\n- }\n-\n- errorAndWarningTuples.splice(low, 0, {id, index});\n- }\n- });\n-\n- this._cachedErrorAndWarningTuples = errorAndWarningTuples;\n }\n \n if (haveRootsChanged) {\n@@ -1187,18 +1197,6 @@ export default class Store extends EventEmitter<{|\n console.groupEnd();\n }\n \n- const indicesOfCachedErrorsOrWarningsAreStale =\n- !haveErrorsOrWarningsChanged &&\n- (addedElementIDs.length > 0 || removedElementIDs.size > 0);\n- if (indicesOfCachedErrorsOrWarningsAreStale) {\n- this._cachedErrorAndWarningTuples.forEach(entry => {\n- const index = this.getIndexOfElementID(entry.id);\n- if (index !== null) {\n- entry.index = index;\n- }\n- });\n- }\n-\n this.emit('mutated', [addedElementIDs, removedElementIDs]);\n };\n \n","files":[{"path":"packages\/react-devtools-shared\/src\/__tests__\/treeContext-test.js","status":"M"},{"path":"packages\/react-devtools-shared\/src\/devtools\/store.js","status":"M"}]}

dillon1000/react

Files changed