wrapShim config #623

Closed
jrburke opened this Issue Feb 15, 2014 · 9 comments

Projects

None yet

5 participants

@jrburke
Member
jrburke commented Feb 15, 2014

A built file with shim config can fail if it has an intermediate dependency that is an AMD module but downstream dependencies are not AMD.

Canonical example: Backbone depends on jQuery and Underscore. jQuery and Underscore do not have any dependencies, so they can also export a global value when they run, and only call define() at the end with the global they set up.

However, Backbone needs jQuery and Underscore loaded before its main module body can execute. So it is not executed right away, it is executed once the dependencies have had their define()'d factories called.

But if there is a shim config for something that depends on Backbone, that shimmed dependency wants Backbone available immediately when it executes, and it does not have a define() wrapper.

Before a build, this all works because the shimmed dep is not fetched until Backbone is fully defined. However, in a build, with all the dependencies inlined, there will be a failure.

A way to avoid this is to wrap the shimmed dependency in a define() call -- this avoids executing it until its dependencies are available.

I have traditionally avoided this because by wrapping, it changes the scope for the shimmed dependency. If that dependency is doing a var A = {} to define a global A, inside a define() wrapping that will not be true. Plus shim is a crutch, an intermediary step to full modularization. But hey, the peoples like to use shim, and some like to even use Backbone.

So allow shimmed scripts to be wrapped, and try to mitigate the wrapping effects. For the var A = {} case, if the shim config configures exports: 'A', then go ahead and assign that as a global a part of this wrapping.

So wrapShim: true will enable the wrapping behavior.

@jrburke jrburke added this to the 2.1.11 milestone Feb 15, 2014
@jrburke jrburke closed this in 16acd18 Feb 15, 2014
@jrburke
Member
jrburke commented Feb 15, 2014

This fix is in master now, it can be tried by using this version of r.js, until 2.1.11 is released:

https://raw.github.com/jrburke/r.js/master/dist/r.js

@KidkArolis

Seems to me that wrapShim: true should be the default and shims should be configured with exports explicitly.

@jrburke
Member
jrburke commented Feb 17, 2014

@KidkArolis I am still concerned about the scope changes wrapping introduced, and given that existing behavior did not wrap, I do not want to automatically change people's projects to introduce wrapping.

I could see though that after we have had this capability in use for a while, and we see it does not have any issues, then for a 3.0 type of release, it could be switched to true by default.

@gobwas
gobwas commented Feb 18, 2014

Is it possible to wrap shim of one concrete module, not all shimmed modules?

@jrburke
Member
jrburke commented Feb 19, 2014

@gobwas if you want to wrap just one, I suggest manually doing it yourself using the onBuildWrite hook.

@jrburke jrburke referenced this issue in requirejs/requirejs Feb 19, 2014
Closed

shim not working? #358

@gobwas
gobwas commented Feb 19, 2014

@jrburke thanks a lot! Was trying to do the same thing with onBuildRead yesterday - your tip is much easier =)

@davisagli davisagli added a commit to plone/mockup that referenced this issue Mar 13, 2014
@davisagli davisagli use wrapShim flag to fix dependence of a shim (backbone.paginator) on…
… an AMD module (backbone)

See requirejs/r.js#623 for more info

Without this we were getting "Uncaught ReferenceError: Backbone is not defined" errors in the plone bundle
2db6a5c
@blocka
blocka commented Apr 30, 2014

Seems that the if I use the shorthand syntax for dependencies, the dependencies do not show up in the build.

@mruzekw
mruzekw commented Jun 8, 2014

I'm finding these two rules conflict: skipModuleInsertion and wrapShim. If I'm creating a bundle called 'common', I don't want the extra define('common', ...) module created (though harmless). At the same time, I need to wrap libraries like underscore for the bundle. skipModuleInsertion: true currently precludes 'warpShim: true`. Is this the expected behavior?

@theurere theurere referenced this issue in strukturag/spreed-webrtc Jul 2, 2014
Merged

Update require.js to 2.1.14 #60

@jrburke
Member
jrburke commented Aug 23, 2014

Shim config is always a less desirable option, since the goal of having a module loader is to use modules with well defined, internally specified modules. There is always a limit to the extent of being able to support shims, since they are just inferior to actual modules, this is one of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment