From e2af25e791b8d2aee21e2516ba460f0b5a18111b Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Wed, 5 May 2021 17:44:55 -0700 Subject: [PATCH 1/4] rfc: ember-data | Modernize PromiseManyArray --- ...deprecate-methods-on-promise-many-array.md | 207 ++++++++++++++++++ 1 file changed, 207 insertions(+) create mode 100644 text/0745-ember-data-deprecate-methods-on-promise-many-array.md diff --git a/text/0745-ember-data-deprecate-methods-on-promise-many-array.md b/text/0745-ember-data-deprecate-methods-on-promise-many-array.md new file mode 100644 index 0000000000..479088e0d7 --- /dev/null +++ b/text/0745-ember-data-deprecate-methods-on-promise-many-array.md @@ -0,0 +1,207 @@ +--- +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 acces 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` +- `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. \ No newline at end of file From 495060f586e19f6bff350c69425faf387eed8efd Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Wed, 13 Apr 2022 16:35:07 -0700 Subject: [PATCH 2/4] Update text/0745-ember-data-deprecate-methods-on-promise-many-array.md Co-authored-by: Sam Clopton --- text/0745-ember-data-deprecate-methods-on-promise-many-array.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0745-ember-data-deprecate-methods-on-promise-many-array.md b/text/0745-ember-data-deprecate-methods-on-promise-many-array.md index 479088e0d7..f83aed6f80 100644 --- a/text/0745-ember-data-deprecate-methods-on-promise-many-array.md +++ b/text/0745-ember-data-deprecate-methods-on-promise-many-array.md @@ -32,7 +32,7 @@ class Post extends Model { } ``` -A user today might acces and map over the comments for a post in the following manner. +A user today might access and map over the comments for a post in the following manner. ```js const commentProxy = post.comments; From 5e74637d64d3bd672b184f4096926a358123a254 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Wed, 13 Apr 2022 16:36:40 -0700 Subject: [PATCH 3/4] Update 0745-ember-data-deprecate-methods-on-promise-many-array.md --- .../0745-ember-data-deprecate-methods-on-promise-many-array.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/text/0745-ember-data-deprecate-methods-on-promise-many-array.md b/text/0745-ember-data-deprecate-methods-on-promise-many-array.md index f83aed6f80..0f4bdfb666 100644 --- a/text/0745-ember-data-deprecate-methods-on-promise-many-array.md +++ b/text/0745-ember-data-deprecate-methods-on-promise-many-array.md @@ -183,6 +183,7 @@ Deprecated API surface - `shiftObject` - `slice` - `sortBy` +- `toArray` - `uniq` - `uniqBy` - `unshiftObject` @@ -204,4 +205,4 @@ 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. \ No newline at end of file +the time comes. From 6441835028fb131d566609c3195398907d9a0101 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Tue, 26 Jul 2022 12:07:38 -0600 Subject: [PATCH 4/4] Update text/0745-ember-data-deprecate-methods-on-promise-many-array.md Co-authored-by: MrChocolatine <47531779+MrChocolatine@users.noreply.github.com> --- text/0745-ember-data-deprecate-methods-on-promise-many-array.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0745-ember-data-deprecate-methods-on-promise-many-array.md b/text/0745-ember-data-deprecate-methods-on-promise-many-array.md index 0f4bdfb666..86ac540718 100644 --- a/text/0745-ember-data-deprecate-methods-on-promise-many-array.md +++ b/text/0745-ember-data-deprecate-methods-on-promise-many-array.md @@ -27,7 +27,7 @@ For instance, given the following `Post` model. ```js class Post extends Model { - @hasMany('comment` { inverse: null, async: true }) + @hasMany('comment' { inverse: null, async: true }) comments } ```