amdjs / amdjs-api

Houses the Asynchronous Module Definition API
https://groups.google.com/group/amd-implement
4.31k stars 499 forks source link

AMD define() signature augmentation request. #5

Closed dschnare closed 12 years ago

dschnare commented 12 years ago

See definejs for an example implementation and usage

I'd like to request that the AMD 'define()' have the following signatures:


define(module value | module factory) Defines an anonymous module, extracting its ID from the module's path. If a module factory is specified then the factory is called and the module's value will be equal to its return value.

define({/* properties of the module */});

define(function() {
  return {
    /* module properties */
  };
});

define([dependency array of module IDs], module value | module factory) Defines an anonymous module, extracting its ID from the module's path. If a module factory is specified then the factory is called with the values of each dependent module loaded from the dependency array, and the module's value will be equal to the factory's return value. The module factory will only be called after each dependency has loaded and the dependcies wil be passed to the factory in the same order as the dependency array.

define(['a', 'b'], {/* module properties */});

define(['a', 'b'], function(a, b) {
  return {
    /* module properties */
  };
});

define(module ID, [dependency array of module IDs], module value | module factory) Defines a named module, where the specified module ID is used for caching instead of the modules path. If a module factory is specified then the factory is called with the values of each dependent module loaded from the dependency array, and the module's value will be equal to the factory's return value. The module factory will only be called after each dependency has loaded and the dependcies wil be passed to the factory in the same order as the dependency array.

define('myModule', ['a', 'b'], {/* module properties */});

define('myModule', ['a', 'b'], function(a, b) {
  return {
    /* module properties */
  };
});

define({imports}, module value | module factory) [new signature] Where 'imports' is an object where each key is the name of an import and whose value is a dependent module ID. Defines an anonymous module, extracting its ID from the module's path. If a module factory is specified then the factory is called with an object where each key from the imports object equals the corresponding loaded module. The module's value will be equal to the factory's return value. The module factory will only be called after each dependency has loaded.

define({a: 'a', b: 'b'}, {/* module properties */});

define({a: 'a', b: 'b'}, function(imports) {
  return {
    /* imports.a and imports.b are the module values from loading 'a' and 'b' */
    /* module properties */
  };
});

define(module ID, {imports}, module value | module factory) [new signature] Where 'imports' is an object where each key is the name of an import and whose value is a dependent module ID. Defines named module, where the specified module ID is used for caching instead of the modules path. If a module factory is specified then the factory is called with an object where each key from the imports object equals the corresponding loaded module. The module's value will be equal to the factory's return value. The module factory will only be called after each dependency has loaded.

define('myModule', {a: 'a', b: 'b'}, {/* module properties */});

define('myModule', {a: 'a', b: 'b'}, function(imports) {
  return {
    /* imports.a and imports.b are the module values from loading 'a' and 'b' */
    /* module properties */
  };
});

The following signatures are not so import to include, but offer an object-literal technique, so that the arguments are are explicitly named and easier to understand when reading code.

define({module: module value | module factory})

define({id: module ID, module: module value | module factory})

define({imports: {imports}, module: module value | module factory})

define({id: module ID, imports: {imports}, module: module value | module factory})

NOTES:

At the very least I'd would love to see the addition of the import object. The advantage of using an import object is that the import name of each module being imported is given at the same location where the dependency is specified and not in the module factory signature. This permits a module factory to have one formal paramter for all imports which much easier to read for modules that have several dependencies.

jrburke commented 12 years ago

Sorry for the delay in response, we were not getting notifications of new issues. Mentioning @unscriptable so he gets notified too.

In general, most of these signatures are supported, but the API doc is very terse and may not make this clear. I have an action item to expand the API doc with more examples to make it clear. Specific feedback:

define(module value | module factory)

this is supported in the current API definition, as long as "module value" is an object literal. Using an array would confuse it with the dependency list.

define([dependency array of module IDs], module value | module factory)

This is supported today, except if there is a dependency array, then the second arg must be a module factory. Having a module value does not make sense here, since if it is a value it means the module is fully formed without needing dependencies.

define(module ID, [dependency array of module IDs], module value | module factory)

Supported today, with the exception of the "module value" caveat mentioned above.

define({imports}, module value | module factory) [new signature]

You can get this today via the simplified commonjs wrapper:

define(function (require) {
    var a = require('a'),
        b = require('./b')
});

The function is toString()'ed and require calls are parsed out, those dependencies are loaded, then the the factory function is executed. In the requirejs optimizer, this form is transformed to explicitly list the dependencies in an array so no toString() work is done after a build.

In this way require acts like the import object, and it works better with tools like jslint, since getting a local var enables getting warnings of an unused dependency and warning of a missing dependency.

I am going to close this for now, but we can continue discussing it and reopen if there is a need. However, I believe adding an imports construct will add more to the API surface and require as used above gives the same behavior.

dschnare commented 12 years ago

Hmm, you make a good point. Perhaps the new signatures are not required.

Sent from my Windows Phone From: James Burke Sent: 15-Nov-2011 2:39 PM To: Darren Schnare Subject: Re: [amdjs-api] AMD define() signature augmentation request. (#5) Sorry for the delay in response, we were not getting notifications of new issues. Mentioning @unscriptable so he gets notified too.

In general, most of these signatures are supported, but the API doc is very terse and may not make this clear. I have an action item to expand the API doc with more examples to make it clear. Specific feedback:

define(module value | module factory)

this is supported in the current API definition, as long as "module value" is an object literal. Using an array would confuse it with the dependency list.

define([dependency array of module IDs], module value | module factory)

This is supported today, except if there is a dependency array, then the second arg must be a module factory. Having a module value does not make sense here, since if it is a value it means the module is fully formed without needing dependencies.

define(module ID, [dependency array of module IDs], module value | module factory)

Supported today, with the exception of the "module value" caveat mentioned above.

define({imports}, module value | module factory) [new signature]

You can get this today via the simplified commonjs wrapper:

define(function (require) {
    var a = require('a'),
        b = require('./b')
});

The function is toString()'ed and require calls are parsed out, those dependencies are loaded, then the the factory function is executed. In the requirejs optimizer, this form is transformed to explicitly list the dependencies in an array so no toString() work is done after a build.

In this way require acts like the import object, and it works better with tools like jslint, since getting a local var enables getting warnings of an unused dependency and warning of a missing dependency.

I am going to close this for now, but we can continue discussing it and reopen if there is a need. However, I believe adding an imports construct will add more to the API surface and require as used above gives the same behavior.


Reply to this email directly or view it on GitHub: https://github.com/amdjs/amdjs-api/issues/5#issuecomment-2749829

dschnare commented 12 years ago

Actually, by using the 'imports' object we do not rely on the JavaScript environment being able to inspect function bodies.

Sent from my Windows Phone From: James Burke Sent: 15-Nov-2011 2:39 PM To: Darren Schnare Subject: Re: [amdjs-api] AMD define() signature augmentation request. (#5) Sorry for the delay in response, we were not getting notifications of new issues. Mentioning @unscriptable so he gets notified too.

In general, most of these signatures are supported, but the API doc is very terse and may not make this clear. I have an action item to expand the API doc with more examples to make it clear. Specific feedback:

define(module value | module factory)

this is supported in the current API definition, as long as "module value" is an object literal. Using an array would confuse it with the dependency list.

define([dependency array of module IDs], module value | module factory)

This is supported today, except if there is a dependency array, then the second arg must be a module factory. Having a module value does not make sense here, since if it is a value it means the module is fully formed without needing dependencies.

define(module ID, [dependency array of module IDs], module value | module factory)

Supported today, with the exception of the "module value" caveat mentioned above.

define({imports}, module value | module factory) [new signature]

You can get this today via the simplified commonjs wrapper:

define(function (require) {
    var a = require('a'),
        b = require('./b')
});

The function is toString()'ed and require calls are parsed out, those dependencies are loaded, then the the factory function is executed. In the requirejs optimizer, this form is transformed to explicitly list the dependencies in an array so no toString() work is done after a build.

In this way require acts like the import object, and it works better with tools like jslint, since getting a local var enables getting warnings of an unused dependency and warning of a missing dependency.

I am going to close this for now, but we can continue discussing it and reopen if there is a need. However, I believe adding an imports construct will add more to the API surface and require as used above gives the same behavior.


Reply to this email directly or view it on GitHub: https://github.com/amdjs/amdjs-api/issues/5#issuecomment-2749829

jrburke commented 12 years ago

Actually, by using the 'imports' object we do not rely on the JavaScript environment being able to inspect function bodies.

We tested JS environments and only the PS3 netfront browser and older opera mobile browsers did not return usable function bodies. Those environments really need built resources (which can have their dependencies parsed out, as the requirejs optimizer does), so this seemed like a workable compromise, and it allowed for easier CommonJS module wrapping.

dschnare commented 12 years ago

That makes sense, thanks James.

Sent from my Windows Phone From: James Burke Sent: 15-Nov-2011 2:52 PM To: Darren Schnare Subject: Re: [amdjs-api] AMD define() signature augmentation request. (#5)

Actually, by using the 'imports' object we do not rely on the JavaScript environment being able to inspect function bodies.

We tested JS environments and only the PS3 netfront browser and older opera mobile browsers did not return usable function bodies. Those environments really need built resources (which can have their dependencies parsed out, as the requirejs optimizer does), so this seemed like a workable compromise, and it allowed for easier CommonJS module wrapping.


Reply to this email directly or view it on GitHub: https://github.com/amdjs/amdjs-api/issues/5#issuecomment-2750185

unscriptable commented 12 years ago
define({
    a: 'a', 
    b: 'b'
}, function(imports) {
    return {
       /* imports.a and imports.b are the module values from loading 'a' and 'b' */
       /* module properties */
    };
});

I really like the way that the variables and module ids line up with this syntax. Kyle Simpson was leaning towards something similar with his mLAB project.

There are a few reasons I hesitate though:

1) we are already overloading the define() and require() functions heavily. 2) we will be straying even farther away from compatibility with commonjs modules

-- John

dschnare commented 12 years ago

John,

I guess I haven't been giving too much thought to providing backward compatibility support for CommonJS lately. Although I agree that having the name of the variable a dependency is 'bound' to is nice to look at when reading modules, not to mention it keeps the module factory signature really small and consistent. I'm extremely interested in uniting what is practical in web application development and the set of CommonJS specifications.

Although, I'm not fully convinced that the CommonJS specifications are what ought to be followed for the future of web development, since it seems that no consideration for web development restrictions were fully considered when writing the CommonJS specifications (although I could be wrong since I am not as intimately familiar with CommonJS as, say one of the specification contributors).

I've taken the liberty of implementing the 'import' signatures as an addendum to AMD in 'definejs' at ( https://github.com/dschnare/definejs ). This implementation adheres to the AMD signatures for 'define()' in addition to supporting the suggested 'imports-as-objects' signatures.

NOTE: Since the full AMD specification is supported in definejs (minus plugins for various reasons -- see the README on github for definejs), computability with CommonJS is fully supported as well. By supporting the 'import' object we are only accommodating JavaScript developers that do not choose to adhere to CommonJS (for their own reasons). In fact, implementing the 'import' object support really wasn't a big issue, and didn't actually introduce much 'surface area' to testing as one might think, since the creation of dependencies is abstracted to a common interface.

I'll be pondering this feedback for some time before I decide to fully commit to it.

I appreciate your feedback John. Kudos to the AMD movement.

On Tue, Nov 15, 2011 at 9:45 PM, John Hann < reply@reply.github.com

wrote:

define({ a: 'a', b: 'b' }, function(imports) { return { /* imports.a and imports.b are the module values from loading 'a' and 'b' / / module properties */ }; });

I really like the way that the variables and module ids line up with this syntax. Kyle Simpson was leaning towards something similar with his mLAB project.

There are a few reasons I hesitate though:

1) we are already overloading the define() and require() functions heavily. 2) we will be straying even farther away from compatibility with commonjs modules

-- John


Reply to this email directly or view it on GitHub: https://github.com/amdjs/amdjs-api/issues/5#issuecomment-2754910

jrburke commented 12 years ago

For me this:

    define({
        a: 'a', 
        b: 'b'
    }, function(imports) {
           imports.a;
           imports.b;
    });

is just not as concise as this:

    define(function (require) {
        var a = require('a'),
            b = require('b');
    });

and the second version has better advantages with jslint. But no more of my ranting on that. :)

unscriptable commented 12 years ago

@jrburke:

Imho, using the object literal notation is clearer. However, the "local require" format separates the "authoring format" from the "transport format" -- and from what I gather from the community, this is a very important point.

@dschnare:

When I speak of compatibility, I'm looking forward to CommonJS Modules/2.0. There are a few ppl in the CJS world that _do_ understand the needs of the browser-side javascript community (they should, front-end devs outnumber server-js devs 200:1). Wes Garland and Christoph Dorn come to mind. In fact, these are the two main drivers of the Modules/2.0 endeavor.

I hope to support CJSM/2.0 in curl.js in the near future (0.7?). BravoJS/PINF already does. If we could get RequireJS (hint hint), as well as RingoJS or Rhino to support it, too, I think we've got enough momentum. (I didn't mention node since they seem the most reluctant to support anything browser-friendly.)

FWIW, I appreciate your creativity. Kudos to you, too!

-- J

dschnare commented 12 years ago

I agree that the second approach, calling require explicitly, is more concise. However, at the end of the day jslint is just a tool. If a particular approach to solving a problem doesn't 'fit' well with a tool, then in my opinion the tool needs work. Anyways, I do understand your opinion. End rant =).

Run with me for a little here; if one of the goals of AMD is to support CommonJS modules and CommonJS philosophies, then perhaps AMD ought to only use the explicit require approach? That is dependencies are completely taken out of the the define() signature. For example:

   // new and only signature:
   // define([module ID], module value | factory function)

   define({
      /* properties */
   });

   define(function(require) {
      var a = require('a'), 
           b = require('b');
   });

   define(function() {
      // no dependencies, just logic
   });

Everything else remains the same, but each factory will have a require() function passed in if there is a require formal parameter in its signature. Behind the scenes we rely on inspecting the factory function body for require() calls as usual. This approach is completely supported already, but I would propose supporting only this approach.

Of course by using only this approach we would be in a pickle with the PS3 browser and legacy Opera mobile browsers like you had mentioned earlier, but would be making it extremely easy for CommonJS developers to use and learn AMD as well as making it clear that CommonJS philosophies ought to be a part of every present and future JavaScript developer's (web or otherwise) vocabulary.

domenic commented 12 years ago

As someone quite active in CommonJS Modules/2.0, I wanted to chime in with some comments on the subject. However, my post quickly expanded to be rather off-topic for this bug, so instead I stuffed it in a gist. If anyone is interested in discussing along those lines, I'd love to do so in the comments over there.

jrburke commented 12 years ago

I am not going to support the api in that commonjs/2.0 draft. The API is too large and over-specified. If there was a commonjs 2.0, it should just be

The folks doing that 2.0 draft want to preserve imperative, synchronous require usage, but that does not map well to ES harmony modules. It is also a bad fit for web-based code. Any computed require that needs imperative require should really be a callback-based require call.

They also wanted to specify "download a module to make it available, but do not execute it until first require('') call". This creates a more verbose callback-require and a more verbose hand coding of a wrapped format like the style used in AMD. It is also not supported in the ES harmony drafts for modules.

If they let go of that desire to preserve that imperative require, and the "download but do not execute", they could adopt the AMD APIs and we'd all be done, and it would fit better with current ES harmony drafts.

jrburke commented 12 years ago

@dschnare the define(function (require){}) is usable now in AMD, but I get enough pushback from "toString() is not supported everywhere" to make it hard to only go that route. Also, the array syntax for define() allows for a fairly compact "universal" boilerplate.