-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Call Stack Exceeded Error in isEqual #4824
Comments
Verified this issue is still present in |
2 tasks
This should (soon) be fixed by #4915. |
benjamn
added a commit
that referenced
this issue
Jun 5, 2019
Follow-up to #4915. I have kept the tests from that PR, just not the lodash-based implementation of isEqual. The lodash.isequal package has a ton of extra features we don't need, and weighs in at a whopping 10KB minified (3.8KB after gzip). Very little of that machinery is really necessary to fix #4824. I extracted this functionality into a separate npm package called @wry/equality, so that we have the flexibility to depend on the same logic in other packages without importing apollo-utilities.
benjamn
added a commit
that referenced
this issue
Jun 6, 2019
Follow-up to #4915. I have kept the tests from that PR, just not the lodash-based implementation of isEqual. The lodash.isequal package has a ton of extra features we don't need, and weighs in at a whopping 10KB minified (3.8KB after gzip). Very little of that machinery is really necessary to fix #4824. I extracted this functionality into a separate npm package called @wry/equality, so that we have the flexibility to depend on the same logic in other packages without importing apollo-utilities.
benjamn
added a commit
that referenced
this issue
Jun 6, 2019
* Reimplement isEqual without pulling in massive lodash.isequal. Follow-up to #4915. I have kept the tests from that PR, just not the lodash-based implementation of isEqual. The lodash.isequal package has a ton of extra features we don't need, and weighs in at a whopping 10KB minified (3.8KB after gzip). Very little of that machinery is really necessary to fix #4824. I extracted this functionality into a separate npm package called @wry/equality, so that we have the flexibility to depend on the same logic in other packages without importing apollo-utilities. * Treat @wry/equality as an external package when bundling apollo-utilities. * Update @wry/equality to latest version (0.1.2). * Mention PR #4924 in CHANGELOG.md.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
👋 Thanks for apollo, it's pretty cool
Intended outcome:
Data fetches without error
Actual outcome:
Call Stack Exceeded Error
I did some digging into this error, and here is what I found:
isDifferentFromLastResult
is called here:https://github.com/apollographql/apollo-client/blob/master/packages/apollo-client/src/core/QueryManager.ts#L697
isEqual
is called here:https://github.com/apollographql/apollo-client/blob/master/packages/apollo-client/src/core/ObservableQuery.ts#L275
The two objects that are passed to isEqual look like this:
a
looks like:b
looks like:When you invoke the getters on
b
it looks likea
. Bothb
anda
are infinitely nested:a.allLinks[0].category.links[0]
is the same object asa.allLinks[0]
b.allLinks[0].category.links[0]
is the same object asb.allLinks[0]
The way
isEqual
is written, it does not support infinitely nested objects. This leads to the maximum call stack error becauseisEqual
keeps calling itself with properties deeper and deeper into the infinitely nested tree.Why is
isEqual
passed an infinitely nested tree? IsisEqual
supposed to support infinitely nested trees?How to reproduce the issue:
I'm not going to create a minimal reproduction quite yet, first I wanted to check and see if you had any insight based on the debugging above ☝️
Versions
We are using
@nuxtjs/apollo@4.0.0-rc4
andapollo-client@2.5.1
, although I don't think the error comes from the nuxt integration. We also are usingapollo-cache-inmemory@1.5.1
Any help or further guidance in debugging would be appreciated 😄
The text was updated successfully, but these errors were encountered: