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

[CHORE] najax deprecation when ember-fetch is also installed #7230

Merged
merged 10 commits into from
Oct 9, 2020
103 changes: 68 additions & 35 deletions packages/adapter/addon/rest.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@
*/

import { getOwner } from '@ember/application';
import { warn } from '@ember/debug';
import { deprecate, warn } from '@ember/debug';
import { computed, get } from '@ember/object';
import { assign } from '@ember/polyfills';
import { run } from '@ember/runloop';
import { DEBUG } from '@glimmer/env';

import { has } from 'require';
import { Promise } from 'rsvp';

import Adapter, { BuildURLMixin } from '@ember-data/adapter';
Expand All @@ -24,11 +25,11 @@ import AdapterError, {
TimeoutError,
UnauthorizedError,
} from '@ember-data/adapter/error';
import { DEPRECATE_NAJAX } from '@ember-data/private-build-infra/deprecations';

import { determineBodyPromise, fetch, parseResponseHeaders, serializeIntoHash, serializeQueryParams } from './-private';

const hasJQuery = typeof jQuery !== 'undefined';
const hasNajax = typeof najax !== 'undefined';

/**
The REST adapter allows your store to communicate with an HTTP server by
Expand Down Expand Up @@ -290,7 +291,7 @@ const hasNajax = typeof najax !== 'undefined';
@extends Adapter
@uses BuildURLMixin
*/
const RESTAdapter = Adapter.extend(BuildURLMixin, {
let RESTAdapter = Adapter.extend(BuildURLMixin, {
defaultSerializer: '-rest',

_defaultContentType: 'application/json; charset=utf-8',
Expand All @@ -309,18 +310,47 @@ const RESTAdapter = Adapter.extend(BuildURLMixin, {
},
}),

useFetch: computed(function() {
let ENV = getOwner(this).resolveRegistration('config:environment');
// TODO: https://github.com/emberjs/data/issues/6093
let jQueryIntegrationDisabled = ENV && ENV.EmberENV && ENV.EmberENV._JQUERY_INTEGRATION === false;

if (jQueryIntegrationDisabled) {
return true;
} else if (hasNajax || hasJQuery) {
return false;
} else {
return true;
}
useFetch: computed({
Copy link
Member

Choose a reason for hiding this comment

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

Lets move this down into the if (DEPRECATE_NAJAX) { conditional, and here (for all users) we'd say useFetch: true.

That way users that opt-in to deprecation stripping of DEPRECATE_NAJAX deprecation can reduce the overall payload, and we avoid the work all together...

get() {
if (this._useFetch !== undefined) {
return this._useFetch;
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we use a symbol here?

let ENV = getOwner(this).resolveRegistration('config:environment');
// TODO: https://github.com/emberjs/data/issues/6093
let jQueryIntegrationDisabled = ENV && ENV.EmberENV && ENV.EmberENV._JQUERY_INTEGRATION === false;

if (jQueryIntegrationDisabled) {
return (this._useFetch = true);
} else if (DEPRECATE_NAJAX && typeof najax !== 'undefined') {
if (has('fetch')) {
deprecate(
'You have ember-fetch and jquery installed. To use ember-fetch instead of najax, set `useFetch: true` in your adapter. In 4.0, ember-data will default to ember-fetch instead of najax when both ember-fetch and jquery are installed in FastBoot.',
Copy link
Member

Choose a reason for hiding this comment

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

Can we tweak this to be "native class" first (e.g. instead of useFetch: true say useFetch = true)?

false,
{
id: 'ember-data:najax-fallback',
until: '4.0',
}
);
} else {
deprecate(
'In 4.0, ember-data will default to ember-fetch instead of najax in FastBoot. It is recommended that you install ember-fetch or similar fetch polyfill in FastBoot.',
Copy link
Member

Choose a reason for hiding this comment

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

Same here...

false,
{
id: 'ember-data:najax-fallback',
until: '4.0',
}
);
}
return (this._useFetch = false);
} else if (hasJQuery) {
return (this._useFetch = false);
} else {
return (this._useFetch = true);
}
},
set(key, value) {
return (this._useFetch = value);
},
}),

/**
Expand Down Expand Up @@ -991,15 +1021,14 @@ const RESTAdapter = Adapter.extend(BuildURLMixin, {
*/
ajax(url, type, options) {
let adapter = this;
let useFetch = get(this, 'useFetch');

let requestData = {
url: url,
method: type,
};
let hash = adapter.ajaxOptions(url, type, options);

if (useFetch) {
if (this.useFetch) {
let _response;
return this._fetchRequest(hash)
.then(response => {
Expand Down Expand Up @@ -1039,21 +1068,6 @@ const RESTAdapter = Adapter.extend(BuildURLMixin, {
jQuery.ajax(options);
},

/**
@method _najaxRequest
@private
@param {Object} options jQuery ajax options to be used for the najax request
*/
_najaxRequest(options) {
if (hasNajax) {
najax(options);
} else {
throw new Error(
'najax does not seem to be defined in your app. Did you override it via `addOrOverrideSandboxGlobals` in the fastboot server?'
);
}
},

_fetchRequest(options) {
let fetchFunction = fetch();

Expand All @@ -1067,9 +1081,9 @@ const RESTAdapter = Adapter.extend(BuildURLMixin, {
},

_ajax(options) {
if (get(this, 'useFetch')) {
if (this.useFetch) {
this._fetchRequest(options);
} else if (get(this, 'fastboot.isFastBoot')) {
} else if (DEPRECATE_NAJAX && get(this, 'fastboot.isFastBoot')) {
this._najaxRequest(options);
} else {
this._ajaxRequest(options);
Expand Down Expand Up @@ -1103,7 +1117,7 @@ const RESTAdapter = Adapter.extend(BuildURLMixin, {

let contentType = options.contentType || this._defaultContentType;

if (get(this, 'useFetch')) {
if (this.useFetch) {
if (options.data && options.type !== 'GET') {
if (!options.headers['Content-Type'] && !options.headers['content-type']) {
options.headers['content-type'] = contentType;
Expand Down Expand Up @@ -1414,4 +1428,23 @@ function ajaxOptions(options, adapter) {
return options;
}

if (DEPRECATE_NAJAX) {
RESTAdapter = RESTAdapter.extend({
/**
@method _najaxRequest
@private
@param {Object} options jQuery ajax options to be used for the najax request
*/
_najaxRequest(options) {
if (typeof najax !== 'undefined') {
najax(options);
} else {
throw new Error(
'najax does not seem to be defined in your app. Did you override it via `addOrOverrideSandboxGlobals` in the fastboot server?'
);
}
},
});
}

runspired marked this conversation as resolved.
Show resolved Hide resolved
export default RESTAdapter;
3 changes: 2 additions & 1 deletion packages/private-build-infra/addon/current-deprecations.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* ## Deprecations
*
* EmberData allows users to remove code that exists to support deprecated
* EmberData allows users to opt-in and remove code that exists to support deprecated
* behaviors.
*
* If your app has resolved all deprecations present in a given version,
Expand Down Expand Up @@ -51,4 +51,5 @@ export default {
DEPRECATE_SERIALIZER_QUERY_RECORD_ARRAY_RESPONSE: '3.4',
DEPRECATE_BELONGS_TO_REFERENCE_PUSH: '3.16',
DEPRECATE_REFERENCE_INTERNAL_MODEL: '3.19',
DEPRECATE_NAJAX: '3.22',
};
1 change: 1 addition & 0 deletions packages/private-build-infra/addon/deprecations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@ export const DEPRECATE_MISMATCHED_INVERSE_RELATIONSHIP_DATA = deprecationState(
);
export const DEPRECATE_BELONGS_TO_REFERENCE_PUSH = deprecationState('DEPRECATE_BELONGS_TO_REFERENCE_PUSH');
export const DEPRECATE_REFERENCE_INTERNAL_MODEL = deprecationState('DEPRECATE_REFERENCE_INTERNAL_MODEL');
export const DEPRECATE_NAJAX = deprecationState('DEPRECATE_NAJAX');