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

[QUEST] Making jQuery Optional #5320

Closed
6 tasks done
wycats opened this issue Jan 9, 2018 · 13 comments
Closed
6 tasks done

[QUEST] Making jQuery Optional #5320

wycats opened this issue Jan 9, 2018 · 13 comments

Comments

@wycats
Copy link
Member

wycats commented Jan 9, 2018

This issue tracks emberjs/ember.js#16058 for Ember Data.

Currently, ember.js is in the progress of making jQuery optional. This does not deprecate jQuery in Ember, but it makes using Ember without jQuery a first-class option. Today, it's possible, but the Ember tests don't run without jQuery, and users need to keep track themselves of which APIs might inadvertently rely on jQuery.

This issue tracks making jQuery optional in the Ember Data component of Ember.

As a general principle, there are a few different kinds of reliance on jQuery:

  • Actually using jQuery internally. For internal-only use, we should switch to a standard API (like fetch() in Ember Data).
  • Reliance on details of jQuery as a result of the fact that we're using jQuery internally. For example, because Ember Data uses jQuery.ajax internally, people can use ajaxSend and other public jQuery APIs to configure the fetch. In this case, the strategy we're using is to keep around the jQuery code, but allow users to opt into a more standard mode explicitly. In general, we would like to try to detect when users are attempting to use jQuery in development mode and give them good assertions.

So in short, we need to remove the use of jQuery in purely internal places, and create a mode for using Ember Data without jQuery on an opt-in basis in places where jQuery is leaked into the public API.

To get started, I'd recommend using an internal flag like jQueryDisabled that you can use as a conditional to opt into the no-jquery semantics. Ember Core will soon create a flag you can use explicitly (we need an RFC) that you can swap to as soon as we have it. But there's no reason to wait.

@tchak would you mind commenting with concrete steps we need here, and I'll add them to the issue?


Work to be done:

  • Remove jQuery usage from tests
  • Remove use of $.parseJSON
  • Figure out how to feature-flag an optional dependency (cc @rwjblue)
    • avoid introducing ember-fetch bloat to people opting out of jQuery
  • Create a new feature flag for using fetch in place of $.ajax
  • Figure out how to avoid introducing unnecessary bloat
@tchak
Copy link
Member

tchak commented Jan 14, 2018

Ember Data jQuery usage can be described as this:

Regarding current state of fetch usage, ember-fetch provides a Mixin for RestAdapter.

Questions I see as of today:

  • Should Ember Data depend on fetch out of the box? If not, how do we add fetch dependency only if opted out of jQuery?
  • Should Ember Data contain both network interfaces implementations? fetch and ajax? What about fastboot integration? Right now it causes trouble because it tries to replace network interface even if fetch is used.
  • What is the state of ds-improved-ajax? Is it dead? Is it planned to get it in? The fact we have a feature flag in our network code makes any changes to it even more complicated. cc @igorT

@tchak
Copy link
Member

tchak commented Jan 14, 2018

So in short, we need to remove the use of jQuery in purely internal places, and create a mode for using Ember Data without jQuery on an opt-in basis in places where jQuery is leaked into the public API.

So, could we completely remove jQuery from Ember Data and provide a replacement $.ajax network layer in @ember/jquery (or another addon)? As far as I can tell, there is no public api relying on jQuery specifics in Ember Data. But I imagine lots of apps today depend on ajaxOptions()(private api) and uses jQuery specific options like beforeSend :(

@tchak
Copy link
Member

tchak commented Jan 14, 2018

I just realised that Ember Data documentation is referring to ds-improved-ajax methods. Why is it still behind a flag then? Worse, if it is enabled, it will be silently disabled by using ember-fetch Mixin via _hasCustomizedAjax check...

@wycats
Copy link
Member Author

wycats commented Jan 15, 2018

@tchak mentioned in chat:

One way could be to detect if ember-fetch is installed, and based on this info to activate fetch support if jQuery is opted out.

I think it would be important to make sure the error messaging for this scenario is very clear (and ideally at build time). "You opted out of jQuery, but Ember Data requires ember-fetch when jQuery is disabled. Please add it to your package.json"

And we should mention it in an "opting out of jQuery" guide that we should write.

@wycats
Copy link
Member Author

wycats commented Jan 15, 2018

@igorT what's the story with ds-improved-ajax?

@sandstrom
Copy link
Contributor

For reference, here is another issue discussing the removal of jQuery from Ember Data: #4929

@sandstrom
Copy link
Contributor

sandstrom commented Jan 18, 2018

@wycats It was implemented in 2015 and then merged (still behind a flag) in 2016. A while later it was decided that it needed to go through the RFC-process, where it has stalled (it has remained behind a flag pending resolution on the RFC).

The aim of the ds-improved-ajax feature implemented in #3099 was to address the lack of public API for customizing a request. Unfortunately, it hasn't gone through a proper RFC process 😔. The feature is not enabled and available in a release, so this RFC should flesh out the specific API for the feature before it can be enabled eventually.

Source: emberjs/rfcs#171 (rendered)

(it was a while since I looked at the RFC closely, but from what I can remember it contained a bunch of good improvements + should be helpful in making jQuery optional, but I'd better have another look before I say anything with certainty)

@wycats
Copy link
Member Author

wycats commented Jan 18, 2018

@sandstrom is there anything stopping us from merging the RFC at this point?

/cc @igorT

@bmac
Copy link
Member

bmac commented Jan 19, 2018

I believe the RFC stalled waiting for some feedback/buy-in on the idea of aligning the proposed makeRequest method's arguments more closely with fetch API.

@wycats
Copy link
Member Author

wycats commented Jan 21, 2018

@bmac is there any problem with that?

@tchak
Copy link
Member

tchak commented Jan 21, 2018

@wycats I commented on the RFC with a list of pending concerns/questions

@tchak
Copy link
Member

tchak commented Jan 21, 2018

I am unsure how we can help to create a consensus on the RFC.

@cibernox
Copy link
Contributor

The truth is that without the ds-improved-ajax things like added auth headers to API calls in ember-data is pretty hard. If we consider ember-data part of an holistic "Ember experience" then I'd consider it a blocker.

tchak added a commit to tchak/data that referenced this issue Mar 12, 2018
tchak added a commit to tchak/data that referenced this issue Mar 12, 2018
bmac pushed a commit that referenced this issue Mar 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants