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

Wrap shim code if have AMD dependencies #197

Closed
jrburke opened this issue Jun 15, 2012 · 4 comments
Closed

Wrap shim code if have AMD dependencies #197

jrburke opened this issue Jun 15, 2012 · 4 comments

Comments

@jrburke
Copy link
Member

jrburke commented Jun 15, 2012

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
Copy link

nagyv commented Oct 5, 2012

+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
Copy link
Member Author

jrburke commented Oct 6, 2012

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.

@jrburke jrburke closed this as completed Oct 6, 2012
@nagyv
Copy link

nagyv commented Oct 6, 2012

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
Copy link
Member Author

jrburke commented Oct 7, 2012

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants