-
Notifications
You must be signed in to change notification settings - Fork 38
Add undefined
to AllowedPropertyValues type
#174
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: main
Are you sure you want to change the base?
Conversation
undefined
to AllowedPropertyValues type
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Additional Comments:
packages/web/src/utils.ts (lines 72-72):
The error message in parseProperties
needs to be updated to include "undefined" as an allowed property value type.
View Details
📝 Patch Details
diff --git a/packages/web/src/utils.ts b/packages/web/src/utils.ts
index 8ca22ec..f4a9b32 100644
--- a/packages/web/src/utils.ts
+++ b/packages/web/src/utils.ts
@@ -68,7 +68,7 @@ export function parseProperties(
throw Error(
`The following properties are not valid: ${errorProperties.join(
', '
- )}. Only strings, numbers, booleans, and null are allowed.`
+ )}. Only strings, numbers, booleans, null, and undefined are allowed.`
);
}
return props as Record<string, AllowedPropertyValues>;
Analysis
Error Message Inconsistency in parseProperties Function
Bug Description
The parseProperties
function in packages/web/src/utils.ts
contains an error message that is inconsistent with the actual type definition. The AllowedPropertyValues
type supports undefined
values, and the validation logic correctly allows undefined
values to pass through, but the error message displayed to developers does not mention undefined
as an allowed value type.
How the Bug Manifests
When developers pass object or array values to parseProperties
, they receive an error message stating:
The following properties are not valid: [property_names]. Only strings, numbers, booleans, and null are allowed.
However, the actual AllowedPropertyValues
type definition includes undefined
:
export type AllowedPropertyValues =
| string
| number
| boolean
| null
| undefined;
Technical Analysis
The validation logic in parseProperties
correctly handles undefined
values:
for (const [key, value] of Object.entries(properties)) {
if (typeof value === 'object' && value !== null) {
// Only objects (not undefined) trigger the error
errorProperties.push(key);
}
}
Since typeof undefined === 'undefined'
(not 'object'
), undefined
values pass through validation without triggering the error message. The bug is purely in the misleading error message text.
Root Cause
Git history reveals that undefined
was recently added to the AllowedPropertyValues
type in commit 4c43f00871259115b02847f750eaa20ff705f481
, but the corresponding error message in the parseProperties
function was not updated to reflect this change.
Impact and Consequences
- Developer Confusion: Developers receive misleading error messages that don't accurately reflect what values are actually allowed
- Documentation Inconsistency: The error message serves as runtime documentation but is inconsistent with the type system
- Debugging Difficulty: Developers may waste time trying to "fix" undefined values that are actually valid
- Trust Issues: Inconsistent messaging between types and runtime errors undermines confidence in the API
Fix Implementation
Updated the error message in packages/web/src/utils.ts
line 70 to include undefined
:
throw Error(
`The following properties are not valid: ${errorProperties.join(
', '
)}. Only strings, numbers, booleans, null, and undefined are allowed.`
);
This change ensures the error message accurately reflects the current type definition and actual runtime behavior.
Validation
The fix has been tested to confirm:
- ✅
undefined
values continue to pass through validation (no behavior change) - ✅ Error message now correctly mentions
undefined
as an allowed value - ✅ All other validation behavior remains unchanged
- ✅ Error message is now consistent with the
AllowedPropertyValues
type definition
Make the type used inside
track
more forgiving.