Skip to content
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

Rewrite ember-fetch to a v2 addon #740

Closed
wants to merge 12 commits into from

Conversation

StephanH90
Copy link

@StephanH90 StephanH90 commented Oct 5, 2023

Description

The goal is to make ember-fetch a v2 addon, removing all the old fetch polyfill stuff while maintaining backwards compatibility. This MR is the first step in this process.

This MR basically:

  • removes all the broccoli magic stuff
  • uses native fetch
  • exposes a setupFetchWaiter function which provides backwards compatibility in tests
  • continues to export a fetch from ember-fetch for backwards compatibility (its just the native fetch)

Breaking

The only change that has to be made, if somebody wants to upgrade to v9 is that they have to import a setupFetchWaiter function which wraps the native fetch with an waitForPromise. This means that fetch will continue to work in tests as it has done in the past. Using the setupFetchWaiter would look something like this:

import { setupFetchWaiter } from 'ember-fetch';

setupFetchWaiter();

In development and production builds this does nothing.

Next steps

In the next steps various other addons will have to be pulled into test apps to make sure that they continue to work with this version of fetch.

@NullVoxPopuli
Copy link
Contributor

Looks like nohe modules got committed 🙃

@StephanH90 StephanH90 changed the base branch from master to next October 5, 2023 12:13
README.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll want to keep README.md and RELEASE.md, .release-it.json, and the CHANGELOG.md should be move in to the ember-fetch directory.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we probably want to keep this as well


export {
setupFetchWaiter,
fetch, // fetch is re-exported here for backwards compatbiility
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fetch will be undefined unless isTesting, which isn't quite right.
We'll want it to be globalThis.fetch

@StephanH90 StephanH90 changed the title Remove basically everything Rewrite ember-fetch to a v2 addon (always use native fetch, preserve backwards compatibility) Oct 10, 2023
@StephanH90 StephanH90 changed the title Rewrite ember-fetch to a v2 addon (always use native fetch, preserve backwards compatibility) Rewrite ember-fetch to a v2 addon Oct 10, 2023
@StephanH90 StephanH90 marked this pull request as ready for review October 10, 2023 06:18
@NullVoxPopuli NullVoxPopuli deleted the branch ember-cli:next December 4, 2023 21:57
@NullVoxPopuli
Copy link
Contributor

Can you re-target main/master? I closed the next branch, and made a v8 for bugfixes/backports

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants