requirejs / r.js

Runs RequireJS in Node and Rhino, and used to run the RequireJS optimizer
Other
2.57k stars 672 forks source link

Wrap shim code if have AMD dependencies #197

Closed jrburke closed 11 years ago

jrburke commented 12 years ago

If a shim module has a dependency on a module that is AMD with dependencies, consider wrapping the shim lib in a define. Watch out for scope problems. Maybe warn on any auto wrap done.

nagyv commented 11 years ago

+1 on this

currently, I have backbone-amd and backbone-relational non-amd used together with

shim: {
    "backbone-relational": ["backbone"]
}

this works fine when developing, but fails when trying to build it

is there any workaround until this becomes a feature?

thanks for your great effort!

jrburke commented 11 years ago

If you include backbone in the build then it should work.

The more I think about it, the more I do not want to try to add a wrapper -- it affects the scope around the wrapped library, and it would actually break some libraries that just define their globals via var myGlobal = ....

So, I'm going to close this ticket for now, as it will just create other problems. The current guidance is to always build in the dependencies of shimmed scripts into the build layer that contains the shimmed script.

nagyv commented 11 years ago

probably, I'm not aware of how to include backbone properly in the build, as I've tried to do that

my built script concatenated the js libs in the following order (taken from the js comments in the final file)

/*!
* Bootstrap.js by @fat & @mdo
* Copyright 2012 Twitter, Inc.
* http://www.apache.org/licenses/LICENSE-2.0.txt
*/
..
/* Modernizr 2.5 (Custom Build) | MIT & BSD
 * Build: http://www.modernizr.com/download/#-fontface-backgroundsize-borderimage-borderradius-boxshadow-flexbox-hsla-multiplebgs-opacity-rgba-textshadow-cssanimations-csscolumns-generatedcontent-cssgradients-cssreflections-csstransforms-csstransforms3d-csstransitions-applicationcache-canvas-canvastext-draganddrop-hashchange-history-audio-video-indexeddb-input-inputtypes-localstorage-postmessage-sessionstorage-websockets-websqldatabase-webworkers-geolocation-inlinesvg-smil-svg-svgclippaths-touch-webgl-shiv-mq-cssclasses-addtest-prefixed-teststyles-testprop-testallprops-hasevent-prefixes-domprefixes-load
 */
..
/*!
 Lo-Dash 0.3.2 lodash.com/license
 Underscore.js 1.3.3 github.com/documentcloud/underscore/blob/master/LICENSE
*/
..
// Backbone.js 0.9.2

// (c) 2010-2012 Jeremy Ashkenas, DocumentCloud Inc.
// Backbone may be freely distributed under the MIT license.
// For all details and documentation:
// http://backbonejs.org
..
/**
 * Backbone-relational.js 0.6.0
 * (c) 2011 Paul Uithol
 * 
 * Backbone-relational may be freely distributed under the MIT license; see the accompanying LICENSE.txt.
 * For details and documentation: https://github.com/PaulUithol/Backbone-relational.
 * Depends on Backbone (and thus on Underscore as well): https://github.com/documentcloud/backbone.
 */
..
etc

Thus backbone is supposed to be present by the time the

(function(){
// this is Backbone.Relational's clojure
})()

get's called

I'm using this version of backbone: https://github.com/amdjs/backbone and this for backbone-relational: https://github.com/PaulUithol/Backbone-relational

I know that this is not related to the original issue any more, and I totally understand your reason behind closing the original ticket

jrburke commented 11 years ago

It looks like jquery is not in the built file, that will need to be in there too. And it may also work better to use the regular backbone.js file instead of the amdjs version.