perf: fix O(n²) allocation in jsonSchemaToZod required field handling#1857
perf: fix O(n²) allocation in jsonSchemaToZod required field handling#1857ctonneslan wants to merge 1 commit intobrowserbase:mainfrom
Conversation
…andling Replace the reduce with spread operator (which copies the entire accumulator on every iteration) with a simple for-of loop that mutates a single object. Also remove the redundant Array.isArray guard since schema.required is already typed as string[]. Fixes browserbase#1773
|
|
This PR is from an external contributor and must be approved by a stagehand team member with write access before CI can run. |
Greptile SummaryThis PR makes a focused, correct performance optimization to Key points:
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["jsonSchemaToZod(schema)"] --> B{schema.type === 'object'?}
B -- No --> C[Handle other types]
B -- Yes --> D{schema.properties?}
D -- No --> E["return z.object({})"]
D -- Yes --> F["Build shape via for...in loop\nO(n)"]
F --> G["zodObject = z.object(shape)"]
G --> H{schema.required?}
H -- No --> J{schema.description?}
H -- Yes --> I["Build requiredFields via for...of loop\nO(n) — no spread allocation"]
I --> K["zodObject = zodObject.partial().required(requiredFields)"]
K --> J
J -- No --> L["return zodObject"]
J -- Yes --> M["zodObject = zodObject.describe(description)"]
M --> L
Last reviewed commit: "perf: fix O(n²) obje..." |
There was a problem hiding this comment.
No issues found across 1 file
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Summary
Fixes two issues in
jsonSchemaToZodrequired field handling:O(n²) object allocation: The
reducewith spread operator ({ ...acc, [field]: true }) copies the entire accumulator on every iteration. Replaced with afor...ofloop that mutates a single object in O(n).Redundant type guard:
Array.isArray(schema.required)is unnecessary sinceschema.requiredis already typed asstring[]in theJsonSchemaPropertyinterface. The truthiness check alone is sufficient.Before
After
Fixes #1773
Summary by cubic
Improve performance of required-field handling in
jsonSchemaToZodby replacing the O(n²) reduce/spread pattern with an O(n) loop to cut allocations on large schemas. Also remove the redundantArray.isArray(schema.required)check sinceschema.requiredis typed asstring[].Written for commit 332c4fb. Summary will update on new commits. Review in cubic