-
Notifications
You must be signed in to change notification settings - Fork 94
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
feat: add dedupe
flag
#247
Conversation
? { | ||
transformedValue: null, | ||
} |
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.
It'd be nice with some symbol here (and for circular references fwiw) to show in the JSON that we have replaced it
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.
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.
Implementation looks solid, i'd like to pause a moment to think about the name + location of that flag.
? { | ||
transformedValue: null, | ||
} |
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.
src/index.ts
Outdated
/** | ||
* If true, SuperJSON will make sure only one instance of referentially equal objects are serialized and the rest are replaced with `null`. | ||
*/ | ||
public dedupe = false; |
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.
My gut instinct was to put this as an optional parameter to serialize
, and call it something like pruneReferentialEqualities
, in reference to meta.referentialEqualities
. I'd also be fine with dedupeReferentialEqualities
. It's a lot longer of a name, but more descriptive I guess? But also harder to parse, so i'm torn between the two. Any strong feelings from your end on 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.
Adding an optional argument to serialize could be seen as a breaking change in cases like items.map(superjson.serialize.bind(superjson)
🙃
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 see where you're coming from, but I don't necessarily agree with that strict of a definition of "breaking change" 😅
You decide everything and me / @juliusmarminge will just follow suit. :) Feel free to just push to my PR or make another stab at it, I gotta go for today now! |
FWIW, changing the circular to anything else but |
Published as part of https://github.com/blitz-js/superjson/releases/tag/v1.13.0 |
Awesome. Unfortunately, for some reason I get an error when running the compiled version yelling that SuperJSON isn't a constructor 🤔 |
Can reproduce locally, let's see what's causing it 🤔 |
It looks like it's related to how TypeScript emits the |
Bundling javascript....
Seems reasonable. Nothing in this PR should cause difference in behavior in that regard |
As a workaround, could you try |
Oops, meant to replace the |
Made a temporary workaround that works if you use a named import instead of the default one in #250 🤷🏼♂️ |
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [superjson](https://togithub.com/blitz-js/superjson) | [`1.12.4` -> `1.13.1`](https://renovatebot.com/diffs/npm/superjson/1.12.4/1.13.1) | [![age](https://developer.mend.io/api/mc/badges/age/npm/superjson/1.13.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/superjson/1.13.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/superjson/1.12.4/1.13.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/superjson/1.12.4/1.13.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>blitz-js/superjson (superjson)</summary> ### [`v1.13.1`](https://togithub.com/blitz-js/superjson/releases/tag/v1.13.1) [Compare Source](https://togithub.com/blitz-js/superjson/compare/v1.13.0...v1.13.1) #### What's Changed - Bump json5 from 1.0.1 to 1.0.2 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/blitz-js/superjson/pull/218](https://togithub.com/blitz-js/superjson/pull/218) - Bump minimatch from 3.0.4 to 3.1.2 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/blitz-js/superjson/pull/213](https://togithub.com/blitz-js/superjson/pull/213) - Bump decode-uri-component from 0.2.0 to 0.2.2 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/blitz-js/superjson/pull/212](https://togithub.com/blitz-js/superjson/pull/212) - Bump tough-cookie from 4.1.2 to 4.1.3 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/blitz-js/superjson/pull/248](https://togithub.com/blitz-js/superjson/pull/248) - fix: TypeError: SuperJSON is not a constructor by [@​juliusmarminge](https://togithub.com/juliusmarminge) in [https://github.com/blitz-js/superjson/pull/250](https://togithub.com/blitz-js/superjson/pull/250) #### New Contributors - [@​juliusmarminge](https://togithub.com/juliusmarminge) made their first contribution in [https://github.com/blitz-js/superjson/pull/250](https://togithub.com/blitz-js/superjson/pull/250) **Full Changelog**: flightcontrolhq/superjson@v1.13.0...v1.13.1 ### [`v1.13.0`](https://togithub.com/blitz-js/superjson/releases/tag/v1.13.0) [Compare Source](https://togithub.com/blitz-js/superjson/compare/v1.12.4...v1.13.0) #### What's Changed - fix: referential equalities should ignore child nodes by [@​KATT](https://togithub.com/KATT) in [https://github.com/blitz-js/superjson/pull/244](https://togithub.com/blitz-js/superjson/pull/244) - feat: add `dedupe` flag by [@​KATT](https://togithub.com/KATT) in [https://github.com/blitz-js/superjson/pull/247](https://togithub.com/blitz-js/superjson/pull/247) **Full Changelog**: flightcontrolhq/superjson@v1.12.4...v1.13.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/Chia1104/chia1104.dev). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi44LjExIiwidXBkYXRlZEluVmVyIjoiMzYuOC4xMSIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [superjson](https://togithub.com/blitz-js/superjson) | [`1.12.4` -> `1.13.1`](https://renovatebot.com/diffs/npm/superjson/1.12.4/1.13.1) | [![age](https://developer.mend.io/api/mc/badges/age/npm/superjson/1.13.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/superjson/1.13.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/superjson/1.12.4/1.13.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/superjson/1.12.4/1.13.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>blitz-js/superjson (superjson)</summary> ### [`v1.13.1`](https://togithub.com/blitz-js/superjson/releases/tag/v1.13.1) [Compare Source](https://togithub.com/blitz-js/superjson/compare/v1.13.0...v1.13.1) #### What's Changed - Bump json5 from 1.0.1 to 1.0.2 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/blitz-js/superjson/pull/218](https://togithub.com/blitz-js/superjson/pull/218) - Bump minimatch from 3.0.4 to 3.1.2 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/blitz-js/superjson/pull/213](https://togithub.com/blitz-js/superjson/pull/213) - Bump decode-uri-component from 0.2.0 to 0.2.2 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/blitz-js/superjson/pull/212](https://togithub.com/blitz-js/superjson/pull/212) - Bump tough-cookie from 4.1.2 to 4.1.3 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/blitz-js/superjson/pull/248](https://togithub.com/blitz-js/superjson/pull/248) - fix: TypeError: SuperJSON is not a constructor by [@​juliusmarminge](https://togithub.com/juliusmarminge) in [https://github.com/blitz-js/superjson/pull/250](https://togithub.com/blitz-js/superjson/pull/250) #### New Contributors - [@​juliusmarminge](https://togithub.com/juliusmarminge) made their first contribution in [https://github.com/blitz-js/superjson/pull/250](https://togithub.com/blitz-js/superjson/pull/250) **Full Changelog**: flightcontrolhq/superjson@v1.13.0...v1.13.1 ### [`v1.13.0`](https://togithub.com/blitz-js/superjson/releases/tag/v1.13.0) [Compare Source](https://togithub.com/blitz-js/superjson/compare/v1.12.4...v1.13.0) #### What's Changed - fix: referential equalities should ignore child nodes by [@​KATT](https://togithub.com/KATT) in [https://github.com/blitz-js/superjson/pull/244](https://togithub.com/blitz-js/superjson/pull/244) - feat: add `dedupe` flag by [@​KATT](https://togithub.com/KATT) in [https://github.com/blitz-js/superjson/pull/247](https://togithub.com/blitz-js/superjson/pull/247) **Full Changelog**: flightcontrolhq/superjson@v1.12.4...v1.13.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/Chia1104/chia-stack). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMS4wIiwidXBkYXRlZEluVmVyIjoiMzYuMTEuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->
add dedupe flag that will make sure that complex objects only appear once in the output