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

Consume ISO Strings as well #2

Open
langpavel opened this issue May 5, 2016 · 6 comments
Open

Consume ISO Strings as well #2

langpavel opened this issue May 5, 2016 · 6 comments

Comments

@langpavel
Copy link

langpavel commented May 5, 2016

Hi, do you plan to support passing ISO serialized date as well? This means this test will be successful.

Reason: I'm using REST API which serves me dates serialized as string. I wish to pass result as is to express-graphql endpoint and don't care of format.

It can be also useful to pass Numbers as UNIX timestamps, but this is problematic because of much API varies between milliseconds and seconds.

@tjmehta
Copy link
Owner

tjmehta commented May 5, 2016

Serialize converts a date to a str - hence why that test fails. Note that parse works fine: https://github.com/tjmehta/graphql-date/blob/master/test/graphql-date.test.js#L48. I happy to accept PRs that support other serialized date values.

@langpavel
Copy link
Author

langpavel commented May 5, 2016

I'm not sure if you understand me.

I wish to forward serialized string formatted as ISO date string. Not Date instance.
It's because I'm only forwarding JSON response from foreign API and I'm receiving serialized form.

I need that serialize accepts already serialized input.

I'm only read your code, not try, but it is clear from test that your module doesn't work this way.

I'm using this implementation now: https://gist.github.com/langpavel/b30f3d507a47713b0c6e89016e4e9eb7

Question now is if you want to support this behavior, because it's not clean and it's use case specific :-)

@tjmehta
Copy link
Owner

tjmehta commented May 5, 2016

I understand now. You want to support various date formats (not serialized). Hmm, my first thought is to export the different formats as different types. The more the strict the type the more likely it will catch errors in your code. What do you think?

@langpavel
Copy link
Author

You are correct. The more the strict the type the more likely it will catch errors in your code... I'm observing with native GraphQL types that they are returning null instead of throwing an exception if coercion fails.
On the other hand, your code is clean. I like it but it does not fit to my needs. There may be other users and they are conform with your library as is.

I think exporting more types is bad idea, because it is exactly one domain type. Some kind of flag or singleton factory should be better..? I don't know ATM.

I suggest let this issue open and I suggest to others, act if you need this behavior. There is 👍 voting :-)

You have 116 downloads per month, not much but not bad as well :-)

Thank you for your quick responses. I'm too early with my use case as well so I have no clear idea on this..

@tjmehta
Copy link
Owner

tjmehta commented May 5, 2016

Cool, will do. Thanks for bringing this up

@langpavel
Copy link
Author

Good to know, I will watch this issue, let me know then. 😄

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

No branches or pull requests

2 participants