chore: refactor store to class #6249
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
runspired
force-pushed
the
refactor/typed-store
branch
from
88f3f04
to
6b6352b
Compare
mike-north
reviewed
Jul 16, 2019
There was a problem hiding this comment.
TS review complete.
- I have one major action item to research class field defaults on a prototype
- @runspired to come up with a small repro of the babel+TS issue he's running into
igorT
reviewed
Jul 17, 2019
igorT
approved these changes
Jul 17, 2019
|
import { Promise } from 'rsvp';and use it as a type |
runspired
force-pushed
the
refactor/typed-store
branch
from
6b6352b
to
614016b
Compare
runspired
added a commit
that referenced
this pull request
Jul 31, 2019
This reverts commit e356cce.
8 tasks
HeroicEric
added a commit
to HeroicEric/data
that referenced
this pull request
Aug 13, 2019
…/store emberjs#6249 refactored `Store` to a class. As part of this refactoring, `Store#defaultAdapter` was implemented using the `@computed` decorator. Using the `@computed` decorator requires Ember 3.10+. This adds [`ember-decorators-polyfill`](https://github.com/pzuraq/ember-decorators-polyfill) as a dependency to `@ember-data/store` to add support for Ember < 3.10.
HeroicEric
added a commit
to HeroicEric/data
that referenced
this pull request
Aug 13, 2019
…/store Extracted from emberjs#6098 emberjs#6249 refactored `Store` to a class. As part of this refactoring, `Store#defaultAdapter` was implemented using the `@computed` decorator. Using the `@computed` decorator requires Ember 3.10+. This adds [`ember-decorators-polyfill`](https://github.com/pzuraq/ember-decorators-polyfill) as a dependency to `@ember-data/store` to add support for Ember < 3.10.
HeroicEric
added a commit
to HeroicEric/data
that referenced
this pull request
Aug 14, 2019
…/store Extracted from emberjs#6098 emberjs#6249 refactored `Store` to a class. As part of this refactoring, `Store#defaultAdapter` was implemented using the `@computed` decorator. Using the `@computed` decorator requires Ember 3.10+. This adds [`ember-decorators-polyfill`](https://github.com/pzuraq/ember-decorators-polyfill) as a dependency to `@ember-data/store` to add support for Ember < 3.10.
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.
Service.extendforces us to move all properties onto the prototype in order for them to be type checked, the syntax does not allow for method overloads, and TS struggles to flow expected types through the store methods.This PR refactors the store into a native class, giving us much improved type checking and allowing us to use method overloads.
It surfaced one issue where we expect folks to be able to overwrite
adapteras a property on the store when extending; however, when using babel+typescript and a class this is transpiled with an automatic initializer tovoid 0that overwrites anything set viacreateorextend. We worked around this by not declaring that particular property and using a (nasty) type cast in the two locations we access it. @rwjblue does not believe this issue of interop ofTS+babel+legacy .extendsyntax is something apps will encounter often.This PR builds on #6246 and #6247