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

Further AMD Support Improvements #155

Closed
jeffrose opened this issue Nov 14, 2012 · 11 comments
Closed

Further AMD Support Improvements #155

jeffrose opened this issue Nov 14, 2012 · 11 comments
Milestone

Comments

@jeffrose
Copy link

I am glad to see the improvements in AMD support introduced in 1.1.0. Looking through the code I still see a couple areas where it would be in CanJS's best entrance to leave certain decisions up to its users.

Avoid Version References

In order to avoid a maintenance headache, anyone working within an AMD environment has found a way to deal with version numbers. Most likely, they have a mapping of ids to actual versioned file names.

require.config( {
    "path": {
        "zepto": "zepto-1.0rc1"
    }
} );

CanJS should publish what versions are officially supported but avoid referencing the actual versions in the code. Leave it up to the developer to decide what version is provided. Otherwise users are dependent on the CanJS team to perform an update when a new version of a library is released. While CanJS 1.1.0 was written to use dojo 1.8.1, it will likely work just fine with 1.8.2.

Avoid Global Variable References

jQuery technically does not follow the AMD specification because even though it defines itself, it still puts variables in the global scope. This was a decision made for backwards compatibility. Those trying to adhere more strictly to the AMD specification have likely found ways of dealing with this, e.g. calling $.noConflict( true ). In such situations, making references to jQuery without requiring it will not work. For example in can/util/jquery, jquery is among the dependencies and assigned to $ but there are references to jQuery in the code.

define(['jquery', 'can/util/can', 'can/util/array/each'], function ($, can) {
    // _jQuery node list._
    $.extend(can, jQuery, {
        trigger: function (obj, event, args) {
...

Instead of referencing the global variable, make a local variable based on the dependency.

define(['jquery', 'can/util/can', 'can/util/array/each'], function (jQuery, can) {
    var $ = jQuery;
    // _jQuery node list._
    $.extend(can, jQuery, {
        trigger: function (obj, event, args) {
...

It would also help to avoid issues like jupiterjs/jquerymx#134.

Avoid Internal References

There are references to specific 3rd-party libraries in the CanJS code, some of which do not exist in zip file downloaded from the site, e.g. 'can/util/yui/yui-3.7.3', 'can/util/mootools/mootools-core-1.4.3', etc. Unless these files are somehow different from the ones provided by the library's site, just use a reasonable id, e.g. 'yui' and leave it up to the developer to map that id or their copy fo the library. If it's not clear what the id is referencing, just provide a comment or documentation.

I am definitely excited by the progress CanJS has made. Thanks for all the hard work!

@daffl
Copy link
Contributor

daffl commented Nov 14, 2012

Thanks for the feedback! Aliasing the libraries was on my Todo list but I forgot adding it except for jQuery.
Not referencing global variables is also a good point. I will do my best to get this into the 1.1.1 release.
Let me know if you find anything else using the different CanJS modules.

@justinbmeyer
Copy link
Contributor

I think we will still add can to the namespace but maybe add a similar no conflict or a way to turn that off.

It drives me crazy always having to write can and jquery when foo depends on them just to use them like

'foo', 'can', 'jquery' ... (Foo, can, jquery){

I just want 'foo' ... (Foo){

I know this is the spirit of amd, but it sucks in practice and is probably not worth it in 98% of apps.

Sent from my iPhone

On Nov 14, 2012, at 12:02 PM, Jeff Rose notifications@github.com wrote:

I am glad to see the improvements in AMD support introduced in 1.1.0. Looking through the code I still see a couple areas where it would be in CanJS's best entrance to leave certain decisions up to its users.

Avoid Version References

In order to avoid a maintenance headache, anyone working within an AMD environment has found a way to deal with version numbers. Most likely, they have a mapping of ids to actual versioned file names.

require.config( {
"path": {
"zepto": "zepto-1.0rc1"
}
} );
CanJS should publish what versions are officially supported but avoid referencing the actual versions in the code. Leave it up to the developer to decide what version is provided. Otherwise users are dependent on the CanJS team to perform an update when a new version of a library is released. While CanJS 1.1.0 was written to use dojo 1.8.1, it will likely work just fine with 1.8.2.

Avoid Global Variable References

jQuery technically does not follow the AMD specification because even though it defines itself, it still puts variables in the global scope. This was a decision made for backwards compatibility. Those trying to adhere more strictly to the AMD specification have likely found ways of dealing with this, e.g. calling $.noConflict( true ). In such situations, making references to jQuery without requiring it will not work. For example in can/util/jquery, jquery is among the dependencies and assigned to $ but there are references to jQuery in the code.

define(['jquery', 'can/util/can', 'can/util/array/each'], function ($, can) {
// jQuery node list.
$.extend(can, jQuery, {
trigger: function (obj, event, args) {
...
Instead of referencing the global variable, make a local variable based on the dependency.

define(['jquery', 'can/util/can', 'can/util/array/each'], function (jQuery, can) {
var $ = jQuery;
// jQuery node list.
$.extend(can, jQuery, {
trigger: function (obj, event, args) {
...
It would also help to avoid issues like jupiterjs/jquerymx#134.

Avoid Internal References

There are references to specific 3rd-party libraries in the CanJS code, some of which do not exist in zip file downloaded from the site, e.g. 'can/util/yui/yui-3.7.3', 'can/util/mootools/mootools-core-1.4.3', etc. Unless these files are somehow different from the ones provided by the library's site, just use a reasonable id, e.g. 'yui' and leave it up to the developer to map that id or their copy fo the library. If it's not clear what the id is referencing, just provide a comment or documentation.

I am definitely excited by the progress CanJS has made. Thanks for all the hard work!


Reply to this email directly or view it on GitHub.

@daffl
Copy link
Contributor

daffl commented Nov 14, 2012

Actually we should even have that functionality already. Like jQuery it adds window.can by default.

But you can set GLOBALCAN = false; (which the AMD generator should probably do actually).

Most can modules return the can object as well (except for Control and Model I think). Not sure if this would break a lot if we make the model and control modules return can instead of Control or Model?

@jeffrose
Copy link
Author

@justinbmeyer Most of my experience with AMD involves RequireJS, so I can't speak for every AMD loader out there, but if all you want is Foo, you shouldn't have to require can and jquery too unless you want a handle to them.

// foo.js
define( [ 'jquery', 'can' ], function( $, can ){
    var Foo;
    // Use $ and can to implement foo's functionality
    return Foo;
} );

Then when you want to use Foo...

require( [ 'foo' ], function( Foo ){
    // $ and can are loaded as dependencies in the background
    // Use Foo
} );

I believe if you wanted to, you could actually do something like this...

require( [ 'require', 'foo' ], function( require, Foo ){
    // Use Foo
    // A need for $ has arisen...
    var $ = require( 'jquery' );
} );

@jeffrose
Copy link
Author

@daffl From what I've seen all of the CanJS AMD modules, including Control and Model just return can. It's more typical for an AMD module to return a handle to the functionality it provides, e.g. can.Control, can.Model, etc. Since all of the functionality is added to a centralized namespace, I can see the argument for just returning can however it does lead to this scenario...

require( [ 'can/control', 'can/model', 'can/route', 'jquery' ], function( can, can_also, can_yet_again, $ ){
...

@justinbmeyer
Copy link
Contributor

can/control should return can.Control like it does here:

https://github.com/bitovi/canjs/blob/master/control/control.js#L749

It's not right now with AMD?

all you want is Foo, you shouldn't have to require can and jquery too unless you want a handle to them.

Yeah, I almost always want a handle to them (in 90% of files).

I do this typically:

steal('can', function(can){
  return can.Control({
    init : function(){
      // something that uses $
    }

  });
})

now, I'm actually that lazy I don't want to add 'jquery' to get a handle on it.

@jeffrose
Copy link
Author

I am looking in can.js.1.1.0.zip\1.1.0\amd\can\control.js and at the bottom of the file, it has return can; which seems in line with the define statement at the top which does...

define(['can/util', 'can/construct'], function (can) {

@rjgotten
Copy link

now, I'm actually that lazy I don't want to add 'jquery' to get a handle on it.

As can.$ refers the original selector library:

define([ "can" ], function( can ) {
  var $ = can.$;

  // Use $ (i.e. jQuery) here
});

Also, if you want to load a main library and don't feel manually requiring all manner of additional dependencies the whole time to make sure it has all its 'plugins' / 'extensions' / etc. applied, then I've got you covered with a little AMD loader plugin.

@jeffrose
Copy link
Author

@rjgotten Cool plugin. Does it have an impact on how r.js bundles files? That is, does plugging libraries together lead to them being bundled by the optimizer?

@justinbmeyer I certainly understand the laziness factor. If anything I think that speaks to the lack of IDE support. Listing off dependencies wouldn't be so bad if they could be autocompleted or even autogenerated by a quick code scan.

In general I've found the pros of being required to list off dependencies outweighs the cons. Your mileage may vary.

@rjgotten
Copy link

That is, does plugging libraries together lead to them being bundled by the optimizer?

It bundles them in the correct order when used through the r.js optimizer, yes.

@daffl
Copy link
Contributor

daffl commented Nov 20, 2012

Ok so version 1.1.1 comes with a whole bunch of AMD improvements:

  • Removed the reference to the global jQuery and used the module specific $ instead
  • Use mootools, yui, dojo and zepto so you can provide your own mappings (plus no version references)
  • No annoying mapping of can-library. Loads can/util.js by default which in turn loads can/util/jquery. Just map can/util.js to e.g. can/util/yui if you want to use one of the other libraries
  • Use the original Steal module return statements. So can/construct actually returns Construct so you can do define(['can/construct'], function(Construct) {});

Thanks for the feedback. Just open a new issue if you find any AMD related problems.

@daffl daffl closed this as completed Nov 20, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants