-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix(router-core): safely format Standard Schema validation issues #7782
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
TemRevil
wants to merge
1
commit into
TanStack:main
Choose a base branch
from
TemRevil:fix/safe-standard-schema-issue-formatting
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+163
−4
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| import type { AnyStandardSchemaValidateIssue } from './validators' | ||
|
|
||
| type IssuePathSegment = PropertyKey | { readonly key: PropertyKey } | ||
|
|
||
| function formatIssuePathSegment(segment: IssuePathSegment): string { | ||
| const key = | ||
| typeof segment === 'object' && segment !== null && 'key' in segment | ||
| ? segment.key | ||
| : segment | ||
|
|
||
| if (typeof key === 'number') { | ||
| return `[${key}]` | ||
| } | ||
| if (typeof key === 'symbol') { | ||
| return `[${key.toString()}]` | ||
| } | ||
| // String keys are always bracketed and quoted. This keeps the path | ||
| // unambiguous when a key itself contains "." or "[" (so a literal key | ||
| // "a.b" can't be confused with the nested path a -> b), and it means a | ||
| // key such as "__proto__" is only ever rendered as text, never used to | ||
| // index into an object. | ||
| return `[${JSON.stringify(key)}]` | ||
| } | ||
|
|
||
| function formatIssuePath( | ||
| path: ReadonlyArray<IssuePathSegment> | undefined, | ||
| ): string { | ||
| if (!path || path.length === 0) { | ||
| return '(root)' | ||
| } | ||
| return path.map(formatIssuePathSegment).join('') | ||
| } | ||
|
|
||
| /** | ||
| * Format Standard Schema validation issues into a readable string. | ||
| * | ||
| * Issue objects can hold values that `JSON.stringify` cannot serialize | ||
| * (symbols in paths, circular references), in which case stringifying them | ||
| * throws and hides the original validation failure. This walks the issues | ||
| * defensively and only ever reads the message and path, so the real | ||
| * validation errors survive. | ||
| * | ||
| * Intended for internal use across router-core and start-client-core; it is | ||
| * not part of the documented public API. | ||
| */ | ||
| export function formatValidationError( | ||
| issues: ReadonlyArray<AnyStandardSchemaValidateIssue> | undefined, | ||
| ): string { | ||
| if (!issues || issues.length === 0) { | ||
| return 'Validation failed' | ||
| } | ||
|
|
||
| return issues | ||
| .map((issue) => { | ||
| const path = formatIssuePath(issue.path) | ||
| const message = | ||
| typeof issue.message === 'string' | ||
| ? issue.message | ||
| : String(issue.message) | ||
| return `${path}: ${message}` | ||
| }) | ||
| .join('\n') | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| import { describe, expect, test } from 'vitest' | ||
| import { formatValidationError } from '../src/validationError' | ||
| import type { AnyStandardSchemaValidateIssue } from '../src/validators' | ||
|
|
||
| describe('formatValidationError', () => { | ||
| test('falls back when there are no issues', () => { | ||
| expect(formatValidationError([])).toBe('Validation failed') | ||
| expect(formatValidationError(undefined)).toBe('Validation failed') | ||
| }) | ||
|
|
||
| test('renders a root issue without a path', () => { | ||
| const issues: Array<AnyStandardSchemaValidateIssue> = [ | ||
| { message: 'Required' }, | ||
| ] | ||
| expect(formatValidationError(issues)).toBe('(root): Required') | ||
| }) | ||
|
|
||
| test('renders a nested string path', () => { | ||
| const issues: Array<AnyStandardSchemaValidateIssue> = [ | ||
| { message: 'Expected string', path: ['user', 'name'] }, | ||
| ] | ||
| expect(formatValidationError(issues)).toBe( | ||
| '["user"]["name"]: Expected string', | ||
| ) | ||
| }) | ||
|
|
||
| test('renders numeric (array index) path segments', () => { | ||
| const issues: Array<AnyStandardSchemaValidateIssue> = [ | ||
| { message: 'Expected number', path: ['scores', 0] }, | ||
| ] | ||
| expect(formatValidationError(issues)).toBe('["scores"][0]: Expected number') | ||
| }) | ||
|
|
||
| test('reads path segments given as { key } objects', () => { | ||
| const issues: Array<AnyStandardSchemaValidateIssue> = [ | ||
| { message: 'Invalid', path: [{ key: 'a' }, { key: 1 }] }, | ||
| ] | ||
| expect(formatValidationError(issues)).toBe('["a"][1]: Invalid') | ||
| }) | ||
|
|
||
| test('does not confuse a literal dotted key with a nested path', () => { | ||
| const flat: Array<AnyStandardSchemaValidateIssue> = [ | ||
| { message: 'x', path: ['a.b'] }, | ||
| ] | ||
| const nested: Array<AnyStandardSchemaValidateIssue> = [ | ||
| { message: 'x', path: ['a', 'b'] }, | ||
| ] | ||
| expect(formatValidationError(flat)).not.toBe(formatValidationError(nested)) | ||
| expect(formatValidationError(flat)).toBe('["a.b"]: x') | ||
| }) | ||
|
|
||
| test('renders a prototype-named key as text without indexing into objects', () => { | ||
| const issues: Array<AnyStandardSchemaValidateIssue> = [ | ||
| { message: 'nope', path: ['__proto__', 'polluted'] }, | ||
| ] | ||
| expect(formatValidationError(issues)).toBe( | ||
| '["__proto__"]["polluted"]: nope', | ||
| ) | ||
| }) | ||
|
|
||
| test('renders symbol path segments instead of throwing', () => { | ||
| const sym = Symbol('id') | ||
| const issues: Array<AnyStandardSchemaValidateIssue> = [ | ||
| { message: 'bad symbol key', path: [sym] }, | ||
| ] | ||
| expect(formatValidationError(issues)).toBe( | ||
| `[${sym.toString()}]: bad symbol key`, | ||
| ) | ||
| }) | ||
|
|
||
| test('joins multiple issues on separate lines', () => { | ||
| const issues: Array<AnyStandardSchemaValidateIssue> = [ | ||
| { message: 'Required', path: ['a'] }, | ||
| { message: 'Too short', path: ['b', 2] }, | ||
| ] | ||
| expect(formatValidationError(issues)).toBe( | ||
| '["a"]: Required\n["b"][2]: Too short', | ||
| ) | ||
| }) | ||
|
|
||
| test('does not throw on a circular issue object', () => { | ||
| const circular: any = { message: 'circular' } | ||
| circular.self = circular | ||
| expect(() => formatValidationError([circular])).not.toThrow() | ||
| expect(formatValidationError([circular])).toBe('(root): circular') | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add curly braces to the
ifstatement.Per the project's coding guidelines for
**/*.{ts,tsx,js,jsx}: "Always use curly braces forif,else, loops, and similar control statements. Never write one-line bodies likeif (foo) x = 1."♻️ Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents
Source: Coding guidelines