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

Remove QueryKeyMaker abstraction. #4245

Merged
merged 1 commit into from
Dec 18, 2018

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Dec 17, 2018

The QueryKeyMaker was first introduced (by me) in #3394 in order to cache the same result for different query objects (or different objects within different queries), provided the objects had identical structure.

It's nice to be able to assume that (sub)queries with equivalent structure share the same object identities, but the cache will work without that assumption, and enforcing that discipline is not free, in terms of runtime cost, implementation complexity, and of course bundle size.

In the future, the graphql-tag tag package could provide a version of the gql template tag function that returns immutable structures that share object identity where possible. By doing that work at query parsing time, we could achieve the same goal without needing the QueryKeyMaker.

This commit alone saves 450ish bytes after minification and gzip!

It's nice to be able to assume that (sub)queries with equivalent structure
share the same object identities, but the cache will work without that
assumption, and enforcing that discipline is not free, both in terms of
runtime cost and in terms of bundle size.

In the future, the `graphql-tag` tag package should provide a version of
the `gql` template tag function that returns immutable structures that
share object identity where possible.

This commit alone saves 450ish bytes after minification and gzip!
@benjamn benjamn self-assigned this Dec 17, 2018
@benjamn benjamn added the idea label Dec 17, 2018
Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

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

Makes complete sense @benjamn (and great idea about moving this functionality into graphql-tag). Thanks!

@benjamn benjamn merged commit d7bf829 into wip-reduce-bundle-size Dec 18, 2018
benjamn added a commit that referenced this pull request Jan 2, 2019
It's nice to be able to assume that (sub)queries with equivalent structure
share the same object identities, but the cache will work without that
assumption, and enforcing that discipline is not free, both in terms of
runtime cost and in terms of bundle size.

In the future, the `graphql-tag` tag package should provide a version of
the `gql` template tag function that returns immutable structures that
share object identity where possible.

This commit alone saves 450ish bytes after minification and gzip!
benjamn added a commit that referenced this pull request Jan 17, 2019
It's nice to be able to assume that (sub)queries with equivalent structure
share the same object identities, but the cache will work without that
assumption, and enforcing that discipline is not free, both in terms of
runtime cost and in terms of bundle size.

In the future, the `graphql-tag` tag package should provide a version of
the `gql` template tag function that returns immutable structures that
share object identity where possible.

This commit alone saves 450ish bytes after minification and gzip!
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 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

2 participants