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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test for issue #196 #197

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Conversation

dgrcode
Copy link

@dgrcode dgrcode commented Aug 17, 2022

@Skn0tt I've added the test hardcoding the data that is creating the issue. However, the test is passing 馃し

I'm opening this PR so you can have a quick look and check if you see any red flag in the test I've written. I'm just hardcoding the value that's giving me trouble, stringifying, parsing, and checking that the parsed value and the hardcoded value are equal.

Let me know if you see something weird.

Thanks!

@dgrcode
Copy link
Author

dgrcode commented Aug 18, 2022

I've been testing locally with different payloads which fail on the app, but all of them make the test pass.

The two possible scenarios that I think we might be in:

  • I'm doing something wrong with the tests and I should be asserting something different.
  • The problem is not in superjson, but in babel-plugin-superjson-next instead.

wdyt?

@Skn0tt
Copy link
Member

Skn0tt commented Aug 18, 2022

The test looks very fine to me. One think I could imagine is happening is that the javascript object representation you've pasted in fails to capture some of the fine details of the objects created by dehydrating. Maybe you could try using the breaking code from #196 in the test?

@Skn0tt
Copy link
Member

Skn0tt commented Aug 18, 2022

I'll be off on vacation for the coming weeks, so I won't be able to assist with this any further for now. If you find the issue, that means the fastest way to resolution is probably fixing it yourself and then opening a PR. Maybe @beerose and team can help with reviewing + publishing during that time.

@dgrcode
Copy link
Author

dgrcode commented Aug 18, 2022

Yes, that makes sense. I'll use the actual dehydrate function and see if that reproduces the issue.

I'll try to fix it myself, no worries. Enjoy your time off!

@dgrcode
Copy link
Author

dgrcode commented Aug 25, 2022

I've updated the tests to use the actual QueryClient and dehydrate from @tanstack/react-query.

EDIT: A quick note on the dependencies added. I had to add react because react-query was complaining about it. I've opened TanStack/query#4080 because I don't see why the code I'm using would require react, but I also might be missing something about that library. Updated the dependencies to avoid using react on c02c0c7

To my surprise, the tests continue passing. I've checked the object produced by dehydrate and it's pretty much the same as the object I originally described on #196, so I really don't know what else I can do to imitate the actual behaviour.

The hypothesis that the issue might be on the babel plugin babel-plugin-superjson-next instead of this library is getting more points.

What do you think should be the next steps?

@Skn0tt
Copy link
Member

Skn0tt commented Sep 13, 2022

back from PTO!

I'm not sure what's causing this. Maybe you could build a minimum reproduction repository? Then I could take a look and check if I can reproduce and have an idea what's causing it

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