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

Move to new ember-fetch import path #732

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ ember-fetch requries ember-cli 2.13 or above.

```js
import Route from '@ember/routing/route';
import fetch from 'fetch';
import fetch from 'ember-fetch';

export default Route.extend({
model() {
Expand All @@ -34,11 +34,22 @@ export default Route.extend({

Available imports:
```js
import fetch, { Headers, Request, Response, AbortController } from 'ember-fetch';
```

`ember-fetch` still supports importing from the old import path, but this will shortly be deprecated and ***will be removed in the next major***:
```js
import fetch, { Headers, Request, Response, AbortController } from 'fetch';
```

### Use with TypeScript
To use `ember-fetch` with TypeScript or enable editor's type support, You can add `"fetch": ["node_modules/ember-fetch"]` to your `tsconfig.json`.
If you import from `ember-fetch`, the types just work:

```ts
import fetch from 'ember-fetch';
```

If you're still using the legacy `fetch` import path, you need to add `"fetch": ["node_modules/ember-fetch"]` to your `tsconfig.json`.

```json
{
Expand Down Expand Up @@ -126,7 +137,7 @@ otherwise you can read the status code to determine the bad response type.

```js
import Route from '@ember/routing/route';
import fetch from 'fetch';
import fetch from 'ember-fetch';
import {
isAbortError,
isServerErrorResponse,
Expand Down
13 changes: 13 additions & 0 deletions assets/fetch-legacy-alias.js.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocker for this PR, but we should also think about how to deprecate this.

* For backwards compatibility, we need to still support consumers importing from the `fetch` path.
* In the future, we should release a breaking change which removes this alias and requires consumers
* to import from `ember-fetch`. This will ensure there is no confusion around folks thinking they are
* importing from the actual `fetch` package.
*/
define('fetch', ['exports', 'ember-fetch'], function(exports, emberFetch) {
const exportKeys = Object.keys(emberFetch);
for (let i = 0; i < exportKeys.length; i++) {
const key = exportKeys[i];
exports[key] = emberFetch[key];
}
});
2 changes: 1 addition & 1 deletion fastboot/instance-initializers/setup-fetch.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { setupFastboot } from 'fetch';
import { setupFastboot } from 'ember-fetch';

/**
* To allow relative URLs for Fastboot mode, we need the per request information
Expand Down
13 changes: 8 additions & 5 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,15 @@ const Rollup = require('broccoli-rollup');
const BroccoliDebug = require('broccoli-debug');
const calculateCacheKeyForTree = require('calculate-cache-key-for-tree');
const VersionChecker = require('ember-cli-version-checker');
const { readFileSync } = require('fs');

const debug = BroccoliDebug.buildDebugCallback('ember-fetch');

// Path to the template that contains the shim wrapper around the browser polyfill
const TEMPLATE_PATH = path.resolve(__dirname + '/assets/browser-fetch.js.t');

const LEGACY_ALIAS_TEMPLATE = readFileSync(path.resolve(__dirname + '/assets/fetch-legacy-alias.js.t'));

/*
* The `index.js` file is the main entry point for all Ember CLI addons. The
* object we export from this file is turned into an Addon class
Expand Down Expand Up @@ -222,7 +225,7 @@ module.exports = {
sourceMapConfig: { enabled: false }
}), 'after-concat');

const moduleHeader = this._getModuleHeader(options);
const moduleHeader = LEGACY_ALIAS_TEMPLATE + this._getModuleHeader(options);

return debug(
new Template(polyfillNode, TEMPLATE_PATH, function (content) {
Expand All @@ -238,14 +241,14 @@ module.exports = {
_getModuleHeader({ hasEmberSourceModules, nativePromise }) {
if (hasEmberSourceModules && nativePromise) {
return `
define('fetch', ['exports', 'ember'], function(exports, Ember__module) {
define('ember-fetch', ['exports', 'ember'], function(exports, Ember__module) {
'use strict';
var Ember = 'default' in Ember__module ? Ember__module['default'] : Ember__module;`;
}

if (hasEmberSourceModules) {
return `
define('fetch', ['exports', 'ember', 'rsvp'], function(exports, Ember__module, RSVP__module) {
define('ember-fetch', ['exports', 'ember', 'rsvp'], function(exports, Ember__module, RSVP__module) {
'use strict';
var Ember = 'default' in Ember__module ? Ember__module['default'] : Ember__module;
var RSVP = 'default' in RSVP__module ? RSVP__module['default'] : RSVP__module;
Expand All @@ -254,13 +257,13 @@ define('fetch', ['exports', 'ember', 'rsvp'], function(exports, Ember__module, R

if (nativePromise) {
return `
define('fetch', ['exports'], function(exports) {
define('ember-fetch', ['exports'], function(exports) {
'use strict';
var Ember = originalGlobal.Ember;`;
}

return `
define('fetch', ['exports'], function(exports) {
define('ember-fetch', ['exports'], function(exports) {
'use strict';
var Ember = originalGlobal.Ember;
var Promise = Ember.RSVP.Promise;`;
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/dummy/app/routes/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Route from '@ember/routing/route';
import { hash } from 'rsvp';
import fetch, { Request } from 'fetch';
import fetch, { Request } from 'ember-fetch';
import ajax from 'ember-fetch/ajax';

export default Route.extend({
Expand Down
2 changes: 1 addition & 1 deletion tests/acceptance/error-test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { module, test } from 'qunit';
import Pretender from 'pretender';
import fetch, { AbortController } from 'fetch';
import fetch, { AbortController } from 'ember-fetch';
import {
isUnauthorizedResponse,
isForbiddenResponse,
Expand Down
17 changes: 16 additions & 1 deletion tests/acceptance/root-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { module, test } from 'qunit';
import { setupApplicationTest } from 'ember-qunit';
import { visit, click, find, currentRouteName } from '@ember/test-helpers';
import Pretender from 'pretender';
import fetch from 'fetch';
import fetch from 'ember-fetch';

var server;

Expand Down Expand Up @@ -32,6 +32,21 @@ module('Acceptance: Root', function(hooks) {
assert.equal(this.element.querySelector('.fetch').textContent.trim(), 'Hello World! fetch');
});

test('legacy import path still works', async function(assert) {
server.get('/omg.json', function() {
return [
200,
{ 'Content-Type': 'text/json'},
JSON.stringify({ name: 'World' })
];
});

await visit('/legacy-import-path');

assert.equal(currentRouteName(), 'legacy-import-path');
assert.equal(this.element.querySelector('.fetch').textContent.trim(), 'Hello World! fetch');
});

test('posting a string', function(assert) {
server.post('/upload', function(req) {
assert.equal(req.requestBody, 'foo');
Expand Down
2 changes: 1 addition & 1 deletion tests/dummy/app/controllers/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Controller from '@ember/controller';
import { run } from '@ember/runloop';
import fetch from 'fetch';
import fetch from 'ember-fetch';

export default Controller.extend({
actions: {
Expand Down
23 changes: 23 additions & 0 deletions tests/dummy/app/controllers/legacy-import-path.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import Controller from '@ember/controller';
import { run } from '@ember/runloop';
import fetch from 'fetch';

export default Controller.extend({
actions: {
fetchSlowData() {
fetch('/slow-data.json')
.then((r) => r.json(), (e) => {
run(() => this.set('fetchedSlowDataFailed', true));
throw e;
})
.then((data) => {
run(() => this.setProperties({ fetchedSlowData: data, fetchedSlowDataFailed: false }));
});
},

badFetch() {
fetch('http://localhost:9999') // probably there is nothing listening here :D
.then((r) => r.json(), () => run(() => this.set('fetchFailed', true)));
}
}
});
1 change: 1 addition & 0 deletions tests/dummy/app/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const Router = EmberRouter.extend({
});

Router.map(function() {
this.route('legacy-import-path')
});

export default Router;
2 changes: 1 addition & 1 deletion tests/dummy/app/routes/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Route from '@ember/routing/route';
import { hash } from 'rsvp';
import fetch from 'fetch';
import fetch from 'ember-fetch';

export default Route.extend({
model: function() {
Expand Down
13 changes: 13 additions & 0 deletions tests/dummy/app/routes/legacy-import-path.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import Route from '@ember/routing/route';
import { hash } from 'rsvp';
import fetch from 'fetch';

export default Route.extend({
model: function() {
return hash({
fetch: fetch('/omg.json').then(function(request) {
return request.json();
})
});
}
});
13 changes: 13 additions & 0 deletions tests/dummy/app/templates/legacy-import-path.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<div class="fetch">
Hello {{model.fetch.name}}! fetch
</div>

<button onclick={{action 'fetchSlowData'}} id="fetch-slow-data-button">Fetch slow data</button>
{{#if fetchedSlowData}}
<span class="fetched-slow-data-span">{{fetchedSlowData.content}}</span>
{{/if}}

<button onclick={{action 'badFetch'}} id="fetch-broken-data-button">Failing fetch</button>
{{#if fetchFailed}}
<span class="fetched-failed-span">Fetch failed</span>
{{/if}}
17 changes: 16 additions & 1 deletion tests/unit/abortcontroller-test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { module, test } from 'qunit';
import { AbortController } from 'fetch';
import { AbortController } from 'ember-fetch';
import { AbortController as LegacyAbortController } from 'ember-fetch';

module('AbortController', function() {
test('signal', function(assert) {
Expand All @@ -15,4 +16,18 @@ module('AbortController', function() {

controller.abort();
})

test('signal works from legacy import', function(assert) {
assert.expect(1);
let controller = new LegacyAbortController();
let signal = controller.signal;

let done = assert.async();
signal.addEventListener('abort', function() {
assert.ok(true);
done();
});

controller.abort();
})
});
2 changes: 1 addition & 1 deletion tests/unit/error-test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { module, test } from 'qunit';
import { Response } from 'fetch';
import { Response } from 'ember-fetch';

import {
isUnauthorizedResponse,
Expand Down
12 changes: 11 additions & 1 deletion tests/unit/headers-test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { module, test } from 'qunit';
import { Headers } from 'fetch';
import { Headers } from 'ember-etch';
import { Headers as LegacyImportHeaders } from 'fetch';

module('Headers', function() {
test('iterator', function(assert) {
Expand All @@ -10,4 +11,13 @@ module('Headers', function() {
assert.ok(headers.keys()[Symbol.iterator]);
assert.ok(headers.entries()[Symbol.iterator]);
});

test('iterator from legacy import', function(assert) {
let headers = new LegacyImportHeaders();

assert.ok(headers[Symbol.iterator]);
assert.ok(headers.values()[Symbol.iterator]);
assert.ok(headers.keys()[Symbol.iterator]);
assert.ok(headers.entries()[Symbol.iterator]);
});
});
14 changes: 13 additions & 1 deletion tests/unit/utils/determine-body-promise-test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { module, test } from 'qunit';
import { Response } from 'fetch';
import { Response } from 'ember-fetch';
import { Response as LegacyImportResponse } from 'fetch';
import determineBodyPromise from 'ember-fetch/utils/determine-body-promise';

module('Unit | determineBodyPromise', function() {
Expand All @@ -14,6 +15,17 @@ module('Unit | determineBodyPromise', function() {
});
});

test('works when imported from legacy import path', function(assert) {
assert.expect(1);

const response = new LegacyImportResponse('{"data": "foo"}', { status: 200 });
const bodyPromise = determineBodyPromise(response, {});

return bodyPromise.then(body => {
assert.deepEqual(body, { data: 'foo' });
});
});

test('determineBodyResponse returns the body even if it is not json', function(assert) {
assert.expect(1);

Expand Down