[CHORE] najax deprecation when ember-fetch is also installed #7230
Conversation
packages/adapter/addon/rest.js
Outdated
| } else if (hasNajax || hasJQuery) { | ||
| return false; | ||
| } else { | ||
| return true; |
There was a problem hiding this comment.
our return value here needs to be set to this._useFetch
There was a problem hiding this comment.
We appear to still not cache off the value for some of the return branches
packages/adapter/addon/rest.js
Outdated
| until: '4.0', | ||
| } | ||
| ); | ||
| } |
There was a problem hiding this comment.
we should add and else that straigh up deprecates najax even when fetch is not present and tells folks to use something like ember-fetch
packages/adapter/addon/rest.js
Outdated
| @@ -5,12 +5,14 @@ | |||
| */ | |||
|
|
|||
| import { getOwner } from '@ember/application'; | |||
| import { deprecate } from '@ember/application/deprecations'; | |||
There was a problem hiding this comment.
Should import from @ember/debug
packages/adapter/addon/rest.js
Outdated
| deprecate( | ||
| 'You have ember-fetch and jquery installed. To use ember-fetch, set `useFetch: true` in your adapter. In 4.0, ember-data will fallback to ember-fetch instead of najax when both ember-fetch and jquery are installed in FastBoot.', | ||
| 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 as a fetch polyfill in FastBoot.', | ||
| false, | ||
| { | ||
| id: 'ember-data:najax-fallback', | ||
| until: '4.0', | ||
| } | ||
| ); |
There was a problem hiding this comment.
Hmmm, maybe we should move this deprecation to happen in the useFetch computed and funnel everything through that still? The thing I'm concerned with is a massive flood of deprecations to the console that are all exactly the same. AFAICT, we are going to trigger this deprecation every AJAX request we need to make...
snewcomer
force-pushed
the
sn/najax-deprecation
branch
from
87ea347
to
b8fd8a6
Compare
packages/adapter/addon/rest.js
Outdated
| if (this._useFetch !== undefined) { | ||
| return this._useFetch; | ||
| } |
packages/adapter/addon/rest.js
Outdated
| } 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.', |
There was a problem hiding this comment.
Can we tweak this to be "native class" first (e.g. instead of useFetch: true say useFetch = true)?
packages/adapter/addon/rest.js
Outdated
| ); | ||
| } 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.', |
packages/adapter/addon/rest.js
Outdated
| } else { | ||
| return true; | ||
| } | ||
| useFetch: computed({ |
There was a problem hiding this comment.
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...
otherwise, if the consuming app has compatWith > 3.22, any najax related code will be stripped
[CHORE] najax deprecation when ember-fetch is also installed
close #7219
This PR seeks to add a deprecation notice for najax (in fastboot) if installed. We want to encourage users to install
ember-fetchif they haven't already.useFetch = truewill be the default for ember data's adapters.