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

Treat dates properly in isEqual #2131

Merged
merged 5 commits into from Sep 13, 2017
Merged

Conversation

nl0
Copy link
Contributor

@nl0 nl0 commented Sep 8, 2017

Hey there.
I've stumbled upon a bug where my query wasn't updated and refetched when I was changing the variables object (calculated from props) that contained dates. I've tracked it down to the isEqual util function, which treated different dates as equal.
Should I add some tests to cover this case?

@apollo-cla
Copy link

@nl0: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@mention-bot
Copy link

@nl0, thanks for your PR! By analyzing the history of the files in this pull request, we identified @calebmer and @jbaxleyiii to be potential reviewers.

@apollo-cla
Copy link

apollo-cla commented Sep 8, 2017

Warnings
⚠️

There are library changes, but not tests. That's OK as long as you're refactoring existing code

Messages
📖

Please add your name and email to the AUTHORS file (optional)

📖

If this was a change that affects the external API, please update the docs and post a link to the PR in the discussion

Generated by 🚫 dangerJS

@nl0
Copy link
Contributor Author

nl0 commented Sep 11, 2017

So, the issues found by CI are fixed. There were no tests for isEqual and I haven't added any.

@jbaxleyiii jbaxleyiii self-assigned this Sep 11, 2017
@nl0
Copy link
Contributor Author

nl0 commented Sep 13, 2017

@jbaxleyiii hi! Any update on this? It blocks the feature I'm currently working on. Anything I can help you with?

@jbaxleyiii
Copy link
Contributor

@nl0 merging now! This will be in the next latest and in the beta! I'll work to get the next latest released this week! sorry for the delay!

@jbaxleyiii jbaxleyiii merged commit fec6457 into apollographql:master Sep 13, 2017
@nl0 nl0 deleted the patch-1 branch September 14, 2017 16:39
@nl0
Copy link
Contributor Author

nl0 commented Sep 14, 2017

@jbaxleyiii thanks a lot! 🍻

@nl0
Copy link
Contributor Author

nl0 commented Sep 27, 2017

@jbaxleyiii hey. sorry for bothering you again, but do you have any eta on the next 1.x release?

@nl0
Copy link
Contributor Author

nl0 commented Sep 27, 2017

@jbaxleyiii nvm, just noticed the new version in the registry. thanks again for your work on the project!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants