Treat jQuery & Tether as dependencies in ES6 source code

[skrivle]

First of all, congrats on the v4 release guys. Looking really nice! I'm especially excited about the new ES6 source code. However, when scanning the source code I noticed that jQuery isn't imported like the other dependencies. Because of this jQuery is also missing in the UMD definitions that are generated via grunt. This causes the UMD modules to be more or less unusable unless you include jQuery globally.

I guess the reason for this is that it's probably difficult to guess how people are going to import jQuery. Import paths and package names may differ between package managers, etc ...

However, since jQuery is the only external dependency here, I think it should be possible. For example you could ask people using requirejs to define a custom path, which they are probably doing already ...

On npm the package for jQuery is 'jquery', so this shouldn't cause any issues. Then the only remaining problem is that babel is converting this:

import $ from 'jquery'

to

factory(mod.exports, mod, global.$);

Which probably will work, but it is kind of risky to use '$' instead of jQuery I guess ... Maybe there is way to hook into babel's compiling step and change module names there ...

Adding jQuery as a dependency will also enable people who are already writing ES6 code to actually import the ES6 code ...

I may be missing a lot of stuff here, so any feedback is more than welcome!

[zacechola]

FWIW Ember handles this by importing jQuery by default while allowing authors to toggle that default off.

[skrivle]

I think the simplest solution would be to just import jQuery as jQuery and then map $ to jQuery. This way you'll get a clean UMD definition, which is usable with bower or npm and requirejs, commonjs or globals ...

import jQuery from 'jquery'

const $ = jQuery

assuming requirejs users create a custom path for 'jquery' ...

[hnrch02]

CC @fat

[skrivle]

Small update:

I was able to get it working just by adding import jQuery as jQuery to every JS file. If you're ok with it, I would suggest to run the unit tests directly on the UMD builds ... I see no need to compile a version without UMD definitions anymore ...

One thing I noticed though while testing this locally, is that there are some additional issues regarding the UMD builds ... I've created a separate issue for this.

Once I fixed these 2 problems I was able to successfully run unit tests against the UMD builds. If you want I can come up with a pull request.

[krnlde]

@vejersele

import jQuery as $ from 'jquery'

[skrivle]

@krnlde That doesn't work ...

import {jQuery as $} from 'jquery'

This does work, but leaves us with the same issue as we had in the first place since it's compiling to global.jquery while we actually want global.jQuery.

[krnlde]

What about import * as $ from jQuery

[skrivle]

I assume you mean import * as $ from 'jQuery' which compiles to global.$ ... Also, it will use require('jQuery') in a node environment, while I don't think require() is case sensitive (at least on a OSX it isn't) it seems like a dangerous guess.

[krnlde]

Sorry for the errors, I answered from my mobile, which autocorrect, and I correctly assumed you'd guess the correct. This aside, const $ = jQuery doesn't look like a good practice to me.

[skrivle]

I'm aware that this isn't the cleanest solution, but at least it works ... I only see two options, either use const $ = jQuery or wrapping the component code within an IIFE with $ as a param (how it's already done right now), which is essentially the same.

[krnlde]

I don't know enough about destructuring, but this could work also, if you can use the same key twice: import {jQuery as $, jQuery} from 'jquery'

[skrivle]

Not sure if I need to create another issue for this, but I also think we should specify jQuery and Tether as dependencies in the package.json and bower.json files ...

[zacechola]

@vejersele Doesn't hurt. Just x-ref the issue numbers as they're all related anyway.

[fat]

@vejersele if you havne't already, a pr would be rad

[skrivle]

Once #18313 is merged I'll put together a pull request.

[krnlde]

There is a "+1"-reaction now, please use this instead.

[DaSchTour]

But the +1 reaction doesn't add you as a participant ;)

[krnlde]

So you want to say bombing this thread is better than doing a second click on the subscribe button? Give that man a burrito.

[DaSchTour]

Subscribe isn't the same as participate ;)

[krnlde]

But apparently trolling is.

[DaSchTour]

You think that's a joke? Just click on the watch/unwatch button and you will see what I mean ;)

[mdo]

According to the comment in #20745, I think #19541 took care of this. Correct me if I'm wrong though!

[bardiharborow]

#20745 was replaced by #19541, but neither completely fix this issue. We need to work out how we are going to manage all this, especially given that native module support in Chrome isn't too far off.

[karolyi]

Well, injecting tether with webpack works this way, but when you use require('Tether') and webpack is configured correctly, it will import tether for itself.

I don't know anything about chrome native module support, but since it doesn't exist yet, you have to transpile TWBS's modules anyway, and that solves the problem.

or did I miss anything?

[gijsbotje]

issue can be closed right? #22444

[karolyi]

hey, thanks for the heads up for this, changing to Popper was news to me, however I welcome it. I had problems here and there with tether, and looking forward replacing it with the next alpha release.

as for this issue, yes it should be closed.

[Johann-S]

Hmm I don't think we should close this issue because we should treat jQuery and (now) Popper.js as dependencies and it will be done here #22888

[Johann-S]

Closed thanks to #23735