-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
chore(ct): throw error when props are not json serializable #22025
Conversation
0b3258b
to
57175cb
Compare
57175cb
to
a1d6506
Compare
boolean: (value: any) => typeof value === 'boolean', | ||
null: (value: any) => value === null, | ||
array: (value: any) => Array.isArray(value) && value.every(isJson), | ||
object: (value: any) => typeof value === 'object' && value !== null && !Array.isArray(value) |
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.
JSON will have a default serializing conventions for types like Date, but I guess it is Ok to be more restrictive here.
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.
Not sure if i fully get that. Thought only the string
, number
, boolean
, null
, array
and object
types are JSON serializable? Do you mean that JSON.parse(JSON.stringify({ boop: Date }))
will return {}
?
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.
Look at what JSON.stringify({ data: new Date() })
produces, but don't bother.
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.
Fix those nits and we are good.
Done, also added the check for |
95ccaa6
to
1683fe5
Compare
1683fe5
to
2e2c67b
Compare
Related to: #21400, #22003