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

apollo-client is producing a massive state for just a few queries #660

Closed
gajus opened this issue Sep 17, 2016 · 12 comments
Closed

apollo-client is producing a massive state for just a few queries #660

gajus opened this issue Sep 17, 2016 · 12 comments

Comments

@gajus
Copy link

gajus commented Sep 17, 2016

I making just two simple queries and that produces state object that takes 88867 characters to represent.

Here is a beautified version of the latter state, https://gist.github.com/gajus/6b73c75d88ec8e72c5f6d28976ddd612.

@gajus
Copy link
Author

gajus commented Sep 17, 2016

It looks like most of the size comes from the query property, which is essentially the same content thats contained in the minimizedQueryString property. State should not contain normalized data.

@stubailo
Copy link
Contributor

@gajus the problem is that you are serializing an object that has recursive/self-referential data.

The loc section of the GraphQL query AST is the same reference every time, but when your serialize it it becomes large.

Do you want to submit a PR to strip out the loc from the query ASTs?

The parsing is handled here, in the graphql-tag package: https://github.com/apollostack/graphql-tag/blob/14205fba66f0eef31e2e141fcbc501081e2bd138/index.js#L18

@gajus
Copy link
Author

gajus commented Sep 17, 2016

Do you want to submit a PR to strip out the loc from the query ASTs?

Are you suggesting that loc property is redundant?

@stubailo
Copy link
Contributor

Yeah, it's actually not used anywhere (even when printing). It's just something that the GraphQL parser returns, and we didn't bother removing it.

@stubailo
Copy link
Contributor

@gajus
Copy link
Author

gajus commented Sep 17, 2016

@gajus
Copy link
Author

gajus commented Sep 19, 2016

@stubailo I think this was closed too early. The loc property has been removed, however there is a lot more duplication.

Here is how the state looks at the moment, https://gist.github.com/gajus/33797f17781259697ce03a58ad788ab0.

Thats a lot better (12587 characters). However, it could be easily cut down to less.

  1. Remove tokenized query.

    Example, all of this information can be extracted from query string. There is no need to store parsed query in the state.

  2. Remove minimizedQueryString.

    As far as I can tell, this is not even used anywhere and it is exactly the same as queryString.

@stubailo
Copy link
Contributor

For (1), we definitely need the parsed query. All of Apollo's internal logic depends on being able to execute the queries against the store, and you need to parse it to do that.

For (2), we currently do use it, but we will remove it as soon as we resolve #615, which will hopefully be this week.

If you have any additional ideas, let's open then as new issues? I'm sure there will always be some optimizations to be made in terms of store size, and so this issue would be open basically forever.

@gajus
Copy link
Author

gajus commented Sep 19, 2016

For (1), we definitely need the parsed query. All of Apollo's internal logic depends on being able to execute the queries against the store, and you need to parse it to do that.

But you can parse it on the client-side.

@stubailo
Copy link
Contributor

Oh, are you referring specifically to serializing the state on the server for store hydration?

@gajus
Copy link
Author

gajus commented Sep 19, 2016

Oh, are you referring specifically to serializing the state on the server for store hydration?

Yes.

@stubailo
Copy link
Contributor

OK, perhaps let's open another issue - identify state that can be eliminated from the server-side hydration. I'm sure there is a lot more stuff that is used on the client side but doesn't need to be in the intial payload.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants