-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
error fixed #8109
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
base: dev-2.0
Are you sure you want to change the base?
error fixed #8109
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -64,7 +64,7 @@ function validateParams(p5, fn, lifecycles) { | |
'Boolean': z.boolean(), | ||
'Function': z.function(), | ||
'Integer': z.number().int(), | ||
'Number': z.number(), | ||
'Number': z.union([z.number(), z.literal(Infinity), z.literal(-Infinity)]), | ||
'Object': z.object({}), | ||
'String': z.string() | ||
}; | ||
|
@@ -413,6 +413,7 @@ function validateParams(p5, fn, lifecycles) { | |
const processUnionError = error => { | ||
const expectedTypes = new Set(); | ||
let actualType; | ||
let hasConstants = false; | ||
|
||
error.errors.forEach(err => { | ||
const issue = err[0]; | ||
|
@@ -425,12 +426,25 @@ function validateParams(p5, fn, lifecycles) { | |
actualType = issue.message.split(', received ')[1]; | ||
expectedTypes.add(issue.expected); | ||
} | ||
// Handle nested union errors (e.g., Number type with number, Infinity, -Infinity) | ||
else if (issue.code === 'invalid_union') { | ||
issue.errors.forEach(nestedErr => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The nested failure |
||
const nestedIssue = nestedErr[0]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
if (nestedIssue && nestedIssue.code === 'invalid_type') { | ||
expectedTypes.add(nestedIssue.expected); | ||
} | ||
}); | ||
} | ||
// The case for constants. Since we don't want to print out the actual | ||
// constant values in the error message, the error message will | ||
// direct users to the documentation. | ||
// Note: Distinguish actual constants from special number values like Infinity/-Infinity | ||
else if (issue.code === 'invalid_value') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like you are checking inside the condition for literals |
||
expectedTypes.add('constant (please refer to documentation for allowed values)'); | ||
actualType = args[error.path[0]]; | ||
const isSpecialNumber = issue.values && issue.values.some(val => val === Infinity || val === -Infinity); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You will never see literals, so you will always get |
||
if (!isSpecialNumber) { | ||
hasConstants = true; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since |
||
actualType = args[error.path[0]]; | ||
} | ||
} else if (issue.code === 'custom') { | ||
const match = issue.message.match(/Input not instance of (\w+)/); | ||
if (match) expectedTypes.add(match[1]); | ||
|
@@ -439,6 +453,11 @@ function validateParams(p5, fn, lifecycles) { | |
} | ||
}); | ||
|
||
// Only add constants to expected types if constant validation actually failed | ||
if (hasConstants) { | ||
expectedTypes.add('constant (please refer to documentation for allowed values)'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the tests are accidentally passing, since |
||
} | ||
|
||
if (expectedTypes.size > 0) { | ||
if (error.path?.length > 0 && args[error.path[0]] instanceof Promise) { | ||
message += 'Did you mean to put `await` before a loading function? ' + | ||
|
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.
was there's a reason to use this instead of
z.number().or(z.literal(Infinity))
as per dave's suggestion?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.
z.number().or(z.literal(Infinity)) only allows finite numbers and Infinity, but not -Infinity.
z.union([z.number(), z.literal(Infinity), z.literal(-Infinity)]) allows finite numbers, Infinity, AND -Infinity.
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.
Maybe this should be better alternative :
z.number().or(z.literal(Infinity)).or(z.literal(-Infinity));