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.
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
remove superjson dependency #274
remove superjson dependency #274
Changes from 3 commits
282eeed
2bde0f7
909bff1
4f5d5de
14e007f
301bdaa
ecd19c3
b7c0118
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
"revive" sorta makes it sound like something died and you're trying to bring it back to life. How about
parse
as the verb here since that's a typical term used for going from string -> structured type?I'm assuming this is the function called for deserialization from the result of
stringifyForStream
correct? SincestringifyForStream
always returns astring
, should the argument here always take a string?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.
No, the argument is already an object. The
string
fromstringifyForStream
gets directly injected into the stream, which makes it end up in JavaScript code. (The browser essentiallyeval
s it)So if the output of
stringifyForStream
is"{ \"foo\": 1 }"
, thenreviveFromStream
will be called with an object{ foo: 1 }
.That's also why our own
revive
implementation is a no-op: nothing needs to be done. OurstringifyForStream
implementation stringifies the object{ foo: 1, bar: undefined }
as"{ \"foo\":1, \"bar\": undefined }"
(which is not valid JSON, but valid JavaScript) - andrevive
will then be called with the{ foo: 1, bar: undefined }
object again.revive
is just "adding additional life" to an already parsed/eval'ed object.So you could have a
stringifyForStream
implementation that converts{ foo: 1, bar: undefined }
to"{ \"foo\":1, \"bar\": \"$u\" }"
andrevive
would then be called with{ foo: 1, bar: "$u" }
and would be expected to swap out the"$u"
withundefined
.Something like that can be useful with e.g.
superjson
which allows you to transport over custom types, but it's not required for our implementation or something likeserialize-javascript
, which just outputs valid JavaScript:{ foo: new Date() }
is just serialized as"{\"foo\":new Date(\"2024-04-09T08:49:42.044Z\")}"
.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.
That's kinda the point. We have a half-alive zombie object that maybe needs some last finishing touches. I thought about
rehydrate
, but that's very confusing as React is already taken the name.SuperJSON uses
deserialize
here, which is a tad better thanparse
, but I'm still not super happy with it.Realistically: I don't expect that a few people will ever use this.
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.
I just shared this yak shave with my RKT co-maintainers and @EskiMojo14 pointed out that
JSON.parse
has areviver
function argument. I probably took the name from there? 🤔