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.

Sign up for GitHub

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

Jump to bottom

Strip apollo directives #374

Closed
wants to merge 8 commits into from
Closed

Strip apollo directives #374

wants to merge 8 commits into from

Conversation

rricard
Copy link
Contributor

@rricard rricard commented Jul 8, 2016 •
edited
Loading

TODO:

  • Update CHANGELOG.md with your change
  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass

Linked to the work of #350

@zol zol added the in progress label Jul 8, 2016
@rricard rricard self-assigned this Jul 8, 2016
@rricard rricard added the ready label Jul 8, 2016
@stubailo stubailo assigned stubailo and unassigned rricard Jul 8, 2016
@rricard
Copy link
Contributor Author

rricard commented Jul 14, 2016

@stubailo How are we on this one ? I need to have it merged in order to continue my work on the infinite pagination ...

@rricard
Copy link
Contributor Author

rricard commented Jul 14, 2016

I'll start by rebasing 😉

@rricard
Copy link
Contributor Author

rricard commented Jul 14, 2016

Rebasing done ✅

@stubailo
Copy link
Contributor

@amandajliu is working on a refactor of query transforming right now, to generalize it so that this code is better.

@rricard
Copy link
Contributor Author

rricard commented Jul 14, 2016

Well, ok, ping me when the refactor is ready so I can work on this again ...

@stubailo
Copy link
Contributor

This should be refactored on top of #412, I think @amandajliu can do it!

@rricard
Copy link
Contributor Author

rricard commented Jul 19, 2016

@stubailo: can do it today

@rricard
Copy link
Contributor Author

rricard commented Jul 19, 2016

Started by rebasing ...

@rricard rricard added in progress and removed ready labels Jul 19, 2016
@rricard
Copy link
Contributor Author

rricard commented Jul 19, 2016

@stubailo: done ✅

@rricard
Copy link
Contributor Author

rricard commented Jul 21, 2016

just rebased again

@rricard
Copy link
Contributor Author

rricard commented Jul 22, 2016

@stubailo any ETA for a merge here ? I need it merged to continue splitting the PRs and I need the overall feature quite fast.

This was referenced Jul 22, 2016
Closed
Closed
Closed
Closed
@rricard
Break down shouldInclude and argument validation
2ba7dfd 
We need primitives to parse directives correctly and validate them
properly.
@rricard
Add apolloFetchMore validation
53b6171 
apolloFetch more accepts an optional name string argument.
@rricard
Implement and test apollo directive stripping methods
08739a2 
We ensure that no apollo directives stays in the documents (queries)
@rricard
Add directive stripping before passing to the net interface
90161cc 
We build on the tested stripApolloDirectivesFromDocument to integrate
at batching-level directive stripping.
@rricard
Add CHANGELOG line for directive stripping
de2a692
@rricard
Strip directives using a QueryTransform
c86d092 
A QueryTransform is functionally unpure though, so this new stripping has side effects!
@rricard rricard mentioned this pull request Jul 26, 2016
Merged
4 tasks
@rricard
Don't prevent all transformers from changing root
ca18b2f 
This prevented us from making the directive stripper work correctly.
@rricard
Remove recursivity in the stripper
2dafa54 
It is not needed given the fact the transformer will do it for us...
@rricard
Copy link
Contributor Author

rricard commented Jul 26, 2016

Rebased and made some important fixes to bugs introduced by #463

@rricard rricard changed the title Strip apollo directives (especially the upcoming @apolloFetchMore) Strip apollo directives Aug 2, 2016
@stubailo stubailo closed this Aug 3, 2016
@stubailo stubailo removed the ready label Aug 3, 2016
@stubailo
Copy link
Contributor

stubailo commented Aug 3, 2016

I'm closing this because I'm not sure we need it anymore? Could reopen if we still do.

@stubailo stubailo deleted the strip-apollo-directives branch September 20, 2016 03:42
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@stubailo stubailo

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rricard @stubailo @zol