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

fix problem with stringifying date objects #485

Merged
merged 3 commits into from
Dec 12, 2019
Merged

fix problem with stringifying date objects #485

merged 3 commits into from
Dec 12, 2019

Conversation

BjoernRave
Copy link
Contributor

@BjoernRave BjoernRave commented Dec 10, 2019

For some reason when I tried to include the library locally I got an error about Invalid hook calls:

Error: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:

So I was not able to test if it fixes those for sure

This possibly:
fixes #479 and fixes #478

@JoviDeCroock
Copy link
Collaborator

JoviDeCroock commented Dec 11, 2019

This is awesome work, thanks for tackling this!
Can we add a test for this? This way we can ensure us not encountering this issue in the future.

The issue you are encountering can probably be solved by aliasing React and react-dom to your local version in webpack like

alias: { react: __dirname/node_modules/react }

This is because you are symlinking from another folder with it's own node_modules/react

@@ -1,6 +1,7 @@
const seen = new Set();

const stringify = (x: any): string => {
if (x && x.toJson) x = x.toJSON();
Copy link
Member

Choose a reason for hiding this comment

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

Seems like this isn’t quite the right check. We could also move it down to avoid the double null check and do a proper typeof being “function” check?

I’m also wondering, what are we really accomplishing for moment here? Don’t we want to convert it and dates to a UTC string?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If I recall correctly the normal way for serializing a complex object is through the .toJSON function which is present on most libraries like moment, day-js,...

@BjoernRave
Copy link
Contributor Author

Not sure if this test is enough or if we should import sth like moment to test it with their dates as well

@kitten kitten merged commit 47d030f into urql-graphql:master Dec 12, 2019
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.

refetch not triggered when variable changed Error in stringifyVariables during mutation
3 participants