Skip to content

Sort fragments during operation normalization (De-composed from #1027).#1112

Merged
abernix merged 10 commits intomasterfrom
abernix/trevors-apollo-graphql-changes-op-normalization
Mar 13, 2019
Merged

Sort fragments during operation normalization (De-composed from #1027).#1112
abernix merged 10 commits intomasterfrom
abernix/trevors-apollo-graphql-changes-op-normalization

Conversation

@abernix
Copy link
Copy Markdown
Member

@abernix abernix commented Mar 13, 2019

This PR brings over a subset of the commits from #1027, in particular the ones which specifically touched apollo-graphql. That PR should be referenced for communication and dialog around the review. This PR represents the direct result of running the following from that branch (which at the time I ran this was at 7714ee1):

$ git checkout shtrevor/operation-normalization
$ git format-patch origin/master..HEAD -- packages/apollo-graphql/
$ git checkout -b abernix/trevors-apollo-graphql-changes-op-normalization # this branch!
$ git am *.patch # With one conflict resolution.

Specifically, this changes the sortAST algorithm to sort fragments at the top-level of the DocumentNode, providing a more deterministic normalization of the operation for use by apollo-engine-reporting (which consumes this package's defaultOperationRegistrySignature function). This will more correctly combine operations for Engine reporting. This also adds a defaultOperationRegistrySignature function for use by the apollo-server-plugin-operation-registry plugin to eventually consume.

* Add apollo-graphql as a dependency (and project reference)
* Remove apollo-engine-reporting as a dependency
* Update sortAST to normalize order of fragments w.r.t operations
* Tests are failing expectedly at this point
This update is exactly as expected. All fragments will now move to the beginning of a normalization result.
This is a valid use case for snapshots. Even better would be inline snapshots - something we can get to later.
These two operations will likely be used in tandem, and we want this to be consistent across consumers.
…iases and be less strict about removing literals.
…ew function, defaultOperationRegistrySignature.

This new function is the effective interim fix, and the current existing function is now left alone.
@abernix abernix changed the title Operation normalization: De-composition of changes from #1027. Sort fragments during operation normalization (De-composed from #1027). Mar 13, 2019
Copy link
Copy Markdown
Contributor

@trevor-scheer trevor-scheer left a comment

Choose a reason for hiding this comment

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

This LGTM!

@abernix abernix merged commit 5913346 into master Mar 13, 2019
@abernix abernix deleted the abernix/trevors-apollo-graphql-changes-op-normalization branch March 13, 2019 14:46
trevor-scheer pushed a commit to apollographql/apollo-server that referenced this pull request May 6, 2020
This incorporates the changes from:

apollographql/apollo-tooling#1112

...which provides a new `defaultOperationRegistrySignature` function that
will be utilized by both the CLI and this plugin to provide the same
normalization.
trevor-scheer pushed a commit to apollographql/apollo-server that referenced this pull request May 6, 2020
This incorporates the changes from:

apollographql/apollo-tooling#1112

...which provides a new `defaultOperationRegistrySignature` function that
will be utilized by both the CLI and this plugin to provide the same
normalization.
trevor-scheer pushed a commit to apollographql/apollo-server that referenced this pull request May 12, 2020
This incorporates the changes from:

apollographql/apollo-tooling#1112

...which provides a new `defaultOperationRegistrySignature` function that
will be utilized by both the CLI and this plugin to provide the same
normalization.
trevor-scheer pushed a commit to apollographql/apollo-server that referenced this pull request May 12, 2020
This incorporates the changes from:

apollographql/apollo-tooling#1112

...which provides a new `defaultOperationRegistrySignature` function that
will be utilized by both the CLI and this plugin to provide the same
normalization.
trevor-scheer pushed a commit to apollographql/apollo-server that referenced this pull request May 14, 2020
This incorporates the changes from:

apollographql/apollo-tooling#1112

...which provides a new `defaultOperationRegistrySignature` function that
will be utilized by both the CLI and this plugin to provide the same
normalization.
trevor-scheer pushed a commit to apollographql/apollo-server that referenced this pull request May 14, 2020
This incorporates the changes from:

apollographql/apollo-tooling#1112

...which provides a new `defaultOperationRegistrySignature` function that
will be utilized by both the CLI and this plugin to provide the same
normalization.
trevor-scheer pushed a commit to apollographql/apollo-server that referenced this pull request May 14, 2020
This incorporates the changes from:

apollographql/apollo-tooling#1112

...which provides a new `defaultOperationRegistrySignature` function that
will be utilized by both the CLI and this plugin to provide the same
normalization.
trevor-scheer pushed a commit to apollographql/apollo-server that referenced this pull request May 14, 2020
This incorporates the changes from:

apollographql/apollo-tooling#1112

...which provides a new `defaultOperationRegistrySignature` function that
will be utilized by both the CLI and this plugin to provide the same
normalization.
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.

2 participants