From bbbcae9a7ae7c78ba0168bc57dc11d9456255b6a Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Fri, 10 Jan 2025 20:39:04 -0500 Subject: [PATCH 1/7] Deprecate ember-fetch --- text/0000-remove-ember-fetch.md | 141 ++++++++++++++++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 text/0000-remove-ember-fetch.md diff --git a/text/0000-remove-ember-fetch.md b/text/0000-remove-ember-fetch.md new file mode 100644 index 0000000000..7cdf9907a2 --- /dev/null +++ b/text/0000-remove-ember-fetch.md @@ -0,0 +1,141 @@ +--- +stage: accepted +start-date: 2025-01-10T00:00:00.000Z +release-date: +release-versions: +teams: # delete teams that aren't relevant + - cli + - data + - framework + - learning +prs: + accepted: # update this to the PR that you propose your RFC in +project-link: +--- + + + +<-- Replace "RFC title" with the title of your RFC --> +# Deprecate and Remove ember-fetch + +## Summary + +This RFC proposes removing `ember-fetch` from the blueprint for new projects, and recommends alternative, more native, ways of having "managed fetch". + +## Motivation + +the package, `ember-fetch`, does a fair bit of deceptive and incorrect behavior that is incompatible with modern JavaScript tooling, such as _being_ `ember-fetch`, yet only importing from `fetch`. + +## Transition Path + +- Remove ember-fetch from all blueprints +- Deprecate the npm package and archieve the github repo. +- Migrate to an alternative for "managed fetch" + +### What does `ember-fetch` do? + +_primarily_, it wraps the native `fetch` in `waitForPromise` from `@ember/test-waiters` (aka "settled state integration"). + + +secondarily, but not popularly used, are a series of utilities (e.g.: for checking kinds of errors). These could be copied into projects that use them. + +### Using native `fetch` + +If you only use `ember-fetch` in route model-hooks, then the settled-state integration isn't used (or rather, you rely on the routing's settled-state integration, and `ember-fetch`'s settled-state integration is extraneous) + + +### Direct replacement + + +To replace `ember-fetch`'s core-functionality using the least amount of effort involves adding a new utility in your apps: + +```ts +import { waitForPromise } from '@ember/test-waiters'; + +const nativeFetch = globalThis.fetch; + +export async function wrappedFetch(...args: Parameters) { + let responsePromise = nativeFetch(...args); + + waitForPromise(responsePromise); + + let response = await responsePromise; + + return new Proxy(response, { + get(target, prop, receiver) { + let original = Reflect.get(target, prop, receiver); + + if (['json', 'text'].includes(prop)) { + return (...args) => { + let parsePromise = original(...args); + + return waitForPromise(parsePromise); + } + } + + return original; + } + }); +} +``` + +And then throughout your project, you could find and replace all imports of `import fetch from 'ember';` with `import { wrappedFetch } from 'app-name/utils/wrapped-fetch';` + + +### Using a service + +Potentially an easier-to-manage implementation uses a service such as the following: + +```ts +import Service from '@ember/service'; +import { waitFor } from '@ember/test-waiters'; + +export default class Fetch extends Service { + + @waitFor + @action + requestJson(...args: Parameters) { + return fetch(...args).then(response => response.json()); + } + + @waitFor + @action + requestText(...args: Parameters) { + return fetch(...args).then(response => response.text()); + } +} +``` + +### Using warp-drive / ember-data + +Docs available on https://github.com/emberjs/data/ + +## How We Teach This + +- Add a deprecation notice to the ember-fetch README +- Archive the ember-fetch repo +- Remove ember-fetch from the blueprints +- Remove ember-fetch from the guides (only one reference per version) + +## Drawbacks + +- if we keep ember-fetch is not compatible with modern tooling and modern SSR approaches + +## Alternatives + +- n/a + +## Unresolved questions + +none From 53092fa9603d5cea14f8bc710bd7f622bcc9e187 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Fri, 10 Jan 2025 20:40:24 -0500 Subject: [PATCH 2/7] Rename file --- text/{0000-remove-ember-fetch.md => 1065-remove-ember-fetch.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename text/{0000-remove-ember-fetch.md => 1065-remove-ember-fetch.md} (98%) diff --git a/text/0000-remove-ember-fetch.md b/text/1065-remove-ember-fetch.md similarity index 98% rename from text/0000-remove-ember-fetch.md rename to text/1065-remove-ember-fetch.md index 7cdf9907a2..e938b7fb75 100644 --- a/text/0000-remove-ember-fetch.md +++ b/text/1065-remove-ember-fetch.md @@ -9,7 +9,7 @@ teams: # delete teams that aren't relevant - framework - learning prs: - accepted: # update this to the PR that you propose your RFC in + accepted: https://github.com/emberjs/rfcs/pull/1065 project-link: --- From 34239a4382bd5dffd2d22040877a321441dabae8 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Fri, 10 Jan 2025 20:42:42 -0500 Subject: [PATCH 3/7] Updates --- text/1065-remove-ember-fetch.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/1065-remove-ember-fetch.md b/text/1065-remove-ember-fetch.md index e938b7fb75..3ba340175e 100644 --- a/text/1065-remove-ember-fetch.md +++ b/text/1065-remove-ember-fetch.md @@ -26,7 +26,6 @@ prs: project-link: Leave as is --> -<-- Replace "RFC title" with the title of your RFC --> # Deprecate and Remove ember-fetch ## Summary @@ -48,7 +47,7 @@ the package, `ember-fetch`, does a fair bit of deceptive and incorrect behavior _primarily_, it wraps the native `fetch` in `waitForPromise` from `@ember/test-waiters` (aka "settled state integration"). -secondarily, but not popularly used, are a series of utilities (e.g.: for checking kinds of errors). These could be copied into projects that use them. +secondarily, but not popularly used, are a series of utilities (e.g.: for checking kinds of errors). These could be copied into projects that use them and modified to fit each project's needs. ### Using native `fetch` @@ -93,6 +92,7 @@ export async function wrappedFetch(...args: Parameters) { And then throughout your project, you could find and replace all imports of `import fetch from 'ember';` with `import { wrappedFetch } from 'app-name/utils/wrapped-fetch';` + ### Using a service Potentially an easier-to-manage implementation uses a service such as the following: From 4f138423351f4b106112ca069eacf06d4ca514f0 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Sat, 18 Jan 2025 11:35:50 -0500 Subject: [PATCH 4/7] Update the untested implementation --- text/1065-remove-ember-fetch.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/text/1065-remove-ember-fetch.md b/text/1065-remove-ember-fetch.md index 3ba340175e..66373e8033 100644 --- a/text/1065-remove-ember-fetch.md +++ b/text/1065-remove-ember-fetch.md @@ -62,10 +62,8 @@ To replace `ember-fetch`'s core-functionality using the least amount of effort i ```ts import { waitForPromise } from '@ember/test-waiters'; -const nativeFetch = globalThis.fetch; - export async function wrappedFetch(...args: Parameters) { - let responsePromise = nativeFetch(...args); + let responsePromise = fetch(...args); waitForPromise(responsePromise); @@ -75,7 +73,7 @@ export async function wrappedFetch(...args: Parameters) { get(target, prop, receiver) { let original = Reflect.get(target, prop, receiver); - if (['json', 'text'].includes(prop)) { + if (['json', 'text', 'arrayBuffer', 'blob', 'formData'].includes(prop)) { return (...args) => { let parsePromise = original(...args); From 9027c12cf256a0d73d3b01a8a83da53ba8509187 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Thu, 13 Feb 2025 16:48:12 -0500 Subject: [PATCH 5/7] Update to suggest new util in test-waiters --- text/1065-remove-ember-fetch.md | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/text/1065-remove-ember-fetch.md b/text/1065-remove-ember-fetch.md index 66373e8033..8aca59daad 100644 --- a/text/1065-remove-ember-fetch.md +++ b/text/1065-remove-ember-fetch.md @@ -57,17 +57,16 @@ If you only use `ember-fetch` in route model-hooks, then the settled-state integ ### Direct replacement -To replace `ember-fetch`'s core-functionality using the least amount of effort involves adding a new utility in your apps: +To replace `ember-fetch`'s core-functionality using the least amount of effort involves adding a new utility in `@ember/test-waiters`, `waitForFetch`: ```ts -import { waitForPromise } from '@ember/test-waiters'; +// in @ember/test-waiters +import { waitForPromise } from './wait-for-promise'; -export async function wrappedFetch(...args: Parameters) { - let responsePromise = fetch(...args); +export async function waitForFetch(fetchPromise: Promise) { + waitForPromise(fetchPromise); - waitForPromise(responsePromise); - - let response = await responsePromise; + let response = await fetchPromise; return new Proxy(response, { get(target, prop, receiver) { @@ -87,6 +86,18 @@ export async function wrappedFetch(...args: Parameters) { } ``` + +To have a single import mirroring the behavior of `ember-fetch`, _in full_, folks can still provide a single export that does: + +```ts +import { waitForFetch } from '@ember/test-waiters'; + +export function wrappedFetch(...args: Parameters) { + return wrappedFetch(fetch(...args)); +} +``` + + And then throughout your project, you could find and replace all imports of `import fetch from 'ember';` with `import { wrappedFetch } from 'app-name/utils/wrapped-fetch';` @@ -133,6 +144,7 @@ Docs available on https://github.com/emberjs/data/ ## Alternatives - n/a +- ask folks to wrap and proxy fetch themselves ## Unresolved questions From 33de9a63a9439c80b1a23888bd2dcbdeac5d89ee Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Mon, 17 Feb 2025 11:15:22 -0500 Subject: [PATCH 6/7] Update text/1065-remove-ember-fetch.md Co-authored-by: Alex --- text/1065-remove-ember-fetch.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/1065-remove-ember-fetch.md b/text/1065-remove-ember-fetch.md index 8aca59daad..7a03600387 100644 --- a/text/1065-remove-ember-fetch.md +++ b/text/1065-remove-ember-fetch.md @@ -74,7 +74,7 @@ export async function waitForFetch(fetchPromise: Promise) { if (['json', 'text', 'arrayBuffer', 'blob', 'formData'].includes(prop)) { return (...args) => { - let parsePromise = original(...args); + let parsePromise = original.call(target, ...args); return waitForPromise(parsePromise); } From f63e23ed7fa79f4e46db98a0171b7f76f2f92bf3 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Mon, 17 Feb 2025 13:50:44 -0500 Subject: [PATCH 7/7] Update text/1065-remove-ember-fetch.md Co-authored-by: Josemar Luedke <230476+josemarluedke@users.noreply.github.com> --- text/1065-remove-ember-fetch.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/1065-remove-ember-fetch.md b/text/1065-remove-ember-fetch.md index 7a03600387..5b2034c1c7 100644 --- a/text/1065-remove-ember-fetch.md +++ b/text/1065-remove-ember-fetch.md @@ -93,7 +93,7 @@ To have a single import mirroring the behavior of `ember-fetch`, _in full_, folk import { waitForFetch } from '@ember/test-waiters'; export function wrappedFetch(...args: Parameters) { - return wrappedFetch(fetch(...args)); + return waitForFetch(fetch(...args)); } ```