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

Fetch + fastboot + jquery detection problem #7219

Closed
vlascik opened this issue Jun 7, 2020 · 5 comments · Fixed by #7230
Closed

Fetch + fastboot + jquery detection problem #7219

vlascik opened this issue Jun 7, 2020 · 5 comments · Fixed by #7230

Comments

@vlascik
Copy link

vlascik commented Jun 7, 2020

If an app has ember-cli-fastboot, ember-fetch and @ember/jquery as dependencies, code in

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;
}
doesn't seem to end up with fetch, and uses najax instead. Which leads to errors like najax: method jqXHR."getAllResponseHeaders" not implemented

Ideally, fetch would be used instead, perhaps with additional check like

    if (this.fastboot && hasFetch) {
      return true;
    } else if (jQueryIntegrationDisabled) { 

Seems this method was added in #6094

Not sure if there is a reason for current logic though, or how to proceed.

@rwjblue
Copy link
Member

rwjblue commented Jun 7, 2020

Not sure if there is a reason for current logic though

Since ember-data added support via najax long before fetch support and ember-fetch was already commonly in use when we added support for fetch we were forced to preserve the priority fallback order (otherwise it would have been arguably a breaking change).

@rwjblue
Copy link
Member

rwjblue commented Jun 7, 2020

I think the easiest thing to do would be to override useFetch in your application adapter (to be true).

@vlascik
Copy link
Author

vlascik commented Jun 8, 2020

Yep, that's what I ultimately did, but the current situation is confusing, maybe this should have at least a warning, documentation, or a more permanent solution, like najax deprecation? Don't think that library is used by many anymore.

@snewcomer
Copy link
Contributor

I was looking at this too at some point so thanks for the explanation! Is it worth making an issue to remind us to change for the 4.0 release?

@rwjblue
Copy link
Member

rwjblue commented Jun 8, 2020

I agree with @vlascik, we can (and probably) should issue a deprecation when we detect that we are going to fallback to the najax side of things. Though we'll have to take care to ensure that the deprecation is triggered in both FastBoot and normal browser runtimes (deprecation in FastBoot alone is much less like to actually be seen unless parsing through server logs).

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 a pull request may close this issue.

3 participants