Skip to content

Commit

Permalink
[BUGFIX release] Remove ember-fetch dependency. (#6077)
Browse files Browse the repository at this point in the history
This change removes ember-fetch as a direct dependency of ember-data,
and allows for this dependency to be provided by the host application
or for users to directly use native fetch.

Having `ember-fetch` as a direct dependency of ember-data has proven
troublesome. There are issues using ember-fetch as a nested dependency
(these are documented in the ember-fetch README), but it is also quite
difficult for ember-data to ensure that the versions of `ember-fetch`
provided will actually include the correct version (duplicated addon
merging in the ember-cli addon space is complicated and error prone).

This change moves the `determineBodyPromise` and `serializeQueryParams`
helper functions directly into `@ember-data/adapter`. A future version
of ember-fetch will drop these utilities (as well as the ember-data
adapter mixin). In addition, this also enables usage of the global
`fetch` if a `fetch` module is not provided.
  • Loading branch information
rwjblue committed May 1, 2019
1 parent 711ef31 commit 83c5ee0
Show file tree
Hide file tree
Showing 9 changed files with 146 additions and 57 deletions.
4 changes: 4 additions & 0 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ jobs:
yarn test:node
displayName: 'Node Tests'
- script: |
yarn test:try-one with-ember-fetch
displayName: 'Basic Tests with ember-fetch'
- job: Windows_tests
dependsOn: Basic_Ember_Data_tests

Expand Down
8 changes: 8 additions & 0 deletions packages/-ember-data/config/ember-try.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ module.exports = function() {
name: 'default',
npm: {},
},
{
name: 'with-ember-fetch',
npm: {
devDependencies: {
'ember-fetch': '^6.5.1',
},
},
},
{
name: 'ember-lts-3.4',
env: {
Expand Down
1 change: 0 additions & 1 deletion packages/-ember-data/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
"ember-cli-test-info": "^1.0.0",
"ember-cli-typescript": "^2.0.1",
"ember-cli-version-checker": "^3.1.2",
"ember-fetch": "^6.5.0",
"ember-inflector": "^3.0.0",
"inflection": "^1.12.0",
"resolve": "^1.8.1",
Expand Down
3 changes: 3 additions & 0 deletions packages/adapter/addon/-private/index.js
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
export { default as parseResponseHeaders } from './utils/parse-response-headers';
export { determineBodyPromise } from './utils/determine-body-promise';
export { serializeQueryParams } from './utils/serialize-query-params';
export { default as fetch } from './utils/fetch';
30 changes: 30 additions & 0 deletions packages/adapter/addon/-private/utils/determine-body-promise.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/**
* Function that always attempts to parse the response as json, and if an error is thrown,
* returns `undefined` if the response is successful and has a status code of 204 (No Content),
* or 205 (Reset Content) or if the request method was 'HEAD', and the plain payload otherwise.
*/
export function determineBodyPromise(
response: Response,
requestData: JQueryAjaxSettings
): Promise<object | string | undefined> {
return response.text().then(function(payload) {
let ret: string | object | undefined = payload;
try {
ret = JSON.parse(payload);
} catch (error) {
if (!(error instanceof SyntaxError)) {
throw error;
}
const status = response.status;
if (
response.ok &&
(status === 204 || status === 205 || requestData.method === 'HEAD')
) {
ret = undefined;
} else {
console.warn('This response was unable to be parsed as json.', payload);
}
}
return ret;
});
}
17 changes: 17 additions & 0 deletions packages/adapter/addon/-private/utils/fetch.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import require, { has } from 'require';

type MaybeFetch = {
(input: RequestInfo, init?: RequestInit | undefined): Promise<Response>;
} | null;

let _fetch: MaybeFetch = null;

if (has('fetch')) {
// use `fetch` module by default, this is commonly provided by ember-fetch
_fetch = require('fetch').default;
} else if (typeof fetch === 'function') {
// fallback to using global fetch
_fetch = fetch;
}

export default _fetch;
69 changes: 69 additions & 0 deletions packages/adapter/addon/-private/utils/serialize-query-params.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
const RBRACKET = /\[\]$/;

function isPlainObject(obj: any): boolean {
return Object.prototype.toString.call(obj) === '[object Object]';
}

/**
* Helper function that turns the data/body of a request into a query param string.
* This is directly copied from jQuery.param.
*/
export function serializeQueryParams(
queryParamsObject: object | string
): string {
var s: any[] = [];

function buildParams(prefix: string, obj: any) {
var i, len, key;

if (prefix) {
if (Array.isArray(obj)) {
for (i = 0, len = obj.length; i < len; i++) {
if (RBRACKET.test(prefix)) {
add(s, prefix, obj[i]);
} else {
buildParams(
prefix + '[' + (typeof obj[i] === 'object' ? i : '') + ']',
obj[i]
);
}
}
} else if (isPlainObject(obj)) {
for (key in obj) {
buildParams(prefix + '[' + key + ']', obj[key]);
}
} else {
add(s, prefix, obj);
}
} else if (Array.isArray(obj)) {
for (i = 0, len = obj.length; i < len; i++) {
add(s, obj[i].name, obj[i].value);
}
} else {
for (key in obj) {
buildParams(key, obj[key]);
}
}
return s;
}

return buildParams('', queryParamsObject)
.join('&')
.replace(/%20/g, '+');
}

/**
* Part of the `serializeQueryParams` helper function.
*/
function add(s: Array<any>, k: string, v?: string | (() => string)) {
// Strip out keys with undefined value and replace null values with
// empty strings (mimics jQuery.ajax)
if (v === undefined) {
return;
} else if (v === null) {
v = '';
}

v = typeof v === 'function' ? v() : v;
s[s.length] = `${encodeURIComponent(k)}=${encodeURIComponent(v)}`;
}
15 changes: 9 additions & 6 deletions packages/adapter/addon/rest.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,18 @@
@module ember-data
*/

import fetch from 'fetch';
import serializeQueryParams from 'ember-fetch/utils/serialize-query-params';
import determineBodyPromise from 'ember-fetch/utils/determine-body-promise';

import RSVP, { Promise as EmberPromise } from 'rsvp';
import { get, computed } from '@ember/object';
import { getOwner } from '@ember/application';
import { run } from '@ember/runloop';
import Adapter, { BuildURLMixin } from '@ember-data/adapter';
import { assign } from '@ember/polyfills';
import { parseResponseHeaders } from '@ember-data/adapter/-private';
import {
determineBodyPromise,
fetch,
parseResponseHeaders,
serializeQueryParams,
} from './-private';
import {
AdapterError,
InvalidError,
Expand Down Expand Up @@ -301,7 +302,9 @@ const RESTAdapter = Adapter.extend(BuildURLMixin, {

useFetch: computed(function() {
let ENV = getOwner(this).resolveRegistration('config:environment');
return (ENV && ENV._JQUERY_INTEGRATION) === false || jQ === undefined;
let shouldUseFetch = (ENV && ENV._JQUERY_INTEGRATION) === false || jQ === undefined;

return shouldUseFetch;
}),

/**
Expand Down
56 changes: 6 additions & 50 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1679,7 +1679,7 @@ abbrev@1:
resolved "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz#f8f2c887ad10bf67f634f005b6987fed3179aac8"
integrity sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==

abortcontroller-polyfill@^1.1.9, abortcontroller-polyfill@^1.3.0:
abortcontroller-polyfill@^1.1.9:
version "1.3.0"
resolved "https://registry.npmjs.org/abortcontroller-polyfill/-/abortcontroller-polyfill-1.3.0.tgz#de69af32ae926c210b7efbcc29bf644ee4838b00"
integrity sha512-lbWQgf+eRvku3va8poBlDBO12FigTQr9Zb7NIjXrePrhxWVKdCP2wbDl1tLDaYa18PWTom3UEWwdH13S46I+yA==
Expand Down Expand Up @@ -2956,7 +2956,7 @@ broccoli-clean-css@^1.1.0:
inline-source-map-comment "^1.0.5"
json-stable-stringify "^1.0.0"

broccoli-concat@*, broccoli-concat@^3.2.2, broccoli-concat@^3.7.3:
broccoli-concat@*, broccoli-concat@^3.7.3:
version "3.7.3"
resolved "https://registry.npmjs.org/broccoli-concat/-/broccoli-concat-3.7.3.tgz#0dca01311567ffb13180e6b4eb111824628e4885"
integrity sha512-2Ma9h81EJ0PRb9n4sW0i8KZlcnpTQfKxcj87zvi5DFe1fd8CTDEdseHDotK2beuA2l+LbgVPfd8EHaBJKm/Y8g==
Expand Down Expand Up @@ -3254,7 +3254,7 @@ broccoli-sri-hash@^2.1.0:
sri-toolbox "^0.2.0"
symlink-or-copy "^1.0.1"

broccoli-stew@*, broccoli-stew@^2.0.1, broccoli-stew@^2.1.0:
broccoli-stew@*, broccoli-stew@^2.0.1:
version "2.1.0"
resolved "https://registry.npmjs.org/broccoli-stew/-/broccoli-stew-2.1.0.tgz#ba73add17fda3b9b01d8cfb343a8b613b7136a0a"
integrity sha512-tgCkuTWYl4uf7k7ib2D79KFEj2hCgnTUNPMnrCoAha0/4bywcNccmaZVWtL9Ex37yX5h5eAbnM/ak2ULoMwSSw==
Expand Down Expand Up @@ -3282,17 +3282,6 @@ broccoli-string-replace@*, broccoli-string-replace@^0.1.2:
broccoli-persistent-filter "^1.1.5"
minimatch "^3.0.3"

broccoli-templater@^2.0.1:
version "2.0.2"
resolved "https://registry.npmjs.org/broccoli-templater/-/broccoli-templater-2.0.2.tgz#285a892071c0b3ad5ebc275d9e8b3465e2d120d6"
integrity sha512-71KpNkc7WmbEokTQpGcbGzZjUIY1NSVa3GB++KFKAfx5SZPUozCOsBlSTwxcv8TLoCAqbBnsX5AQPgg6vJ2l9g==
dependencies:
broccoli-plugin "^1.3.1"
fs-tree-diff "^0.5.9"
lodash.template "^4.4.0"
rimraf "^2.6.2"
walk-sync "^0.3.3"

broccoli-test-helper@*, broccoli-test-helper@^2.0.0:
version "2.0.0"
resolved "https://registry.npmjs.org/broccoli-test-helper/-/broccoli-test-helper-2.0.0.tgz#1cfbb76f7e856ad8df96d55ee2f5e0dddddf5d4f"
Expand Down Expand Up @@ -3377,7 +3366,7 @@ browserslist@^3.2.6:
caniuse-lite "^1.0.30000844"
electron-to-chromium "^1.3.47"

browserslist@^4.0.0, browserslist@^4.5.2, browserslist@^4.5.4:
browserslist@^4.5.2, browserslist@^4.5.4:
version "4.5.5"
resolved "https://registry.npmjs.org/browserslist/-/browserslist-4.5.5.tgz#fe1a352330d2490d5735574c149a85bc18ef9b82"
integrity sha512-0QFO1r/2c792Ohkit5XI8Cm8pDtZxgNl2H6HU4mHrpYz7314pEYcsAVVatM0l/YmxPnEzh9VygXouj4gkFUTKA==
Expand Down Expand Up @@ -3581,17 +3570,7 @@ can-symlink@^1.0.0:
dependencies:
tmp "0.0.28"

caniuse-api@^3.0.0:
version "3.0.0"
resolved "https://registry.npmjs.org/caniuse-api/-/caniuse-api-3.0.0.tgz#5e4d90e2274961d46291997df599e3ed008ee4c0"
integrity sha512-bsTwuIg/BZZK/vreVTYYbSWoe2F+71P7K5QGEX+pT250DZbfU1MQ5prOKpPR+LL6uWKK3KMwMCAS74QB3Um1uw==
dependencies:
browserslist "^4.0.0"
caniuse-lite "^1.0.0"
lodash.memoize "^4.1.2"
lodash.uniq "^4.5.0"

caniuse-lite@^1.0.0, caniuse-lite@^1.0.30000844, caniuse-lite@^1.0.30000960:
caniuse-lite@^1.0.30000844, caniuse-lite@^1.0.30000960:
version "1.0.30000963"
resolved "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30000963.tgz#5be481d5292f22aff5ee0db4a6c049b65b5798b1"
integrity sha512-n4HUiullc7Lw0LyzpeLa2ffP8KxFBGdxqD/8G3bSL6oB758hZ2UE2CVK+tQN958tJIi0/tfpjAc67aAtoHgnrQ==
Expand Down Expand Up @@ -4598,7 +4577,7 @@ ember-cli-babel-plugin-helpers@^1.0.0, ember-cli-babel-plugin-helpers@^1.1.0:
resolved "https://registry.npmjs.org/ember-cli-babel-plugin-helpers/-/ember-cli-babel-plugin-helpers-1.1.0.tgz#de3baedd093163b6c2461f95964888c1676325ac"
integrity sha512-Zr4my8Xn+CzO0gIuFNXji0eTRml5AxZUTDQz/wsNJ5AJAtyFWCY4QtKdoELNNbiCVGt1lq5yLiwTm4scGKu6xA==

ember-cli-babel@^6.0.0-beta.4, ember-cli-babel@^6.0.0-beta.7, ember-cli-babel@^6.12.0, ember-cli-babel@^6.16.0, ember-cli-babel@^6.6.0, ember-cli-babel@^6.8.1, ember-cli-babel@^6.8.2, ember-cli-babel@^6.9.0:
ember-cli-babel@^6.0.0-beta.4, ember-cli-babel@^6.0.0-beta.7, ember-cli-babel@^6.12.0, ember-cli-babel@^6.16.0, ember-cli-babel@^6.6.0, ember-cli-babel@^6.8.1, ember-cli-babel@^6.9.0:
version "6.18.0"
resolved "https://registry.npmjs.org/ember-cli-babel/-/ember-cli-babel-6.18.0.tgz#3f6435fd275172edeff2b634ee7b29ce74318957"
integrity sha512-7ceC8joNYxY2wES16iIBlbPSxwKDBhYwC8drU3ZEvuPDMwVv1KzxCNu1fvxyFEBWhwaRNTUxSCsEVoTd9nosGA==
Expand Down Expand Up @@ -5029,24 +5008,6 @@ ember-export-application-global@*, ember-export-application-global@^2.0.0:
dependencies:
ember-cli-babel "^6.0.0-beta.7"

ember-fetch@^6.5.0:
version "6.5.1"
resolved "https://registry.npmjs.org/ember-fetch/-/ember-fetch-6.5.1.tgz#6512153b9b85042744ed5a7934c8edb4a5b02dd0"
integrity sha512-ZA90lbTFZHhQCQKQNA6/3TdtiF1wBoT8nC7EI1pmkUZ49XDBP7i2eddUodR6qWRn8jK/D7IavQy3tx5fJBx7mw==
dependencies:
abortcontroller-polyfill "^1.3.0"
broccoli-concat "^3.2.2"
broccoli-debug "^0.6.5"
broccoli-merge-trees "^3.0.0"
broccoli-rollup "^2.1.1"
broccoli-stew "^2.1.0"
broccoli-templater "^2.0.1"
calculate-cache-key-for-tree "^2.0.0"
caniuse-api "^3.0.0"
ember-cli-babel "^6.8.2"
node-fetch "^2.3.0"
whatwg-fetch "^3.0.0"

ember-inflector@^3.0.0:
version "3.0.0"
resolved "https://registry.npmjs.org/ember-inflector/-/ember-inflector-3.0.0.tgz#7e1ee8aaa0fa773ba0905d8b7c0786354d890ee1"
Expand Down Expand Up @@ -7903,11 +7864,6 @@ lodash.keys@~2.3.0:
lodash._shimkeys "~2.3.0"
lodash.isobject "~2.3.0"

lodash.memoize@^4.1.2:
version "4.1.2"
resolved "https://registry.npmjs.org/lodash.memoize/-/lodash.memoize-4.1.2.tgz#bcc6c49a42a2840ed997f323eada5ecd182e0bfe"
integrity sha1-vMbEmkKihA7Zl/Mj6tpezRguC/4=

lodash.merge@^4.3.1, lodash.merge@^4.6.0:
version "4.6.1"
resolved "https://registry.npmjs.org/lodash.merge/-/lodash.merge-4.6.1.tgz#adc25d9cb99b9391c59624f379fbba60d7111d54"
Expand Down

0 comments on commit 83c5ee0

Please sign in to comment.