Escape hatch added to defaultSerializeQueryArgs for unserializable data #4297
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 2639cea:
|
✅ Deploy Preview for redux-starter-kit-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -27,6 +29,12 @@ export const defaultSerializeQueryArgs: SerializeQueryArgs<any> = ({ | |||
| }, {}) | |||
| : value, | |||
| ) | |||
| } catch (e) { | |||
There was a problem hiding this comment.
(I'm relatively new to open source process && this project, so if I should keep my comments to the issue rather than the PR, please let me know -- or if I'm missing too much context to be useful)
I suspect that for most use cases, silently swallowing an error is worse than throwing it. We probably don't want a silent divergence in behavior based on the input type.
There was a problem hiding this comment.
This doesn't actually change the majority of the behaviour by just catching this. As seralizeQueryArgs still runs so if the user has provided a way to handle it changing, or if the query args is actually changing it will still behave as normal.
I have a test added to indicate that the base render assumptions don't change with this addition. Is that what you are referring to? or the actual handling. Because I definitely want guidance on the best approach to handling the error rather than my dev mode warning.
There was a problem hiding this comment.
Actually, I have a thought! Let me put together a PR.
fixes #4279
There's some discussion there about the expected behaviour and potential other fixes involving additional options or control for a user to override the defaultSerializeQueryArgs implementation.
I figured this is the best initial starting point because if
defaultQueryArgsdoes throw an error due to say BigInt being passed, it will just continue the process that involves running the user definedserializeQueryArgsanyway.I am not sure what the handling for the error should be. I presume a warning being sent every single time this happens especially if a user is aware it will happen isn't what we want? But I wanted to put something there for clarity.