-
-
Notifications
You must be signed in to change notification settings - Fork 408
Deprecate ember-fetch #1065
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
Merged
Deprecate ember-fetch #1065
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
bbbcae9
Deprecate ember-fetch
NullVoxPopuli 53092fa
Rename file
NullVoxPopuli 34239a4
Updates
NullVoxPopuli 4f13842
Update the untested implementation
NullVoxPopuli 9027c12
Update to suggest new util in test-waiters
NullVoxPopuli 33de9a6
Update text/1065-remove-ember-fetch.md
NullVoxPopuli f63e23e
Update text/1065-remove-ember-fetch.md
NullVoxPopuli 932d00f
Merge branch 'master' into remove-ember-fetch
ef4 7c7e3c5
Merge branch 'master' into remove-ember-fetch
ef4 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
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,151 @@ | ||
| --- | ||
| 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: https://github.com/emberjs/rfcs/pull/1065 | ||
| project-link: | ||
| --- | ||
|
|
||
| <!--- | ||
| Directions for above: | ||
|
|
||
| stage: Leave as is | ||
| start-date: Fill in with today's date, 2032-12-01T00:00:00.000Z | ||
| release-date: Leave as is | ||
| release-versions: Leave as is | ||
| teams: Include only the [team(s)](README.md#relevant-teams) for which this RFC applies | ||
| prs: | ||
| accepted: Fill this in with the URL for the Proposal RFC PR | ||
| project-link: Leave as is | ||
| --> | ||
|
|
||
| # 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 and modified to fit each project's needs. | ||
|
|
||
| ### 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 `@ember/test-waiters`, `waitForFetch`: | ||
|
|
||
| ```ts | ||
| // in @ember/test-waiters | ||
| import { waitForPromise } from './wait-for-promise'; | ||
|
|
||
| export async function waitForFetch<Value>(fetchPromise: Promise<Value>) { | ||
| waitForPromise(fetchPromise); | ||
|
|
||
| let response = await fetchPromise; | ||
|
|
||
| return new Proxy(response, { | ||
| get(target, prop, receiver) { | ||
| let original = Reflect.get(target, prop, receiver); | ||
|
|
||
| if (['json', 'text', 'arrayBuffer', 'blob', 'formData'].includes(prop)) { | ||
| return (...args) => { | ||
| let parsePromise = original.call(target, ...args); | ||
|
|
||
| return waitForPromise(parsePromise); | ||
| } | ||
| } | ||
|
|
||
| return original; | ||
| } | ||
| }); | ||
| } | ||
| ``` | ||
|
|
||
|
|
||
| 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<typeof fetch>) { | ||
| 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';` | ||
|
|
||
|
|
||
|
|
||
| ### 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<typeof fetch>) { | ||
| return fetch(...args).then(response => response.json()); | ||
| } | ||
|
|
||
| @waitFor | ||
| @action | ||
| requestText(...args: Parameters<typeof fetch>) { | ||
| 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 | ||
| - ask folks to wrap and proxy fetch themselves | ||
|
|
||
| ## Unresolved questions | ||
|
|
||
| none | ||
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.