Configurable shim module format - AMD or ES2015

[timiyay]

This proof-of-concept PR suggests a solution to #98. It allows downstream apps to choose the module format for the ember-query shim added in #33.

This solves an issue for AMD-dependent systems like one of ours, which uses ember-cli/loader to set up a module alias. This was breaking down when receiving the shim in ES2015 format.

[timiyay]

I don't have a complete understanding of the underlying Ember build systems, so I'm uncertain whether this solution will have unwanted side effects.

I did some testing against one of our apps, where an Ember client wraps a HTML5 game framework. The game code that relies on the AMD jQuery alias started working again, and none of our Ember code was adversely affected, even the components that use import $ from 'jquery'.

[timiyay]

The Travis CI run failed due to an unrelated issue. The release-it dependency is failing to install due to the Node version.

[simonihmig]

Although I did that shim change back then, I see myself unable to make a final call here, as I am not sure we want to support this here.

My understanding is:

• before the RFC386 related changes, import jquery from 'jquery'; would be transpiled to something like const jquery = Ember.$;, so actually not using a module at all
• with the recent changes, having @ember/jquery around will deactivate that transpilation, so the shim is used
• this shim does exactly what ember g vendor-shim generates, so is following what Ember assumes a module should look like (i.e. ES2015)
• the use of require('jquery') was working before because jQuery itself provided an AMD compatibility wrapper: https://github.com/jquery/jquery/blob/e743cbd28553267f955f71ea7248377915613fd9/src/exports/amd.js#L21
• the new shim will override the module definition here: https://github.com/emberjs/ember-jquery/blob/master/vendor/shims/jquery.js#L11

I wondered why the AMD shim actually worked with Ember, but it seems there is some explicit compatibility support in loader.js: https://github.com/ember-cli/loader.js/blob/master/lib/loader/loader.js#L120

Can we eventually rely on that feature, and just use an AMD shim?

Any thoughts @rwjblue?

The Travis CI run failed due to an unrelated issue. The release-it dependency is failing to install due to the Node version.

Indeed, also failing for dependabot updates. Seems that package did not really follow semver! I dropped node 6 support here: #100

[timiyay]

This is great background, thanks @simonihmig, especially the info on how ember-cli/loader's makeDefaultExport function enables AMD compatibility.

Thanks for patching the Travis failure, this PR is all green now 🍏

I'll wait to hear from others, before digging further into Ember's vendor-shim functionality (I've not worked with it before).

[rwjblue]

Can we eventually rely on that feature, and just use an AMD shim?

No, we are trying to deprecate/remove makeDefaultExport over in ember-cli/loader.js#128 (though I dropped the ball, and it needs a kick start to push into motion again).

[rwjblue]

@timiyay - Can you explain more about your usage of loader.js? Are you manually doing let jQuery = require('jquery')? In general, I would have assumed folks were already doing import jQuery from 'jquery' which roughly transpiles to reading require('jquery').default which does work (in 0.5 and 0.6).

[timiyay]

@rwjblue I'll give my best explanation, with the caveat that I'm working on a legacy system I don't 100% understand just yet.

We have a HTML5 game framework which uses AMD modules. These games are embedded within our client Ember apps. The Ember apps are responsible for loading the game manifests, and executing the relevant game module.

At this point, the game module takes over, and loads its modules via AMD.

Since the games are always served within in Ember app, the game framework uses ember-cli/loader as its AMD module loader, instead of RequireJS. We also recycle the Ember-provided jQuery and use it within the games.

We recently upgraded ember-jquery to 0.6.1, because we needed jQuery 3.4.1 for an iOS 10 bugfix. This caused our game AMD loading to hit an issue, when it attempts to require jquery, as it expects an AMD module but receives an object { default: 'jquery-module', __esModule: true }

It sounds likely that our legacy module system using ember-cli/loader will need a rework in the near future, but it's no mean feat, so we're looking for strategies to maintain compatibility for as long as possible.

[rwjblue]

I think you could probably fix the issue by defining your own jquery module in your app and app.import it. 🤔

[timiyay]

@rwjblue Hey yeah this crossed my mind, but I'm yet to properly consider it.

To be clear, you're suggesting we could skip ember-jquery altogether, and consume jquery in our preferred fashion straight from npm?

Would this nerf Ember.$?

[rwjblue]

No, I was more thinking that you could use @ember/jquery, but in your app have a file like:

// vendor/jquery/shim.js
define('jquery', [], function() {
  self.jQuery.default = self.jQuery;
  return self.jQuery;
});

Then the app.import that @ember/jquery is doing already will get your file instead of the shim in this repo, but we should also add a config flag here to just avoid the app.import (since we really don't want to depend on the merging + clobbering behavior here).

[timiyay]

Ah brilliant! I didn't connect the dots that files in the vendor dir can be overridden in this way, like with other dirs.

This fixes our issue, and seems like a reasonable solution for us.

[timiyay]

We've tested out @rwjblue's solution, and it works for us, and is nice and explicit.

I'm satisfied.