Skip to content

Conversation

@volodymyr-kushnir
Copy link

@volodymyr-kushnir volodymyr-kushnir commented Oct 22, 2025

This fixes a nesting issue I've originally hit with the ajv resolver (albeit the issue itself is applicable to all resolvers) which is that toNestErrors may lose keys depending on the order of the keys returned by the parseErrorSchema, e.g. in this particular example the error for the config is actually last (the console output of parsedErrors is sorted thus a bit misleading) and simply replaces the config: { type: …, userId: … } object that existed one nesting step earlier —

image

I was explaining this to my coworkers so I have a screencast (100 seconds) that illustrates the issue, attaching it here in hope that it'd explain the changes behind the PR better than my writing —

resolvers.mp4

Also I was not very comfortable with passing the objects straight to Object.assign, I think I've hit a bug with objects being mutated in place, so even though I can no longer reproduce it I've still wrapped "foreign" objects in a structuredClone, just to be on a safe side.

@jorisre jorisre requested a review from Copilot November 6, 2025 16:04
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the toNestErrors function to use structuredClone for deep cloning error objects, preventing potential mutation issues. The change ensures that error objects and field errors are properly cloned before being merged, which addresses scenarios where nested errors could be incorrectly shared or mutated.

  • Uses structuredClone to create deep copies of error objects and field errors
  • Adjusts the order of Object.assign parameters in the non-field-array case to properly merge existing field errors
  • Adds test coverage for nested field errors (name.firstName)

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
src/toNestErrors.ts Introduces structuredClone for deep cloning errors and field errors; reorders Object.assign parameters in the else branch
src/tests/toNestErrors.ts Adds test case for nested field name 'name.firstName'
src/tests/snapshots/toNestErrors.ts.snap Updates snapshots to include expected nested firstName field in both test cases

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@volodymyr-kushnir
Copy link
Author

Fair enough, I've removed structuredClone due to "target" being "ES2017" and left it like it was originally. If I manage to reproduce the issue I was once observing I'll circle back to this. I've tried to quickly plumb it with the JSON.parse(JSON.stringify(…)) technique but an array of say [empty × 3, {…}] is getting stringified as [null,null,null,{}] which doesn't play well down the road (resulting nested object gets "0":null key-value pair(s), etc.).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant