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

More robust type detection in get-key.js #290

Merged
merged 2 commits into from
Jan 31, 2020
Merged

Conversation

hypothete
Copy link
Contributor

Recently on our Ember app we've been having issues using ember-data-storefront to retrieve some - but not all - items from the shoebox. We determined that the frontend and Fastboot were able to generate the appropriate keys for retrieval, but ember-data-storefront was sometimes trying to fetch with malformed keys. We tracked the bug down to the object constructor comparison in get-key.js, where sometimes our params constructor appeared like so instead of as the Object constructor:

[Function: Object] {
  ___dart_isolate_tags_: [Object: null prototype] { _ZxYxX_0_: 1 }
}

This appears to have come from ember-cli-sass using dart-sass, which pollutes the Object constructor. We still haven't determined why this only affects some uses of cacheKey().

Granted, this bug is not ember-data-storefront's fault, but with a small change to get-key.js the library can validate arrays and objects without comparing constructors. Would you be supportive of this change?

Duncan Alexander and others added 2 commits January 29, 2020 12:10
@ryanto
Copy link
Member

ryanto commented Jan 31, 2020

Awesome!

I would have never been able to debug this :D so thanks for digging in and getting it solved.

I'll get this merged and released tomorrow.

@hypothete
Copy link
Contributor Author

Thanks @ryanto ! Looking forward to your release.

@ryanto ryanto merged commit 4116b88 into embermap:master Jan 31, 2020
@ryanto
Copy link
Member

ryanto commented Jan 31, 2020

Ok released as v0.17.2! Thanks again for getting this fixed.

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

Successfully merging this pull request may close these issues.

2 participants