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

Not sure how to include with AMD #28

Closed
SimplGy opened this issue Jun 3, 2013 · 11 comments
Closed

Not sure how to include with AMD #28

SimplGy opened this issue Jun 3, 2013 · 11 comments

Comments

@SimplGy
Copy link

SimplGy commented Jun 3, 2013

Hi!

I'm working on a transport issue.

When I include draggabilly.js as an AMD dependency, it tries to track down the dependencies of your library in the context of my app. Is this appropriate? Here's the output I see:

GET http://assessments-front.dev/get-style-property.js 404 (Not Found) require.js:1872
Uncaught Error: Script error for: get-style-property
http://requirejs.org/docs/errors.html#scripterror require.js:163
GET http://assessments-front.dev/classie.js 404 (Not Found) require.js:1872
GET http://assessments-front.dev/eventEmitter.js 404 (Not Found) require.js:1872
GET http://assessments-front.dev/eventie.js 404 (Not Found) require.js:1872
GET http://assessments-front.dev/get-size.js 404 (Not Found)

I see why this is (the amd define at the bottom of draggabilly indicates those dependencies. What is the right way to resolve this? Should I add require config to map these module names to the correct dependency path? I don't see those dependencies in the draggabilly project.

@SimplGy
Copy link
Author

SimplGy commented Jun 3, 2013

Oh, I see what's going on. The packaged version defines dependencies like classie as AMD modules but the bower install draggabilly source version does not.

@SimplGy
Copy link
Author

SimplGy commented Jun 3, 2013

And finally the actual, real root cause. Bower is pulling in the dependencies correctly, but the default AMD path doesn't match where bower puts them. What about changing the current define:

if ( typeof define === 'function' && define.amd ) {
  // AMD
  define( [
      'classie',
      'eventEmitter',
      'eventie',
      'get-style-property',
      'get-size'
    ],
    draggabillyDefinition );
} else { /* ... */ }

Why not change it to this, so that the paths work by default?

if ( typeof define === 'function' && define.amd ) {
  // AMD
  define( [
      '../classie/classie',
      '../eventEmitter/EventEmitter.min',
      '../eventie/eventie',
      '../get-style-property/get-style-property',
      '../get-size/get-size'
    ],
    draggabillyDefinition );
}

@desandro
Copy link
Owner

desandro commented Jun 3, 2013

Why not change it to this, so that the paths work by default?

I recommend setting up paths in your RequireJS config. This allows better flexibility.

@SimplGy
Copy link
Author

SimplGy commented Jun 3, 2013

Done, but I've never seen a library require me to config the paths for its dependencies. Looks like this. Have to add a whole section for draggability deps. Smells funny:

define [], () ->
  require.config
    paths:
      # 3rd Party Bower Libraries
      Handlebars: "bower_components/require-handlebars-plugin/Handlebars"
      underscore: "bower_components/underscore-amd/underscore"
      jquery: "bower_components/jquery/jquery"
      jqueryui: "bower_components/jquery-ui/jqueryui"
      backbone: "bower_components/backbone-amd/backbone"
      syphon: 'bower_components/backbone.syphon/lib/amd/backbone.syphon'
      text: "bower_components/requirejs-text/text"
      toastr: "bower_components/toastr"
      draggabilly: 'bower_components/draggabilly/draggabilly'
      # draggabilly deps
      classie: 'bower_components/classie/classie'
      eventEmitter: 'bower_components/eventEmitter/EventEmitter.min'
      'get-style-property': 'bower_components/get-style-property/get-style-property'
      'get-size': 'bower_components/get-size/get-size'
      eventie: 'bower_components/eventie/eventie'

      # 3rd Party non-Bower Libraries
      nested_view: "scripts/vendor/nested_view"
      bootstrap: "scripts/vendor/bootstrap"
      chart: "scripts/vendor/Chart"

      # Convenience Folder Mapping
      assessments: "scripts/views/assessments"
      dashboard: "scripts/views/dashboard"
      components: "scripts/views/components"
      results: "scripts/views/results"
      routers: "scripts/routers"
      models: "scripts/models"
      controllers: "scripts/controllers"
      collections: "scripts/collections"
      helpers: "scripts/helpers"
      messages: "scripts/views/messages"
    shim:
      bootstrap:
        deps: ["jquery"]
        exports: "jquery"
      chart:
        exports: "Chart"

@desandro
Copy link
Owner

desandro commented Jun 4, 2013

I agree this is not an ideal solution. We're still working out the best practices with Bower integration with AMD. What would be ideal for you? Using relative paths in the draggabilly source?

@andreu86
Copy link

It would be great a version with all dependencies inside it, and another version minified, both would be interesting.

@SimplGy
Copy link
Author

SimplGy commented Jun 13, 2013

@andreu86: If you include all the depencencies inside a dependency, this means the client will have to download too much. For example, say that:

`SprocketMaster` depends on `jQuery`

and

`Lightbox` depends on `jQuery`

In a "all dependencies inside it" model, the client would actually have to download jQuery two times.
The trick is to come up with a convention so that everything that depends on jQuery (or anything else) has a standard place to look for it. I think Bower/jam/etc are the closest thing we have to a convention like that.

@andreu86
Copy link

I understand your point of view, maybe a Grunt task to generate a full version would be enough, as an extra option.

Thanks for replying!

@desandro
Copy link
Owner

Yeah, it does smell funny. Have you tried the Bower+RequireJS Grunt task? It might be the missing piece of the puzzle.

@desandro
Copy link
Owner

I'm looking for feedback on how to best set up RequireJS dependencies in my components. Help me out! See https://github.com/desandro/requirejs-bower-homework

@desandro
Copy link
Owner

draggabilly.pkgd.js is now generated with RequireJS. So you can require the packaged file. Or you can manage dependencies with Bower. See http://draggabilly.desandro.com/#requirejs

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

3 participants