Skip to content

Commit 0287bc8

Browse files
committed
fix: treat non-finite coordinates as needing layout, cover ELK rejection
Addresses review feedback on #14258. `needsLayout` used a typeof + NaN check, which let `Infinity` through: it is a number and is not NaN, so a node with an infinite coordinate skipped the synchronous seeding and could still reach React Flow as an unusable position. `Number.isFinite` does not coerce, so it subsumes the typeof check while also rejecting NaN and ±Infinity. Also adds the missing error-path coverage for the ELK fallback: the previous tests only exercised successful layouts, never the catch branch. The new test mocks elk.layout to reject and asserts getLayoutedNodes resolves to the deterministic grid rather than propagating. Both tests fail without their corresponding fix.
1 parent 9702b33 commit 0287bc8

2 files changed

Lines changed: 52 additions & 4 deletions

File tree

src/frontend/src/utils/__tests__/layoutUtils.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,19 @@ describe("needsLayout", () => {
7474
expect(needsLayout([node])).toBe(true);
7575
});
7676

77+
it("flags non-finite coordinates", () => {
78+
const positive = {
79+
...makeNode("A"),
80+
position: { x: Number.POSITIVE_INFINITY, y: 0 },
81+
};
82+
const negative = {
83+
...makeNode("B"),
84+
position: { x: 0, y: Number.NEGATIVE_INFINITY },
85+
};
86+
expect(needsLayout([positive])).toBe(true);
87+
expect(needsLayout([negative])).toBe(true);
88+
});
89+
7790
it("accepts well-formed positions", () => {
7891
const node = { ...makeNode("A"), position: { x: 0, y: 0 } };
7992
expect(needsLayout([node])).toBe(false);
@@ -103,6 +116,41 @@ describe("getLayoutedNodes with handle-less edges", () => {
103116
});
104117
});
105118

119+
describe("getLayoutedNodes when ELK rejects", () => {
120+
afterEach(() => {
121+
jest.dontMock("elkjs/lib/elk.bundled.js");
122+
jest.resetModules();
123+
jest.restoreAllMocks();
124+
});
125+
126+
it("falls back to the deterministic grid instead of propagating", async () => {
127+
jest.resetModules();
128+
jest.doMock("elkjs/lib/elk.bundled.js", () => ({
129+
__esModule: true,
130+
default: class {
131+
layout = jest.fn().mockRejectedValue(new Error("ELK exploded"));
132+
},
133+
}));
134+
jest.spyOn(console, "error").mockImplementation(() => {});
135+
136+
// Re-require so the module picks up the failing ELK singleton.
137+
const {
138+
getFallbackGridPositions: freshFallback,
139+
getLayoutedNodes: withFailingElk,
140+
} = require("../layoutUtils");
141+
142+
const nodes = ["ChatInput-a", "Prompt-b", "ChatOutput-c"].map(makeNode);
143+
const edges = [makeHandlelessEdge("ChatInput-a", "Prompt-b")];
144+
145+
const layouted = await withFailingElk(nodes, edges);
146+
147+
// Resolves rather than rejecting, and every node is usable.
148+
expect(layouted).toHaveLength(3);
149+
expect(layouted.every(hasNumericPosition)).toBe(true);
150+
expect(layouted).toEqual(freshFallback(nodes));
151+
});
152+
});
153+
106154
describe("processFlows on a flow with no positions", () => {
107155
const makeFlow = () =>
108156
({

src/frontend/src/utils/reactflowUtils.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -609,13 +609,13 @@ export const processFlows = (DbData: FlowType[], skipUpdate = true) => {
609609
};
610610

611611
export const needsLayout = (nodes: AllNodeType[]) => {
612+
// Number.isFinite does not coerce, so this also rejects non-numeric
613+
// coordinates as well as NaN and ±Infinity.
612614
return nodes.some(
613615
(node) =>
614616
!node.position ||
615-
typeof node.position.x !== "number" ||
616-
typeof node.position.y !== "number" ||
617-
Number.isNaN(node.position.x) ||
618-
Number.isNaN(node.position.y),
617+
!Number.isFinite(node.position.x) ||
618+
!Number.isFinite(node.position.y),
619619
);
620620
};
621621

0 commit comments

Comments
 (0)