diff --git a/packages/next/src/next-devtools/dev-overlay/segment-explorer-trie.test.tsx b/packages/next/src/next-devtools/dev-overlay/segment-explorer-trie.test.tsx index 8b05181b2f02..8845f3b82d69 100644 --- a/packages/next/src/next-devtools/dev-overlay/segment-explorer-trie.test.tsx +++ b/packages/next/src/next-devtools/dev-overlay/segment-explorer-trie.test.tsx @@ -24,6 +24,7 @@ const createSegmentNode = ({ } describe('Segment Explorer', () => { + let act: typeof import('@testing-library/react').act let cleanup: typeof import('@testing-library/react').cleanup let renderHook: typeof import('@testing-library/react').renderHook let useSegmentTree: typeof SegmentExplorer.useSegmentTree @@ -42,6 +43,7 @@ describe('Segment Explorer', () => { const rtl = require('@testing-library/react/pure') renderHook = rtl.renderHook cleanup = rtl.cleanup + act = rtl.act }) afterEach(() => { @@ -154,9 +156,11 @@ describe('Segment Explorer', () => { value: undefined, }) - removeSegmentNode( - createSegmentNode({ pagePath: '/constructor/page.js', type: 'page' }) - ) + act(() => { + removeSegmentNode( + createSegmentNode({ pagePath: '/constructor/page.js', type: 'page' }) + ) + }) expect(result.current).toEqual({ children: { @@ -274,9 +278,11 @@ describe('Segment Explorer', () => { value: undefined, }) - removeSegmentNode( - createSegmentNode({ pagePath: '/a/b/layout.js', type: 'layout' }) - ) + act(() => { + removeSegmentNode( + createSegmentNode({ pagePath: '/a/b/layout.js', type: 'layout' }) + ) + }) expect(result.current).toEqual({ children: { @@ -340,4 +346,38 @@ describe('Segment Explorer', () => { value: undefined, }) }) + + test('gives changed nodes a new identity so consumers can memoize on them', () => { + insertSegmentNode( + createSegmentNode({ pagePath: '/a/layout.js', type: 'layout' }) + ) + + const { result } = renderHook(useSegmentTree) + + // `pagePath` splits on '/', so the leading slash produces an empty segment. + const rootBefore = result.current + const aBefore = rootBefore.children['']!.children['a']! + + act(() => { + insertSegmentNode( + createSegmentNode({ pagePath: '/a/page.js', type: 'page' }) + ) + }) + + const rootAfter = result.current + const aAfter = rootAfter.children['']!.children['a']! + + // Consumers memoize on `node.children`, so every node along the mutated + // path must be a fresh object. Reusing them would leave newly inserted + // segments invisible until the consumer remounted. + expect(rootAfter).not.toBe(rootBefore) + expect(rootAfter.children).not.toBe(rootBefore.children) + expect(aAfter).not.toBe(aBefore) + expect(aAfter.children).not.toBe(aBefore.children) + expect(Object.keys(aAfter.children)).toEqual(['layout.js', 'page.js']) + + // Untouched subtrees stay shared, and the previous snapshot is not mutated. + expect(Object.keys(aBefore.children)).toEqual(['layout.js']) + expect(aAfter.children['layout.js']).toBe(aBefore.children['layout.js']) + }) }) diff --git a/packages/next/src/next-devtools/dev-overlay/segment-explorer-trie.ts b/packages/next/src/next-devtools/dev-overlay/segment-explorer-trie.ts index 99938333027e..ad1918362250 100644 --- a/packages/next/src/next-devtools/dev-overlay/segment-explorer-trie.ts +++ b/packages/next/src/next-devtools/dev-overlay/segment-explorer-trie.ts @@ -75,55 +75,81 @@ function createTrie({ } } - function insert(value: Value) { - let currentNode = root - const segments = getCharacters(value) + function copyChildren(children: TrieNode['children']) { + return Object.assign(Object.create(null), children) + } + // Snapshots must be immutable for `useSyncExternalStore` consumers, so + // updates copy the nodes along the mutated path instead of mutating them + // in place. Untouched subtrees stay shared. + function copyPath(segments: string[]): TrieNode[] { + const newRoot: TrieNode = { + value: root.value, + children: copyChildren(root.children), + } + + const path: TrieNode[] = [newRoot] + let currentNode = newRoot for (const segment of segments) { - if (!currentNode.children[segment]) { - currentNode.children[segment] = { - value: undefined, - // Skip value for intermediate nodes - children: Object.create(null), - } + const existingNode = currentNode.children[segment] + const copiedNode: TrieNode = { + value: existingNode?.value, + children: existingNode + ? copyChildren(existingNode.children) + : Object.create(null), } - currentNode = currentNode.children[segment] + currentNode.children[segment] = copiedNode + currentNode = copiedNode + path.push(copiedNode) } - currentNode.value = value + return path + } - root = { ...root } + function insert(value: Value) { + const segments = getCharacters(value) + const path = copyPath(segments) + + path[path.length - 1].value = value + + root = path[0] markUpdated() } function remove(value: Value) { - let currentNode = root const segments = getCharacters(value) - const stack: TrieNode[] = [] - let found = true + // Locate the node first, so a miss doesn't copy or notify. + let currentNode = root for (const segment of segments) { - if (!currentNode.children[segment]) { - found = false - break + const childNode = currentNode.children[segment] + if (!childNode) { + return } - stack.push(currentNode) - currentNode = currentNode.children[segment]! + currentNode = childNode } // If the value is not found, skip removal - if (!found || !compare(currentNode.value, value)) { + if (!compare(currentNode.value, value)) { return } - currentNode.value = undefined - for (let i = stack.length - 1; i >= 0; i--) { - const parentNode = stack[i] + + const path = copyPath(segments) + path[path.length - 1].value = undefined + + // Prune nodes that no longer hold a value or any children. + for (let i = segments.length - 1; i >= 0; i--) { + const parentNode = path[i] const segment = segments[i] - if (Object.keys(parentNode.children[segment]!.children).length === 0) { + const childNode = parentNode.children[segment]! + if ( + childNode.value === undefined && + Object.keys(childNode.children).length === 0 + ) { delete parentNode.children[segment] } } - root = { ...root } + root = path[0] markUpdated() }