Skip to content
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

bug repro with dedupe=true #253

Merged
merged 5 commits into from
Sep 27, 2023
Merged

bug repro with dedupe=true #253

merged 5 commits into from
Sep 27, 2023

Conversation

KATT
Copy link
Contributor

@KATT KATT commented Jul 27, 2023

We found a bug with dedupe option.

I'm sorry for this repro, it's awful and huge and bloated. Haven't narrowed it down more, but AFAICT, the results of these should be equal but they aren't.

The problem shows in the diff between non-deduped and deduped; some values stay as null after deserializing the deduped one.

@KATT KATT requested a review from Skn0tt as a code owner July 27, 2023 19:34
@Skn0tt
Copy link
Member

Skn0tt commented Sep 27, 2023

Finally got around to take a look at this. It was a nifty small bug, definitely a fun sherlocking adventure! :D

I put some prose as to what's happening into the test in 593841d, if you're curious.

Copy link
Member

@Skn0tt Skn0tt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks super good. I couldn't have written this better myself, except for the parts I wrote myself.

@Skn0tt Skn0tt merged commit a8949df into blitz-js:main Sep 27, 2023
3 checks passed
@Skn0tt
Copy link
Member

Skn0tt commented Sep 27, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants