From 2548cf440cf7393ba105e2fddfb292b44e513fcf Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Thu, 10 Nov 2022 18:05:50 -0800 Subject: [PATCH 01/12] rfc: EmberData Request Service draft --- text/0000-ember-data-request-service.md | 200 ++++++++++++++++++++++++ 1 file changed, 200 insertions(+) create mode 100644 text/0000-ember-data-request-service.md diff --git a/text/0000-ember-data-request-service.md b/text/0000-ember-data-request-service.md new file mode 100644 index 0000000000..ef47f85e36 --- /dev/null +++ b/text/0000-ember-data-request-service.md @@ -0,0 +1,200 @@ +--- +stage: accepted +start-date: +release-date: +release-versions: +teams: + - data +prs: + accepted: # update this to the PR that you propose your RFC in +project-link: +--- + + + +# EmberData | Request Service + +## Summary + +Proposes a simple abstraction over `fetch` to enable easy management of request/response +flows. Provides associated utilities to assist in migrating to this new abstraction. Adds + new APIs to the Store to make use of this abstraction. + +## Motivation + +- `Serializer` lacks the context necessary to serialize/normalize data on a per-request basis +- `Adapter` is inflexible and difficult to grow as an interface for managing data +fulfillment from a source. +- Applications have need of a low-level primitive solution for managed fetch to ensuring proper headers, authentication, error handling, SSR support, test-waiter support, and request de-duping/caching. + +## Detailed design + +#### RequestManager + +A `RequestManager` provides a request/response flow in which +configured middlewares are successively given the opportunity +to handle, modify, or pass-along a request. + +```ts +import RequestManager, { Fetch } from '@ember-data/request'; +import Auth from 'ember-simple-auth/ember-data-middleware'; +import Config from './config'; + +const { apiUrl } = Config; + +// ... create manager +const manager = new RequestManager(); +manager.use([Auth, Fetch]); + +// ... execute a request +const response = await manager.request({ + url: `${apiUrl}/users` +}); +``` + +**Futures** + +The return value of `manager.request` is a `Future`, which allows +access to limited information about the request while it is still +pending and fulfills with the final state when the request completes. + +A `Future` is cancellable via `abort`. + +Middlewares may optionally expose a ReadableStream to the `Future` +for streaming data; however, when doing so the future should not +resolve until the response stream is fully read. + +```ts +interface Future extends Promise> { + abort(): void; + + async stream(): ReadableStream | null; +} +``` + +The `StructuredDocument` interface is the same as is proposed in RFC 854 and copied below: + +```ts +interface StructuredDocument { + request: { + url: string; + cache?: { key?: string, reload?: boolean, backgroundReload?: boolean }; + method: 'GET' | 'POST' | 'PUT' | 'DELETE' | 'PATCH'; + data?: Record; + options?: Record; + headers: Record; + } + response: { + status: HTTPStatusCode; + headers: Record; + } + data: T; + error?: Error; +} +``` + +**Registering Middlewares** + +**The Middleware API** + +**Using as a Service** + +Most applications will desire to have a single `RequestManager` instance, which +can be achieved using module-state patterns for singletons, or for Ember +applications by exporting the manager as an Ember service. + +*services/request.ts* +```ts +import RequestManager, { Fetch } from '@ember-data/request'; +import Auth from 'ember-simple-auth/ember-data-middleware'; + +export default class extends RequestManager { + constructor(createArgs) { + super(createArgs); + this.use([Auth, Fetch]); + } +} +``` + +**Using with the Store Service** + +Assuming a manager has been registered as the `request` service. + +*services/store.ts* +```ts +import Store from '@ember-data/store'; +import { service } from '@ember/service'; + +export default class extends Store { + @service('request') requestManager; +} +``` + +Alternatively to have a request service unique to the store: + +```ts +import Store from '@ember-data/store'; +import RequestManager, { Fetch } from '@ember-data/request'; + +export default class extends Store { + requestManager = new RequestManager(); + + constructor(args) { + super(args); + this.requestManager.use([Fetch]); + } +} +``` + + + +We would introduce new url-building and request building utility functions in a new package +`@ember-data/request-utils`. + +We would introduce a new middleware in it's own package `@ember-data/legacy-network-middleware` +to encapsulate legacy behaviors of adapters, and serializers. + +We would introduce a new lifetimes behavior to the `Store` to replace the reload APIs on `Adapter`. + +## How we teach this + +> What names and terminology work best for these concepts and why? How is this +idea best presented? As a continuation of existing Ember patterns, or as a +wholly new one? + +> Would the acceptance of this proposal mean the Ember guides must be +re-organized or altered? Does it change how Ember is taught to new users +at any level? + +> How should this feature be introduced and taught to existing Ember +users? + +## Drawbacks + +> Why should we *not* do this? Please consider the impact on teaching Ember, +on the integration of this feature with other existing and planned features, +on the impact of the API churn on existing apps, etc. + +> There are tradeoffs to choosing any path, please attempt to identify them here. + +## Alternatives + +> What other designs have been considered? What is the impact of not doing this? + +> This section could also include prior art, that is, how other frameworks in the same domain have solved this problem. + +## Unresolved questions + +> Optional, but suggested for first drafts. What parts of the design are still +TBD? From 725df0e3cbfebb6cc2ce1afd7e1bb264810fdeae Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Fri, 11 Nov 2022 15:53:54 -0800 Subject: [PATCH 02/12] updates --- text/0000-ember-data-request-service.md | 84 +++++++++++++++++++++++-- 1 file changed, 80 insertions(+), 4 deletions(-) diff --git a/text/0000-ember-data-request-service.md b/text/0000-ember-data-request-service.md index ef47f85e36..a23b4f0fd1 100644 --- a/text/0000-ember-data-request-service.md +++ b/text/0000-ember-data-request-service.md @@ -40,10 +40,10 @@ fulfillment from a source. ## Detailed design -#### RequestManager +### RequestManager A `RequestManager` provides a request/response flow in which -configured middlewares are successively given the opportunity +configured handlers are successively given the opportunity to handle, modify, or pass-along a request. ```ts @@ -71,7 +71,7 @@ pending and fulfills with the final state when the request completes. A `Future` is cancellable via `abort`. -Middlewares may optionally expose a ReadableStream to the `Future` +Handlers may optionally expose a ReadableStream to the `Future` for streaming data; however, when doing so the future should not resolve until the response stream is fully read. @@ -104,7 +104,77 @@ interface StructuredDocument { } ``` -**Registering Middlewares** +**Request Handlers** + +Requests are fulfilled by handlers. A handler receives the request context +as well as a `next` function with which to pass along a request to the next +handler if they so choose. + +```ts + +type NextFn

= (req: RequestInfo) => Future

; + +interface Handler { + async request(context: RequestContext, next: NextFn

): T; +} +``` + +`RequestContext` contains information about the request as well as a few methods + for building up the `StructuredDocument` and `Future` that will be part of the + response. + + Note: `setStream` MUST be called synchronously before awaiting any async within + the method in order for `getStream` to work appropriately. + + +```ts +interface RequestContext { + readonly request: RequestInfo; + + setStream(stream: ReadableStream): void; + setResponse(response: Response): void; +} + +interface RequestInfo { + url: string; + cache?: { key?: string, reload?: boolean, backgroundReload?: boolean }; + method: 'GET' | 'POST' | 'PUT' | 'DELETE' | 'PATCH'; + data?: Record; + options?: Record; + headers: Record; + signal: AbortSignal; +} +``` + +A basic `fetch` handler with support for streaming content updates while +the download is still underway might look like the following, where we use +[`response.clone()`](https://developer.mozilla.org/en-US/docs/Web/API/Response/clone) to `tee` the `ReadableStream` into two streams. + +A more efficient handler might read from the response stream, building up the +response data before passing along the chunk downstream. + +```ts +const FetchHandler = { + async request(context) { + const response = await fetch(context.request); + context.setResponse(reponse); + context.setStream(response.clone().body); + + return response.json(); + } +} +``` + +Request handlers are registered by configuring the manager via `use` + +```ts +manager.use([Handler1, Handler2]) +``` + +Handlers will be invoked in the order they are registered ("fifo", first-in +first-out), and may only be registered up until the first request is made. It +is recommended but not required to register all handlers at one time in order +to ensure explicitly visible handler ordering. **The Middleware API** @@ -157,7 +227,13 @@ export default class extends Store { } ``` +### Using `store.request()` + +### Cache Lifetimes + +### Migrating Away From Legacy Finders +### Legacy Adapter/Serializer Support We would introduce new url-building and request building utility functions in a new package `@ember-data/request-utils`. From 0a829a403b3b547d769326142d02bb08620c2e30 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Fri, 11 Nov 2022 17:08:43 -0800 Subject: [PATCH 03/12] rename file --- ...data-request-service.md => 0860-ember-data-request-service.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename text/{0000-ember-data-request-service.md => 0860-ember-data-request-service.md} (100%) diff --git a/text/0000-ember-data-request-service.md b/text/0860-ember-data-request-service.md similarity index 100% rename from text/0000-ember-data-request-service.md rename to text/0860-ember-data-request-service.md From 9d5d9dd43a62aeb250df043c854edd351143634f Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Fri, 11 Nov 2022 18:14:34 -0800 Subject: [PATCH 04/12] add lifetimes overview --- text/0860-ember-data-request-service.md | 63 +++++++++++++++++++++++-- 1 file changed, 58 insertions(+), 5 deletions(-) diff --git a/text/0860-ember-data-request-service.md b/text/0860-ember-data-request-service.md index a23b4f0fd1..2171fe35c8 100644 --- a/text/0860-ember-data-request-service.md +++ b/text/0860-ember-data-request-service.md @@ -46,6 +46,15 @@ A `RequestManager` provides a request/response flow in which configured handlers are successively given the opportunity to handle, modify, or pass-along a request. +```ts +interface RequestManager { + async request(req: RequestInfo): Future; +} +``` + + +For example: + ```ts import RequestManager, { Fetch } from '@ember-data/request'; import Auth from 'ember-simple-auth/ember-data-middleware'; @@ -123,10 +132,6 @@ interface Handler { for building up the `StructuredDocument` and `Future` that will be part of the response. - Note: `setStream` MUST be called synchronously before awaiting any async within - the method in order for `getStream` to work appropriately. - - ```ts interface RequestContext { readonly request: RequestInfo; @@ -137,7 +142,6 @@ interface RequestContext { interface RequestInfo { url: string; - cache?: { key?: string, reload?: boolean, backgroundReload?: boolean }; method: 'GET' | 'POST' | 'PUT' | 'DELETE' | 'PATCH'; data?: Record; options?: Record; @@ -229,8 +233,57 @@ export default class extends Store { ### Using `store.request()` +The `Store` will add support for using the `RequestManager` via `store.request()`. + +```ts +class Store { + async request(req: RequestInfo): Future>; +} +``` + +There are two significant differences when calling `store.request` instead of `requestManager.request`. + +1) the Store will be added to `RequestInfo`, and an additional `cacheOptions` property is available + +```ts +interface StoreRequestInfo extends RequestInfo { + cacheOptions?: { key?: string, reload?: boolean, backgroundReload?: boolean }; + store: Store; +} +``` + +2) The `StructuredDocument` is supplied to `cache.put(doc)` and the return value's + `data` member is altered to either a single record or array of records resulting + from instantiating the entities contained in the `ResourceDocument` returned by + `cache.put`. + ### Cache Lifetimes +In the past, cache lifetimes for single resources were controlled by either +supplying the `reload` and `backgroundReload` options or by the Adapter's hooks +for `shouldReloadRecord`, `shouldReloadAll`, `shouldBackgroundReloadRecord` and +`shouldBackgroundReloadAll`. + +This behavior will now be controlled by the combination of either supplying `cacheOptions` +on the associated `RequestInfo` or by supplying a `lifetimes` service to the `Store`. + +```ts +class Store { + lifetimes: LifetimesService; +} + +interface LifetimesService { + isExpired(url: string, method: HTTPMethod) {} +} +``` + +**Legacy Compatibility** + +In order to support the legacy adapter-driven lifetime behaviors of `findRecord` +and similar store methods, these methods will still consult the adapter prior to +consulting the lifetimes service. Requests that originate through `store.request` +will not consult the Adapter methods. + ### Migrating Away From Legacy Finders ### Legacy Adapter/Serializer Support From ae9a4e14d198254d8870bc56fd697564ec3a938e Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Fri, 11 Nov 2022 18:16:03 -0800 Subject: [PATCH 05/12] update PR link --- text/0860-ember-data-request-service.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0860-ember-data-request-service.md b/text/0860-ember-data-request-service.md index 2171fe35c8..37dd19cef7 100644 --- a/text/0860-ember-data-request-service.md +++ b/text/0860-ember-data-request-service.md @@ -6,7 +6,7 @@ release-versions: teams: - data prs: - accepted: # update this to the PR that you propose your RFC in + accepted: https://github.com/emberjs/rfcs/pull/860 project-link: --- From 1fe281282523ce9c7d2ea46db6673294597fb06d Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Fri, 11 Nov 2022 18:16:38 -0800 Subject: [PATCH 06/12] add start date --- text/0860-ember-data-request-service.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0860-ember-data-request-service.md b/text/0860-ember-data-request-service.md index 37dd19cef7..f466360d8f 100644 --- a/text/0860-ember-data-request-service.md +++ b/text/0860-ember-data-request-service.md @@ -1,6 +1,6 @@ --- stage: accepted -start-date: +start-date: 2023-11-10 release-date: release-versions: teams: From 9ce52c6be04ca32d5db2ad6f60edc8fe556fcd6a Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Fri, 11 Nov 2022 18:36:25 -0800 Subject: [PATCH 07/12] more updates --- text/0860-ember-data-request-service.md | 58 ++++++++++++++++++++----- 1 file changed, 47 insertions(+), 11 deletions(-) diff --git a/text/0860-ember-data-request-service.md b/text/0860-ember-data-request-service.md index f466360d8f..6d2f34f418 100644 --- a/text/0860-ember-data-request-service.md +++ b/text/0860-ember-data-request-service.md @@ -88,7 +88,7 @@ resolve until the response stream is fully read. interface Future extends Promise> { abort(): void; - async stream(): ReadableStream | null; + async getStream(): ReadableStream | null; } ``` @@ -231,6 +231,26 @@ export default class extends Store { } ``` +If using the package `ember-data`, the following configuration will automatically be done in order +to preserve the legacy Adapter and Serializer behavior. Additional handlers or a service injection +like the above would need to be done by the consuming application in order to make broader use of +`RequestManager`. + +```ts +import Store from '@ember-data/store'; +import RequestManager from '@ember-data/request'; +import LegacyHandler from '@ember-data/legacy-network-handler'; + +export default class extends Store { + requestManager = new RequestManager(); + + constructor(args) { + super(args); + this.requestManager.use([LegacyHandler]); + } +} +``` + ### Using `store.request()` The `Store` will add support for using the `RequestManager` via `store.request()`. @@ -284,18 +304,39 @@ and similar store methods, these methods will still consult the adapter prior to consulting the lifetimes service. Requests that originate through `store.request` will not consult the Adapter methods. -### Migrating Away From Legacy Finders - ### Legacy Adapter/Serializer Support +In order to provide migration support for Adapter and Serializer, a `LegacyNetworkHandler` would be +provided. This handler would take a request and convert it into the older form, calling the appropriate +Adapter and Serializer methods. If no adapter exists for the type (including no application adapter), this +handler would call `next`. In this manner an app can incrementally migrate request-handling to this +new paradigm on a per-type basis as desired. + +The package `ember-data` would automatically configure this handler. If not using `ember-data` +this configuration would need to be done explicitly. + +We intend to support this handler through at least the 5.x series, not deprecating it's usage +before 6.0. + +Similarly, the methods `adapterFor` and `serializerFor` will not be deprecated until at least 6.0; +however, it should no longer be assumed that an application has an adapter or serializer at all. + +### Migrating Away From Legacy Finders + We would introduce new url-building and request building utility functions in a new package `@ember-data/request-utils`. -We would introduce a new middleware in it's own package `@ember-data/legacy-network-middleware` -to encapsulate legacy behaviors of adapters, and serializers. +### Deprecating Legacy Finders -We would introduce a new lifetimes behavior to the `Store` to replace the reload APIs on `Adapter`. +We would not deprecate methods on the Store for requesting data until at least 5.x; however, + applications should begin migrating all requests to this new paradigm and expect that the + following methods will be deprecated at some point during the 5.x cycle + - `store.findRecord` + - `store.findAll` + - `store.query` + - `store.queryRecord` + - `store.saveRecord` ## How we teach this > What names and terminology work best for these concepts and why? How is this @@ -322,8 +363,3 @@ on the impact of the API churn on existing apps, etc. > What other designs have been considered? What is the impact of not doing this? > This section could also include prior art, that is, how other frameworks in the same domain have solved this problem. - -## Unresolved questions - -> Optional, but suggested for first drafts. What parts of the design are still -TBD? From 7f7f3fcc923de3fb62afab6b6423e6a26b19ec73 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Sat, 12 Nov 2022 00:01:19 -0800 Subject: [PATCH 08/12] finalize --- text/0860-ember-data-request-service.md | 233 +++++++++++++++++++++--- 1 file changed, 208 insertions(+), 25 deletions(-) diff --git a/text/0860-ember-data-request-service.md b/text/0860-ember-data-request-service.md index 6d2f34f418..57a41a7b60 100644 --- a/text/0860-ember-data-request-service.md +++ b/text/0860-ember-data-request-service.md @@ -36,7 +36,7 @@ flows. Provides associated utilities to assist in migrating to this new abstract - `Serializer` lacks the context necessary to serialize/normalize data on a per-request basis - `Adapter` is inflexible and difficult to grow as an interface for managing data fulfillment from a source. -- Applications have need of a low-level primitive solution for managed fetch to ensuring proper headers, authentication, error handling, SSR support, test-waiter support, and request de-duping/caching. +- Applications have need of a low-level primitive solution for managed fetch to ensure proper headers, authentication, error handling, SSR support, test-waiter support, and request de-duping/caching. ## Detailed design @@ -119,6 +119,9 @@ Requests are fulfilled by handlers. A handler receives the request context as well as a `next` function with which to pass along a request to the next handler if they so choose. +If a handler calls `next`, it receives a `Future` which resolves to a `StructuredDocument` +that it can then compose how it sees fit with its own response. + ```ts type NextFn

= (req: RequestInfo) => Future

; @@ -136,7 +139,7 @@ interface Handler { interface RequestContext { readonly request: RequestInfo; - setStream(stream: ReadableStream): void; + setStream(stream: ReadableStream | Promise): void; setResponse(response: Response): void; } @@ -180,7 +183,76 @@ first-out), and may only be registered up until the first request is made. It is recommended but not required to register all handlers at one time in order to ensure explicitly visible handler ordering. -**The Middleware API** +**Stream Currying** + +`RequestManager.request` differs from `fetch` in one **extremely crucial detail** +and we feel the need to deeply describe how and why. + +For context, it helps to understand a few of the use-cases that RequestManager +is intended to allow. + +- Historically EmberData could not be used to manage and return streaming content (such as + video files). With this change, it can be. (The Identifiers RFC and Cache 2.1 RFCs also + make this ability pervasive throughout all layers of EmberData) +- It might be the case that a handler "tees" or "forks" a request, fulfilling it by either + making multiple parallel `fetch` requests, or by calling `next` multiple times, or by + fulfilling part of the request from one source (one API, in-memory, localStorage, IndexedDB + etc.) and the rest from another source (a different API, a WebWorker, etc.) +- Handlers may only be amending the request and passing it along, for instance an Auth handler + may simply be ensuring the correct tokens or headers or cookies are attached. + +`await fetch()` behaves differently than some realize. The fetch promise resolves not +once the entirety of the request has been received but rather at the moment headers are +received. This allows for the body of the request to be processed as a stream by application +code *while chunks are still being received by the browser*. When for an app chooses to +`await response.json()` what actually occurs is the browser reads the stream to completion +and then returns the result. Additionally, this stream may only be read **once**. + +In designing the `RequestManager`, we do not want to eliminate this ability to subscribe to +and utilize the stream. We believe it crucial that the full power and flexibility of native +APIs remains in developers hands, and do not want to create a restriction such that developers +feel the need to create complicated workarounds for what would feel like an unnecessary +restriction to gain access to built-in APIs. + +However, because there is potentially a chain of handlers involved, and because there are +potentially multiple streams involved, and because we require that `await manager.request()` + resolves with fully realized content, we find ourselves in a design conundrum. + +We have considered several variations on how to support streams: from a two-tiered promise structure + similar to `fetch` (which quickly fails due to the chained nature of handlers), to enforcing that +handlers synchronously call `setStream` either with a ReadableStream or a promise resolving to one. + +Each variation has had drawbacks, some were critical and some simply had poor ergonomics. What we +have arrived at is this: + +Each handler may call `setStream` only once, but may do so *at any time* until the promise that it +returns has resolved. The associated promise returned by calling `future.getStream` will resolve +with the stream set by `setStream` if that method is called, or `null` if that method has not been +called by the time that the handler's request method has resolved. + +Handlers that do not create a stream of their own, but which call `next`, may defensively +pipe the stream forward; however, this is not required (see automatic currying below). + +```ts +context.setStream(future.getStream()); +``` + +Handlers that either call `next` multiple times or otherwise have reason to create multiple +fetch requests should either choose to return no stream, meaningfully combine the streams, +or select a single prioritized stream. + +Of course, any handler may choose to read and handle the stream, and return either no stream or +a different stream in the process. + +**Automatic Currying of Stream and Response** + +In order to simplify what we believe will be a common case for handlers which are merely decorating +a request, if `next` is called only a single time and `setResponse` was never called by the handler +the response set by the next handler in the chain will applied to that handler's outcome. For +instance, this makes the following pattern work `return (await next()).data;`. + +Similarly, if `next` is called only a single time and neither `setStream` nor `getStream` was + called, we automatically curry the stream from the future returned by `next` onto the future returned by the handler. **Using as a Service** @@ -261,7 +333,7 @@ class Store { } ``` -There are two significant differences when calling `store.request` instead of `requestManager.request`. +There are three significant differences when calling `store.request` instead of `requestManager.request`. 1) the Store will be added to `RequestInfo`, and an additional `cacheOptions` property is available @@ -277,6 +349,38 @@ interface StoreRequestInfo extends RequestInfo { from instantiating the entities contained in the `ResourceDocument` returned by `cache.put`. +3) Both an operation (`op`) and and array of identifiers (`records`) may be provided + as part of the request. While this information could also be included in `options`, + we are giving it top-level precedence since handlers which perform data normalization + will almost always require this information. + + `op` may be any `string` that your handlers will recognize, though EmberData will provide + an `op` matching one of the current Adapter request types when it is used to build the + RequestInfo object. + + `records` should be all records expected to be saved or fetched during the course of + the request. Similarly, EmberData will populate this for you when using the request-builders + or when the request is generated by the Store. This list will be used to update the status + of the `RequestStateService` detailed in [RFC #466](https://rfcs.emberjs.com/id/0466-request-state-service) + +```ts +interface StoreRequestInfo extends RequestInfo { + cacheOptions?: { key?: string, reload?: boolean, backgroundReload?: boolean }; + store: Store; + + op?: 'findRecord' | 'updateRecord' | 'query' | 'queryRecord' | 'findBelongsTo' | 'findHasMany' | 'createRecord' | 'deleteRecord'; + records?: StableRecordIdentifier[]; +} +``` + +### RequestStateService + +We do not intend to make any adjustments to the RequestStateService at this time, though +this new paradigm enables us to easily manage a list of requests key'd by URL that could +be useful for both application code and the Ember Inspector. If you are interested in adding +such support, we would accept an RFC. With the greatly improved flow this RFC brings we +expect that the overall design of the RequestStateService ought to be revisited. + ### Cache Lifetimes In the past, cache lifetimes for single resources were controlled by either @@ -323,43 +427,122 @@ however, it should no longer be assumed that an application has an adapter or se ### Migrating Away From Legacy Finders -We would introduce new url-building and request building utility functions in a new package -`@ember-data/request-utils`. +In order to support transitioning to this new paradigm, we would introduce new url-building +and request-building utility functions in a new package (`@ember-data/request-utils`) that +closely mirror what occurs by using the corresponding store and Adapter methods today. + +Note: the lack of `findAll` in this list is intentional, we do not intend to implement this +separately from `query`. + +```ts +import { findRecord, queryRecord, query, updateRecord, createRecord, deleteRecord, saveRecord } from '@ember-data/request-utils'; + +const { data: user } = await store.request(findRecord('user', '1')); +const { data: user } = await store.request(queryRecord('user', { username: 'runspired' })); +const { data: users } = await store.request(query('user', {})); + +await store.request(updateRecord(user)); +await store.request(createRecord(user)); +await store.request(deleteRecord(user)); +await store.request(saveRecord(user)); +``` + +Each of these request-builders returns an object satisfying the `RequestInfo` interface, which +could also be manually constructed. + +Additionally, a url-builder similar in behavior to the `BuildURLMixin` is provided. +Notable differences include that is also serializes query params into the URL, and +assumes the first argument is the "path for type". + +The following config properties will be supply-able via the app's `ember-cli-build` + +```ts +interface Config { + '@embroider/macros': { + setConfig: { + '@ember-data/request-utils': { + { + apiNamespace: string; + apiHost: string; + } + } + } + } +} +``` + + +```ts +import { buildUrl } from '@ember-data/request-utils'; + +// findRecord with include +const url = buildUrl('user', '1', { include: 'friends' }); + +// query page 3 of users +const url = buildUrl('users', null, { limit: 25, offset: 50 }); + +// query for a single user +const url = buildUrl('user', null, { username: 'runspired' }); + +// query the first page of comments for post 1 +const url = buildUrl('post/1/comments/list', null, { limit: 10, offset: 0 }); +``` ### Deprecating Legacy Finders -We would not deprecate methods on the Store for requesting data until at least 5.x; however, - applications should begin migrating all requests to this new paradigm and expect that the - following methods will be deprecated at some point during the 5.x cycle +We would not immediately deprecate methods on the Store for requesting data until at +least 5.0; however, applications should begin migrating all requests to this new +paradigm and expect that the following methods will be deprecated at some point during +the 5.x cycle - `store.findRecord` - `store.findAll` - `store.query` - `store.queryRecord` - `store.saveRecord` + +### Migrating Away from Serializers + +We do not intend to provide an alternative to Serializers in any form. Instead, +given the current power and flexibility of the Cache, we recommend aligning the +Cache implementation with your API implementation. + +If data normalization is still needed, we recommend writing a few helper functions +that a handler can use to quickly transform the data as necessary. + +We expect some addons may be created that offer helper functions for common transformations. +Since Serializers will not be officially deprecated until some point after 6.0, we feel +that this is more than ample time for applications and addons to explore this space and +either become comfortable with the realization that such data transformation is largely + unecessary and wasteful or can be done via much simpler and surgical mechanisms. + ## How we teach this -> What names and terminology work best for these concepts and why? How is this -idea best presented? As a continuation of existing Ember patterns, or as a -wholly new one? +- EmberData should create new documentation and guides to cover using the RequestManager. -> Would the acceptance of this proposal mean the Ember guides must be -re-organized or altered? Does it change how Ember is taught to new users -at any level? +- Existing documentation and guides should be re-written to either reference these new +patterns or to be clear that they discuss a legacy pattern that is no longer recommended. -> How should this feature be introduced and taught to existing Ember -users? +- The learning story for EmberData should be reworked to one that incrementally grows from + a simple abstraction over fetch, to fetch with a cache, to fetch with a cache and resource + graph and presentational concerns. ## Drawbacks -> Why should we *not* do this? Please consider the impact on teaching Ember, -on the integration of this feature with other existing and planned features, -on the impact of the API churn on existing apps, etc. - -> There are tradeoffs to choosing any path, please attempt to identify them here. +Historically, Adapters have hidden away construction of requests from app-developers kept + application code focused only on working with data that was magically fetched and processed + in the background. When this worked, it worked very well, and many users have loved this + magic deeply. However, this abstraction came at the great cost of making EmberData difficult + to fit into many common scenarios, difficult to reason about and debug when data-fetching + failed, and difficult to extend when even very trivial changes to request construction were + required. We do not feel the occassional magic of it all working outweights the drawbacks of + keeping the system as is, and so we have chosen a slightly more verbose approach that grants + developers flexibity, power, and ease-of-use. ## Alternatives -> What other designs have been considered? What is the impact of not doing this? - -> This section could also include prior art, that is, how other frameworks in the same domain have solved this problem. +We considered building this over the existing Adapter interface, deprecating Serializers and +instead encouraging data-transformation to be done within the Adapter. In fact, this pattern +is fully possible today, we could just better document it and do nothing more. However, this +approach does not solve the need for more general request management, nor does it interact +well with common development paradigms such as GraphQL query building. \ No newline at end of file From cbff7e637d4d9444357fd9006ae640a50e3a2af2 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Sat, 12 Nov 2022 00:50:39 -0800 Subject: [PATCH 09/12] add clarifying note --- text/0860-ember-data-request-service.md | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/text/0860-ember-data-request-service.md b/text/0860-ember-data-request-service.md index 57a41a7b60..a5bc7a081c 100644 --- a/text/0860-ember-data-request-service.md +++ b/text/0860-ember-data-request-service.md @@ -230,8 +230,9 @@ returns has resolved. The associated promise returned by calling `future.getStre with the stream set by `setStream` if that method is called, or `null` if that method has not been called by the time that the handler's request method has resolved. -Handlers that do not create a stream of their own, but which call `next`, may defensively -pipe the stream forward; however, this is not required (see automatic currying below). +Handlers that do not create a stream of their own, but which call `next`, should defensively +pipe the stream forward. While this is not required (see automatic currying below) it is better +to do so in most cases. ```ts context.setStream(future.getStream()); @@ -254,6 +255,12 @@ instance, this makes the following pattern work `return (await next()).data Similarly, if `next` is called only a single time and neither `setStream` nor `getStream` was called, we automatically curry the stream from the future returned by `next` onto the future returned by the handler. +Finally, if the return value of a handler is a `Future`, we curry the entire thing. This makes the + following possible and ensures even `data` is curried when doing so: `return next()`. + +In the case of the `Future` being returned, `Stream` proxying is automatic and immediate and does +not wait for the `Future` to resolve. + **Using as a Service** Most applications will desire to have a single `RequestManager` instance, which From 2160863c557ccbd02f3eed8d482db0bb118daedf Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Sat, 12 Nov 2022 13:13:06 -0800 Subject: [PATCH 10/12] make import path nicer --- text/0860-ember-data-request-service.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0860-ember-data-request-service.md b/text/0860-ember-data-request-service.md index a5bc7a081c..7830edc171 100644 --- a/text/0860-ember-data-request-service.md +++ b/text/0860-ember-data-request-service.md @@ -57,7 +57,7 @@ For example: ```ts import RequestManager, { Fetch } from '@ember-data/request'; -import Auth from 'ember-simple-auth/ember-data-middleware'; +import Auth from 'ember-simple-auth/ember-data-handler'; import Config from './config'; const { apiUrl } = Config; @@ -270,7 +270,7 @@ applications by exporting the manager as an Ember service. *services/request.ts* ```ts import RequestManager, { Fetch } from '@ember-data/request'; -import Auth from 'ember-simple-auth/ember-data-middleware'; +import Auth from 'ember-simple-auth/ember-data-handler'; export default class extends RequestManager { constructor(createArgs) { From 817283feda6ae07dd5b1d66fd3ab81d2e0c9c73e Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Mon, 14 Nov 2022 11:47:55 -0800 Subject: [PATCH 11/12] update text --- text/0860-ember-data-request-service.md | 86 +++++++++++++++++-------- 1 file changed, 58 insertions(+), 28 deletions(-) diff --git a/text/0860-ember-data-request-service.md b/text/0860-ember-data-request-service.md index 7830edc171..c7f2437d86 100644 --- a/text/0860-ember-data-request-service.md +++ b/text/0860-ember-data-request-service.md @@ -34,9 +34,16 @@ flows. Provides associated utilities to assist in migrating to this new abstract ## Motivation - `Serializer` lacks the context necessary to serialize/normalize data on a per-request basis + - This is especially true when performing "actions", RPC style requests, "partial" save + requests, and "transactional" saves + - Often users end up needing to pre-normalize in the adapter in order to supply information + contained in either `headers` or to convert into `JSON` from other forms (such as `jsonb`, `json5` `protocol buffers` or similar) - `Adapter` is inflexible and difficult to grow as an interface for managing data fulfillment from a source. - Applications have need of a low-level primitive solution for managed fetch to ensure proper headers, authentication, error handling, SSR support, test-waiter support, and request de-duping/caching. +- The `Adapter` pattern stands in the way of pagination-by-default and query caching +- The `Adapter` pattern does not fit with many common data-fetching paradigms today +- The `Adapter` pattern does not fit with transactional saves ## Detailed design @@ -117,7 +124,7 @@ interface StructuredDocument { Requests are fulfilled by handlers. A handler receives the request context as well as a `next` function with which to pass along a request to the next -handler if they so choose. +handler if it so chooses. If a handler calls `next`, it receives a `Future` which resolves to a `StructuredDocument` that it can then compose how it sees fit with its own response. @@ -202,17 +209,17 @@ is intended to allow. may simply be ensuring the correct tokens or headers or cookies are attached. `await fetch()` behaves differently than some realize. The fetch promise resolves not -once the entirety of the request has been received but rather at the moment headers are +once the entirety of the request has been received, but rather at the moment headers are received. This allows for the body of the request to be processed as a stream by application -code *while chunks are still being received by the browser*. When for an app chooses to +code *while chunks are still being received by the browser*. When an app chooses to `await response.json()` what actually occurs is the browser reads the stream to completion and then returns the result. Additionally, this stream may only be read **once**. In designing the `RequestManager`, we do not want to eliminate this ability to subscribe to -and utilize the stream. We believe it crucial that the full power and flexibility of native -APIs remains in developers hands, and do not want to create a restriction such that developers -feel the need to create complicated workarounds for what would feel like an unnecessary -restriction to gain access to built-in APIs. +and utilize the stream by either the application or the handler. We believe it crucial that +the full power and flexibility of native APIs remains in developers hands, and do not want +to create a restriction such that developers feel the need to create complicated workarounds +for what would feel like an unnecessary restriction to gain access to built-in APIs. However, because there is potentially a chain of handlers involved, and because there are potentially multiple streams involved, and because we require that `await manager.request()` @@ -225,14 +232,15 @@ handlers synchronously call `setStream` either with a ReadableStream or a promis Each variation has had drawbacks, some were critical and some simply had poor ergonomics. What we have arrived at is this: -Each handler may call `setStream` only once, but may do so *at any time* until the promise that it -returns has resolved. The associated promise returned by calling `future.getStream` will resolve -with the stream set by `setStream` if that method is called, or `null` if that method has not been -called by the time that the handler's request method has resolved. +Each handler may call `setStream` only once, but may do so *at any time* until the promise that +the handler returns has resolved. The associated promise returned by calling `future.getStream` +will resolve with the stream set by `setStream` if that method is called, or `null` if that method +has not been called by the time that the handler's request method has resolved. Handlers that do not create a stream of their own, but which call `next`, should defensively pipe the stream forward. While this is not required (see automatic currying below) it is better -to do so in most cases. +to do so in most cases as otherwise the stream may not become available to downstream handlers +or the application until the upstream handler has fully read it. ```ts context.setStream(future.getStream()); @@ -249,7 +257,7 @@ a different stream in the process. In order to simplify what we believe will be a common case for handlers which are merely decorating a request, if `next` is called only a single time and `setResponse` was never called by the handler -the response set by the next handler in the chain will applied to that handler's outcome. For +the response set by the next handler in the chain will be applied to that handler's outcome. For instance, this makes the following pattern work `return (await next()).data;`. Similarly, if `next` is called only a single time and neither `setStream` nor `getStream` was @@ -498,9 +506,9 @@ const url = buildUrl('post/1/comments/list', null, { limit: 10, offset: 0 }); ### Deprecating Legacy Finders We would not immediately deprecate methods on the Store for requesting data until at -least 5.0; however, applications should begin migrating all requests to this new +least 6.0; however, applications should begin migrating all requests to this new paradigm and expect that the following methods will be deprecated at some point during -the 5.x cycle +the 6.x cycle - `store.findRecord` - `store.findAll` @@ -508,21 +516,39 @@ the 5.x cycle - `store.queryRecord` - `store.saveRecord` +Users that want to maintain these finder methods for longer would be able to add them back + within their own application or library if desired; however, because these methods cannot + easily utilize the full feature set of the cache, pagination, or request-manager we expect + that their utility will diminish quickly. + ### Migrating Away from Serializers -We do not intend to provide an alternative to Serializers in any form. Instead, +We do not intend to provide a direct replacement of Serializers in any form. Instead, given the current power and flexibility of the Cache, we recommend aligning the Cache implementation with your API implementation. If data normalization is still needed, we recommend writing a few helper functions -that a handler can use to quickly transform the data as necessary. - +that a handler can use to quickly transform the data as necessary. Due to having better +context of the request, and due to the much smaller surface area to reason about, writing +a function to transform data between formats should prove to be simple, quick and effective. We expect some addons may be created that offer helper functions for common transformations. + Since Serializers will not be officially deprecated until some point after 6.0, we feel that this is more than ample time for applications and addons to explore this space and either become comfortable with the realization that such data transformation is largely unecessary and wasteful or can be done via much simpler and surgical mechanisms. +Of course, users can always choose to continue using Serializers (and Adapters) forever. +Their deprecation within EmberData will be scoped to (1) EmberData itself no longer being +aware of the concept and (2) the `adapter` and `serializer` packages being deprecated. + +If desired, other libraries could take on support of these packages, and make use of +public APIs to restore these behaviors, utilizing the same public APIs EmberData will +use to support them until deprecated. We suspect, however, the insane improvement in ergonomics +and feature-set that this shift brings will –over the course of the few years prior to full +removal– prove to users that the Adapter and Serializer world is no longer the best paradigm +for their applications. + ## How we teach this - EmberData should create new documentation and guides to cover using the RequestManager. @@ -536,15 +562,18 @@ patterns or to be clear that they discuss a legacy pattern that is no longer rec ## Drawbacks -Historically, Adapters have hidden away construction of requests from app-developers kept - application code focused only on working with data that was magically fetched and processed - in the background. When this worked, it worked very well, and many users have loved this - magic deeply. However, this abstraction came at the great cost of making EmberData difficult - to fit into many common scenarios, difficult to reason about and debug when data-fetching - failed, and difficult to extend when even very trivial changes to request construction were - required. We do not feel the occassional magic of it all working outweights the drawbacks of - keeping the system as is, and so we have chosen a slightly more verbose approach that grants - developers flexibity, power, and ease-of-use. +Historically, Adapters hid away construction of requests from app-developers which kept +application code focused only on working with data that was magically fetched and processed +in the background. + +When this worked, it worked very well, and many users have loved this magic deeply. However, +this abstraction came at the great cost of making EmberData difficult to fit into many common +scenarios, difficult to reason about and debug when data-fetching failed, and difficult to +extend when even very trivial changes to request construction were required. + +We do not feel the occassional magic of it all working outweighs the drawbacks of +keeping the system as is, and so we have chosen a slightly more verbose approach that grants +developers flexibity, power, and ease-of-use. ## Alternatives @@ -552,4 +581,5 @@ We considered building this over the existing Adapter interface, deprecating Ser instead encouraging data-transformation to be done within the Adapter. In fact, this pattern is fully possible today, we could just better document it and do nothing more. However, this approach does not solve the need for more general request management, nor does it interact -well with common development paradigms such as GraphQL query building. \ No newline at end of file +well with common development paradigms such as GraphQL query building, nor does it allow us +to introduce pagination-by-default. \ No newline at end of file From 2093cdeb8dd6291e6686c16c54b09762063ba095 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Tue, 22 Nov 2022 23:59:18 -0800 Subject: [PATCH 12/12] update rfc text --- text/0860-ember-data-request-service.md | 149 +++++++++++++++++------- 1 file changed, 108 insertions(+), 41 deletions(-) diff --git a/text/0860-ember-data-request-service.md b/text/0860-ember-data-request-service.md index c7f2437d86..47765512ab 100644 --- a/text/0860-ember-data-request-service.md +++ b/text/0860-ember-data-request-service.md @@ -55,7 +55,7 @@ to handle, modify, or pass-along a request. ```ts interface RequestManager { - async request(req: RequestInfo): Future; + request(req: RequestInfo): Future; } ``` @@ -63,7 +63,8 @@ interface RequestManager { For example: ```ts -import RequestManager, { Fetch } from '@ember-data/request'; +import { RequestManager } from '@ember-data/request'; +import { Fetch } from '@ember/data/request/fetch'; import Auth from 'ember-simple-auth/ember-data-handler'; import Config from './config'; @@ -99,42 +100,100 @@ interface Future extends Promise> { } ``` -The `StructuredDocument` interface is the same as is proposed in RFC 854 and copied below: +The `StructuredDocument` interface is the same as is proposed in emberjs/rfcs#854 but is shown here in richer detail. ```ts -interface StructuredDocument { - request: { - url: string; - cache?: { key?: string, reload?: boolean, backgroundReload?: boolean }; - method: 'GET' | 'POST' | 'PUT' | 'DELETE' | 'PATCH'; - data?: Record; - options?: Record; - headers: Record; - } - response: { - status: HTTPStatusCode; - headers: Record; - } +interface RequestInfo { + /** + * data that a handler should convert into + * the query (GET) or body (POST) + */ + data?: Record; + /** + * options specifically intended for handlers + * to utilize to process the request + */ + options?: Record; + /** + * Allows supplying a custom AbortController for + * the request, if none is supplied one is generated + * for the request. When calling `next` if none is + * provided the primary controller for the request + * is used. + * + * controller will not be passed through onto the immutable + * request on the context supplied to handlers. + */ + controller?: AbortController; + + // the below options perfectly mirror the + // native Request interface + cache?: RequestCache; + credentials?: RequestCredentials; + destination?: RequestDestination; + /** + * Once a request has been made it becomes immutable, this + * includes Headers. To modify headers you may copy existing + * headers using `new Headers([...headers.entries()])`. + * + * Immutable headers instances have an additional method `clone` + * to allow this to be done swiftly. + */ + headers?: Headers; + integrity?: string; + keepalive?: boolean; + method?: string; + mode?: RequestMode; + redirect?: RequestRedirect; + referrer?: string; + referrerPolicy?: ReferrerPolicy; + /** + * Typically you should not set this, though you may choose to curry + * a received signal if calling next. signal will automatically be set + * to the associated controller's signal if none is supplied. + */ + signal?: AbortSignal; + url?: string; +} +interface ResponseInfo { + headers: Headers; + ok: boolean; + redirected: boolean; + status: number; + statusText: string; + type: string; + url: string; +} + +interface StructuredDataDocument { + request: RequestInfo; + response: ResponseInfo; data: T; - error?: Error; } +interface StructuredErrorDocument extends Error { + request: RequestInfo; + response: ResponseInfo; + error: string | object; +} +type StructuredDocument = StructuredDataDocument | StructuredErrorDocument; ``` +A `Future` resolves with a StructuredDataDocument or rejects with a StructuredErrorDocument. + **Request Handlers** Requests are fulfilled by handlers. A handler receives the request context as well as a `next` function with which to pass along a request to the next handler if it so chooses. -If a handler calls `next`, it receives a `Future` which resolves to a `StructuredDocument` +If a handler calls `next`, it receives a `Future` which fuulfills to a `StructuredDocument` that it can then compose how it sees fit with its own response. ```ts +type NextFn =

(req: RequestInfo) => Future

; -type NextFn

= (req: RequestInfo) => Future

; - -interface Handler { - async request(context: RequestContext, next: NextFn

): T; +interface Handler { + request(context: RequestContext, next: NextFn): T; } ``` @@ -146,17 +205,8 @@ interface Handler { interface RequestContext { readonly request: RequestInfo; - setStream(stream: ReadableStream | Promise): void; - setResponse(response: Response): void; -} - -interface RequestInfo { - url: string; - method: 'GET' | 'POST' | 'PUT' | 'DELETE' | 'PATCH'; - data?: Record; - options?: Record; - headers: Record; - signal: AbortSignal; + setStream(stream: ReadableStream | Promise): void; + setResponse(response: ResponseInfo | Response | null): void; } ``` @@ -264,10 +314,9 @@ Similarly, if `next` is called only a single time and neither `setStream` nor `g called, we automatically curry the stream from the future returned by `next` onto the future returned by the handler. Finally, if the return value of a handler is a `Future`, we curry the entire thing. This makes the - following possible and ensures even `data` is curried when doing so: `return next()`. + following possible and ensures even `data` and `error` is curried when doing so: `return next()`. -In the case of the `Future` being returned, `Stream` proxying is automatic and immediate and does -not wait for the `Future` to resolve. +In the case of the `Future` being returned from a handler not using `async/await`, `Stream` proxying is automatic and immediate and does not wait for the `Future` to resolve. If the handler uses `async/await` we have no ability to detect the Future until the handler has fully resolved. This means that if using `async/await` in your handler you should always pro-actively pipe the stream. **Using as a Service** @@ -277,7 +326,8 @@ applications by exporting the manager as an Ember service. *services/request.ts* ```ts -import RequestManager, { Fetch } from '@ember-data/request'; +import { RequestManager } from '@ember-data/request'; +import { Fetch } from '@ember/data/request/fetch'; import Auth from 'ember-simple-auth/ember-data-handler'; export default class extends RequestManager { @@ -306,7 +356,8 @@ Alternatively to have a request service unique to the store: ```ts import Store from '@ember-data/store'; -import RequestManager, { Fetch } from '@ember-data/request'; +import { RequestManager } from '@ember-data/request'; +import { Fetch } from '@ember/data/request/fetch'; export default class extends Store { requestManager = new RequestManager(); @@ -325,8 +376,8 @@ like the above would need to be done by the consuming application in order to ma ```ts import Store from '@ember-data/store'; -import RequestManager from '@ember-data/request'; -import LegacyHandler from '@ember-data/legacy-network-handler'; +import { RequestManager } from '@ember-data/request'; +import { LegacyHandler } from '@ember-data/legacy-network-handler'; export default class extends Store { requestManager = new RequestManager(); @@ -344,7 +395,7 @@ The `Store` will add support for using the `RequestManager` via `store.request(< ```ts class Store { - async request(req: RequestInfo): Future>; + request(req: RequestInfo): Future>; } ``` @@ -388,6 +439,13 @@ interface StoreRequestInfo extends RequestInfo { } ``` +**Background Reload Error Handling** + +When an error occurs during a background request we will update the cache with the StructuredErrorDocument but will swallowed the Error at that point. + +This prevents consuming applications from being required to catch the error unless +they wish to via a handler. + ### RequestStateService We do not intend to make any adjustments to the RequestStateService at this time, though @@ -549,6 +607,15 @@ and feature-set that this shift brings will –over the course of the few years removal– prove to users that the Adapter and Serializer world is no longer the best paradigm for their applications. +### Typescript Support + +Although EmberData has not more broadly shipped support for Typescript, experimental types +will be shipped specifically for the RequestManager package. We can do this because the lack +of entanglement with the other packages affords us the ability to more safely ship this subset +of types while the others are still incomplete. + +Types for other packages will eventually be provided but we will not rush them at this time. + ## How we teach this - EmberData should create new documentation and guides to cover using the RequestManager.