perf: optimize traversals with a very large number of errors #1172
Conversation
|
Possibly I forgot the |
Dimava
force-pushed
the
dimava/lost-in-errors
branch
from
feccc9e
to
ecc6025
Compare
ark/schema/shared/traversal.ts
Outdated
| const propString = pathToPropString(path) | ||
| return this.errors.some(e => propString.startsWith(e.propString)) | ||
| for (let i = 0; i <= path.length; i++) { | ||
| const partialPropString = pathToPropString(path.slice(0, i)) |
There was a problem hiding this comment.
pathToPropString already has to iterate the path. Look at that logic and reuse the internal methods so build it here as you go so this isn't n^2
ssalbdivad
changed the title
ark/schema/shared/traversal.ts
Outdated
| const propString = pathToPropString(path) | ||
| return this.errors.some(e => propString.startsWith(e.propString)) | ||
| let partialPropString: string = "" | ||
| if (Object.hasOwn(this.errors.byPath, partialPropString)) return true |
There was a problem hiding this comment.
Can you change this to use in and then change errorsByPath to initialize to Object.create(null)?
You are right to consider that edge case but I think solving it in the places where we're using records is better, see #1121
| export const appendPropToPathString: AppendPropToPathStringFn = ( | ||
| path, | ||
| prop, | ||
| ...[opts] |
There was a problem hiding this comment.
Can this just be opts without TS complaining? The signature there is really just a TS trick to force opts to be passed in some situations.
There was a problem hiding this comment.
Not without anys
It's possible to make it a function with overloads and loose implementation though
There was a problem hiding this comment.
Actually I can make it required here, as it's internal anyways
No description provided.