Implement a subset of the AMD #1173
Conversation
…ules/AsynchronousDefinition> the 'id' argument is ignored and only one define call per module is allowed.
|
@isaacs, can you review? I've been slowly convinced that we should do this. |
|
See also #1170 |
|
why? |
|
@Gozala @fjacobs @ry /cc @kriskowal I've got an implementation in my "AMD" branch, which provides compatibility with the other AMD implementations out there, without getting into the underspecified semantics of the id and dependencies arguments. Also, it works with NODE_MODULE_CONTEXTS mode, and passes all tests. I still need to add some more tests and docs to explain and validate the behavior, but in my cursory poking, this seems to work pretty good and deliver what we want. Here are the relevant commits so far: isaacs/node@861ef0a The goal of this should be to be compatible with modules written for AMD systems, while adding the minimum necessary API surface to node. Towards that end, I think that doing anything special with the id or deps arguments is misguided. Only the last argument matters for node's purposes. Since require() is synchronous anyway, there is no advantage to pre-loading the dependencies. (Clearly, for systems that load modules asynchronously, this is a much more relevant point.) |
|
is this going to be opt-in or will all node modules be shifting to this api? |
|
Strictly opt-in. The goal is simply for modules written for AMD to not have to sniff for the presence of a define function and add boilerplate. I am strongly opposed to anything that requires a wholesale rewrite of every node module. |
|
+1 for opt-in |
|
Really, it should only be a problem if you previously depended on a global thing called |
|
fewf, then im +1 in general |
|
This is meant at purely opt-in to make client server code sharing easier On Sunday, June 12, 2011, visionmedia
|
|
Forcing an implementation of only define(function(require, module, exports) {}) causes more problems than it's worth. This is supposed to assist people wanting to share their client/server code and it only ends up hindering them. |
|
@bmavity That is not true, all the current implementations support that, it's actually sweet spot that works across many diff implementations. Also I don't think more than that is necessary, as it will raise another wave of debates. |
|
if you know before-hand which code will be used in the page I think there is little reason to use something like this, but I guess for lazy loaded stuff it's fine |
|
I could be missing something. How would you do dojo-style loading with this now? If you can, that's fine. But if you can't, you're now unnecessarily limiting browser code to using sync require or some ugly ass futures or whatever. There's a way to code this where any of the styles are supported. Since node.js theoretically doesn't care about browser code, they shouldn't be choosing what style "wins" |
|
That is a subset of AMD spec, that is well supported by requirejs, which is also known to be working with dojo: http://dojotoolkit.org/reference-guide/releasenotes/1.6.html Also I think it's better to discuss implementation details in the AMD mailing list. Node does care about developers though who want to write modules that can be loaded without hazards both in browser and node. Also, I think it's better to take this discussion to the mailing list: https://groups.google.com/forum/#!topic/nodejs-dev/DJz3OYGRnmg |
|
Yeah, I figured this wasn't a good place for this. Sorry about that. :) |
|
@isaacs lgtm +1 |
Implement a subset of the AMD spec http://wiki.commonjs.org/wiki/Modules/AsynchronousDefinition
the 'id' argument is ignored and only one define call
per module is allowed.