Simplify store.adapterFor and store.serializerFor.
#5300
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Use an inline series of lookups (instead of `ContainerInstanceCache`)
This was previously being done by ContainerInstanceCache (when it would look up an instance, it would always `set(instance, 'store', store)`), but the DI system can already do this for us automatically. This simplifies adapter / serializer testing quite a bit...
There is no reason to have them be non-singleton since we were already caching in the `ContainerInstanceCache`...
|
I created a tracking issue (adopted-ember-addons/ember-data-model-fragments#285) in ember-data-model-fragments... |
|
I chatted with @bmac offline, and there was some concern around the change making adapters and serializers singletons. Specifically, the move away from being singletons was done in order to support having different adapters and serializer instances with multiple store services (where each store got its own adapter and serializer instances). There were no tests to that effect, however it would be fairly easy to revert that small subsection here. I am concerned about lack of proper test coverage if this is actually feature to the system... |
bmac
approved these changes
Dec 22, 2017
rondale-sc
added a commit
to rondale-sc/ember-data-model-fragments
that referenced
this pull request
Jan 26, 2018
Ember Data 3.0 removed the ContainerInstanceCache class which we were (prior to this patch) monkey-patching to override the default serailizer fallback lookup. This PR goes back to a similar approach to what ember-data-model-fragments was using prior to ember-data 2.11 (when ContainerInstanceCache was added) For a bit of context here is the patch that introduced the monkey-patch for Ember Data 2.11: adopted-ember-addons@a856c24 And here is the patch that remove ContainerInstanceCache class from Ember Data making the aforementioned monkey patch throw in 3.0: emberjs/data#5300 This PR ensures that the correct serializer is looked up as per the tests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
adapterForandserializerForContainerInstanceCache