a repair that cannot stick must not be written
useAuthoredLayout turned a harmless fallback into a hard crash, and /browser on fofos is where it showed: "This screen failed to render", react #185, maximum update depth exceeded. WorkspaceView's appTypes guard repairs an unrecognised appType and WRITES THE REPAIR BACK. that was self-limiting while the structure was persisted — the repair stuck and the next render agreed with it. with the structure coming from source, it can never stick: recomputed, rewritten, discarded, recomputed, every render, until react gives up. the browser plugin's layout names officerdev/chat, which is not one of its own panel keys, so every single render of that screen produced a repair. the same trap proxy-manager hit last night, except that one only rendered the wrong panel — this one loops. so a write whose result equals the current value is dropped. not an optimisation: it is the thing that terminates the loop. persisting an identical tree was pointless anyway. tested by grafting a repaired structure and asserting it comes back identical to the current value — which is the exact condition setValue now checks, so if it ever writes there, the loop is back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043
This commit is contained in:
@@ -96,3 +96,34 @@ describe('applySizes', () => {
|
|||||||
expect(JSON.stringify(source)).toBe(before);
|
expect(JSON.stringify(source)).toBe(before);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The loop that broke `/browser`.
|
||||||
|
*
|
||||||
|
* `WorkspaceView` repairs an appType it does not recognise and writes the repair back. With the structure
|
||||||
|
* coming from source, that repair can never persist — so unless an identical write is dropped, it is
|
||||||
|
* recomputed and rewritten every render until React gives up (#185).
|
||||||
|
*/
|
||||||
|
describe('a repair that cannot stick must not be written', () => {
|
||||||
|
test('grafting a repaired structure yields exactly the current value', () => {
|
||||||
|
// What the guard produces: the same tree with a disallowed appType swapped for the fallback.
|
||||||
|
const repaired = group('root', [
|
||||||
|
{
|
||||||
|
node: group('lists', [
|
||||||
|
{ node: panel('hosts', 'proxy-manager-hosts'), size: 62 },
|
||||||
|
{ node: panel('certs', 'proxy-manager-certificates'), size: 38 },
|
||||||
|
]),
|
||||||
|
size: 66,
|
||||||
|
},
|
||||||
|
// `chat` replaced by the fallback — the substitution the allow-list makes every render.
|
||||||
|
{ node: panel('chat', 'proxy-manager-hosts'), size: 34 },
|
||||||
|
]);
|
||||||
|
|
||||||
|
const current = applySizes(source, collectSizes(source));
|
||||||
|
const afterRepair = applySizes(source, collectSizes(repaired));
|
||||||
|
|
||||||
|
// Identical, because only sizes survive and the repair changed none. `setValue` compares these and
|
||||||
|
// returns without writing; if it ever writes, the render loop is back.
|
||||||
|
expect(JSON.stringify(afterRepair)).toBe(JSON.stringify(current));
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -93,7 +93,27 @@ export function useAuthoredLayout(key: string, source: LayoutNode) {
|
|||||||
const setValue = useCallback(
|
const setValue = useCallback(
|
||||||
(update: LayoutNode | ((prev: LayoutNode) => LayoutNode)) => {
|
(update: LayoutNode | ((prev: LayoutNode) => LayoutNode)) => {
|
||||||
const next = typeof update === 'function' ? update(value) : update;
|
const next = typeof update === 'function' ? update(value) : update;
|
||||||
stored.setValue(applySizes(source, collectSizes(next)));
|
const merged = applySizes(source, collectSizes(next));
|
||||||
|
|
||||||
|
/*
|
||||||
|
A write that changes nothing is dropped, and this is not an optimisation — it is what stops an
|
||||||
|
infinite render loop.
|
||||||
|
|
||||||
|
`WorkspaceView`'s `appTypes` guard repairs an unrecognised appType and WRITES THE REPAIR BACK. That
|
||||||
|
was self-limiting while the structure was persisted: the repair stuck and the next render agreed
|
||||||
|
with it. Here the structure always comes from `source`, so the repair can never stick — it is
|
||||||
|
recomputed, rewritten, discarded and recomputed, every render, forever. React gives up with
|
||||||
|
"maximum update depth exceeded" (#185) and the screen fails to render.
|
||||||
|
|
||||||
|
Not hypothetical: the `browser` plugin's layout names `officerdev/chat`, which is not one of its own
|
||||||
|
panel keys, so every render of `/browser` produced a repair. Shipping the sizes-only hook turned a
|
||||||
|
silent, harmless fallback into a hard crash on that screen.
|
||||||
|
|
||||||
|
Comparing the serialised value is enough — these trees are small, this runs only on a real change,
|
||||||
|
and it is the same check `setValue` would otherwise force React Query to make against a new object.
|
||||||
|
*/
|
||||||
|
if (JSON.stringify(merged) === JSON.stringify(value)) return;
|
||||||
|
stored.setValue(merged);
|
||||||
},
|
},
|
||||||
// `stored.setValue` is stable per key; `value` is memoised above.
|
// `stored.setValue` is stable per key; `value` is memoised above.
|
||||||
[source, value, stored],
|
[source, value, stored],
|
||||||
|
|||||||
Reference in New Issue
Block a user