-
-
Notifications
You must be signed in to change notification settings - Fork 405
rfc: ember-data | Modernize PromiseManyArray #745
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
Merged
runspired
merged 4 commits into
emberjs:master
from
runspired:ember-data/deprecate-methods-on-promise-many-array
Aug 5, 2022
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
e2af25e
rfc: ember-data | Modernize PromiseManyArray
runspired 495060f
Update text/0745-ember-data-deprecate-methods-on-promise-many-array.md
runspired 5e74637
Update 0745-ember-data-deprecate-methods-on-promise-many-array.md
runspired 6441835
Update text/0745-ember-data-deprecate-methods-on-promise-many-array.md
runspired File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
208 changes: 208 additions & 0 deletions
208
text/0745-ember-data-deprecate-methods-on-promise-many-array.md
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,208 @@ | ||
| --- | ||
| Stage: Accepted | ||
| Start Date: 2021-04-23 | ||
| Release Date: Unreleased | ||
| Release Versions: | ||
| ember-source: vX.Y.Z | ||
| ember-data: vX.Y.Z | ||
| Relevant Team(s): ember-data | ||
| RFC PR: https://github.com/emberjs/rfcs/pull/745 | ||
| --- | ||
|
|
||
| # EmberData | Modernize PromiseManyArray | ||
|
|
||
| ## Summary | ||
|
|
||
| Deprecates reading, mutating or operating on an async `hasMany` relationship before | ||
| resolving it's value in the application's Javascript code. Rendering an async `hasMany` | ||
| in a template will continue to work as expected. | ||
|
|
||
| ## Motivation | ||
|
|
||
| `PromiseManyArray` is a promisified `ArrayProxy` that serves as a fetch wrapper when | ||
| a user either implicitly or explicitly defines a `hasMany` relationship as async. It | ||
| is the synchronous return value when a user access the async relationship on a record instance. | ||
|
|
||
| For instance, given the following `Post` model. | ||
|
|
||
| ```js | ||
| class Post extends Model { | ||
| @hasMany('comment' { inverse: null, async: true }) | ||
| comments | ||
| } | ||
| ``` | ||
|
|
||
| A user today might access and map over the comments for a post in the following manner. | ||
|
|
||
| ```js | ||
| const commentProxy = post.comments; | ||
| const commentIds = commentProxy.map(comment => comment.id); | ||
| ``` | ||
|
|
||
| This often works because any `resolved` data for the relationship is immediately accessible | ||
| via the `ArrayProxy`. However, if the network request initiated by accessing the comments | ||
| relationship results in changes to what records are available, those changes would not be | ||
| reflected in the `commentIds`. More often, users will `await` the relationship to ensure the | ||
| data is fully ready for use. | ||
|
|
||
| ```js | ||
| const comments = await post.comments; | ||
| const commentIds = comments.map(comment => comment.id); | ||
| ``` | ||
|
|
||
| As the language and framework have evolved, better features have become available that | ||
| negate the value of many of the methods that exist on `PromiseManyArray`. This RFC seeks | ||
| to better align the way folks interact with an async relationship with the `Octane` paradigm, | ||
| `tracked` properties, and `async/await`. Doing so allows us to take a major first step towards | ||
| simplifying the use of array-like proxies throughout EmberData. | ||
|
|
||
| ## Detailed design | ||
|
|
||
| This RFC will reduce the API surface area of PromiseManyArray to just those methods needed to | ||
| serve it's primary functions. | ||
|
|
||
| - as a thenable/awaitable value (with promise state flags) | ||
| - as a custom enumerable that for-convenience can be directly iterated by templates. | ||
|
|
||
| All other methods and properties on PromiseManyArray would be deprecated. Additionally, | ||
| the methods `addArrayObserver` and `removeArrayObserver` on `ManyArray` will be deprecated. | ||
|
|
||
| Code examples, guides, and documentation should encourage users to resolve the relationship | ||
| before operating on it, including avoiding the use of the remaining iterable APIs (which will | ||
| be marked private). | ||
|
|
||
| This deprecation would target `5.0` and would become `enabled` no-sooner than `4.1` (although | ||
| it may be made `available` before then). | ||
|
|
||
| We will keep the following properties and methods previously supplied by the `PromiseProxyMixin` | ||
|
|
||
| - the public promise state flag `isSettled` | ||
| - the public promise state flag `isPending` | ||
| - the public promise state flag `isRejected` | ||
| - the public promise state flag `isFulfilled` | ||
| - the private property `promise` | ||
| - the method `then` | ||
| - the method `catch` | ||
| - the method `finally` | ||
|
|
||
| But we will eliminate the private property `reason`. | ||
|
|
||
| We will keep the following property we had supplied on our own | ||
|
|
||
| - the public property `links` | ||
| - the method `reload` | ||
|
|
||
| While deprecating the following methods which delegated to the ManyArray | ||
|
|
||
| - `createRecord` | ||
|
|
||
| As `PromiseManyArray` historically extended `ArrayProxy`, it inherited a large API surface from | ||
| `MutableArray` `EmberArray` and `EmberObject` the totality of which will be deprecated *except* for: | ||
|
|
||
| - the private property `content` | ||
| - the property `length` | ||
| - the public property `isDestroyed` | ||
| - the iterator method `objectAt` (but which will now be marked private) | ||
|
|
||
|
|
||
| ### Finalized API of PromiseManyArray | ||
|
|
||
| Remaining API surface | ||
|
|
||
| - `content` (private) | ||
| - `length` | ||
| - `isDestroyed` | ||
| - `objectAt` (private) | ||
| - `links` | ||
| - `reload` | ||
| - `promise` (private) | ||
| - `isSettled` | ||
| - `isPending` | ||
| - `isFulfilled` | ||
| - `isRejected` | ||
| - `then` | ||
| - `catch` | ||
| - `finally` | ||
|
|
||
| Deprecated API surface | ||
|
|
||
| - `reason` | ||
| - `createRecord` | ||
| - `firstObject` | ||
| - `lastObject` | ||
| - `addObserver` | ||
| - `cacheFor` | ||
| - `decrementProperty` | ||
| - `get` | ||
| - `getProperties` | ||
| - `incrementProperty` | ||
| - `notifyPropertyChange` | ||
| - `removeObserver` | ||
| - `set` | ||
| - `setProperties` | ||
| - `toggleProperty` | ||
| - `addArrayObserver` | ||
| - `addObject` | ||
| - `addObjects` | ||
| - `any` | ||
| - `arrayContentDidChange` | ||
| - `arrayContentWillChange` | ||
| - `clear` | ||
| - `compact` | ||
| - `every` | ||
| - `filter` | ||
| - `filterBy` | ||
| - `find` | ||
| - `findBy` | ||
| - `forEach` | ||
| - `getEach` | ||
| - `includes` | ||
| - `indexOf` | ||
| - `insertAt` | ||
| - `invoke` | ||
| - `isAny` | ||
| - `isEvery` | ||
| - `lastIndexOf` | ||
| - `map` | ||
| - `mapBy` | ||
| - `objectsAt` | ||
| - `popObject` | ||
| - `pushObject` | ||
| - `pushObjects` | ||
| - `reduce` | ||
| - `reject` | ||
| - `rejectBy` | ||
| - `removeArrayObserver` | ||
| - `removeAt` | ||
| - `removeObject` | ||
| - `removeObjects` | ||
| - `replace` | ||
| - `reverseObjects` | ||
| - `setEach` | ||
| - `setObjects` | ||
| - `shiftObject` | ||
| - `slice` | ||
| - `sortBy` | ||
| - `toArray` | ||
| - `uniq` | ||
| - `uniqBy` | ||
| - `unshiftObject` | ||
| - `unshiftObjects` | ||
| - `without` | ||
|
|
||
| ## How we teach this | ||
|
|
||
| As this deprecation targets 4.x, users would need to have upgraded to Octane paradigms before | ||
| they could encounter the deprecations listed here. This means for *most* apps this should be as | ||
| trivial as adding an `await` where necessary. | ||
|
|
||
| ## Drawbacks | ||
|
|
||
| It has become fairly common to interact with async hasMany relationships as if they are | ||
| synchronous, and it is not easy to programmatically codemod a conversion. This may result in | ||
| a sizeable amount of "find the missing await" for some applications. | ||
|
|
||
| ## Alternatives | ||
|
|
||
| Deprecate array-like APIs on ManyArray simultaneously. Why not? While this is something we seek to do as a follow up, that migration is a more difficult one than this which *only* pushes users to resolve async before interacting with the value. As the ManyArray has all these same methods in a non-deprecated state, existing code will continue to run as expected and without deprecation so long as that resolution is done. This is a first step which will make that deprecation easier when | ||
| the time comes. | ||
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.
Uh oh!
There was an error while loading. Please reload this page.