amdjs / amdjs-api

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

Wrong example for "global require"? #12

Closed tlindig closed 11 years ago

tlindig commented 11 years ago

The example code for global require looks wrong for me:

https://github.com/amdjs/amdjs-api/wiki/require#requirearray-function-

define(function (require) {
      require(['a', 'b'], function (a, b) {
          //modules a and b are now available for use.
      });
  });

If I am right, the require inside the factory function is the local require (got it as argument) and not the global require. Example should be something like this:

define(function (require) {
          // local require
          var a = require('a'); 
});

// global require
require(['a', 'b'], function (a, b) {
          //modules a and b are now available for use.
});
unscriptable commented 11 years ago

Hey @tlindig,

Indeed, the code snippet is for the local require. This entire page describes the behavior of the local require. By extension, it also describes some of the behavior of the optional global require. I think this is clear if the reader has read the section, "Local vs Global require", no?

I am closing this issue since the code snippet is correct as is. However, if after re-reading this page you still feel that the difference between global and local require is insufficient, maybe you could open another issue?

Regards,

-- John

tlindig commented 11 years ago

Indeed, the code snippet is for the local require.

If so, it is very confusing me. A local require is described here: http://wiki.commonjs.org/wiki/Modules/1.1.1#Require That function has one parameter, a string, and synchronously return. Thats fine and clear.

But now you say, that the asynchronously version, with array and callback can also be used in the factory function as local require? If so, I can not understand, how you will be able to guarantee that the return exports is consistent. The factory function is synchronously [https://github.com/amdjs/amdjs-api/wiki/AMD#factory-], isn't it? define will take the returned value or the exports as the module exports. But if you call the asynchronously version of require inside this factory function, the callback you gave to the require, can mutate the module instance (if not, the callback has no aim, isn't it?) after the factory function finished his processing and returned the module instance. And now you can get inconsistent module instance/exports.

So, how you will handle this?

jrburke commented 11 years ago

Lots of things can mutate the module export value after the value is first given to the loader, not just the async callback in the require. This is a useful and desirable feature. For instance, a Model module that fetches data from the server and tracks adds/deletes. Modules are not mandated to be frozen. If that is desired, a module can do that on a case by case basis via ES5 APIs like seal and freeze. This will also be the case in ECMAScript modules, they will not be sealed/frozen by default.

unscriptable commented 11 years ago

Hi @tlindig,

While @jrburke and the original AMD authors did a great job assimilating the CommonJS specifications into AMD, there are plenty of things that CommonJS did not do well (and still does not do well) in an async environment. The async local require, as James states, is necessary for some tasks such as dynamically loading more modules some time after the factory has run.

Just to nitpick, the require as defined by CommonJS is not called the "local require", it is called the "scoped require" or the "free require". Also, imho it is stated quite clearly at the top of the AMD require page that the require in AMD differs from CJS -- and why. Is there a better way we should illustrate that CommonJS require != AMD require?

So, how you will handle this?

We cannot protect the developer from every mistake they may make. This is true of nearly any language and any environment. We all try to set forth examples that minimize misunderstandings and mistakes.

It's not even clear to me that this is a widespread problem. In my experience, mutable exports has never, ever been a problem.

Regards,

-- John

hallettj commented 11 years ago

I shared Tobias' confusion. It seems to me that the problem is that the AMD spec describes two require functions but is somewhat vague about whether the behavior of the global require is also implemented by the scoped require. The example in question seems to indicate that it is. The text of the specification should be more clear about that.

On Tue, May 14, 2013 at 12:00 PM, John Hann notifications@github.comwrote:

Hi @tlindig https://github.com/tlindig,

While @jrburke https://github.com/jrburke and the original AMD authors did a great job assimilating the CommonJS specifications into AMD, there are plenty of things that CommonJS did not do well (and still does not do well) in an async environment. The async local require, as James states, is necessary for some tasks such as dynamically loading more modules some time after the factory has run.

Just to nitpick, the require as defined by CommonJS is not called the "local require", it is called the "scoped require" or the "free require". Also, imho it is stated quite clearly at the top of the AMD require pagehttps://github.com/amdjs/amdjs-api/wiki/requirethat the require in AMD differs from CJS -- and why. Is there a better way we should illustrate that CommonJS require != AMD require?

So, how you will handle this?

We cannot protect the developer from every mistake they may make. This is true of nearly any language and any environment. We all try to set forth examples that minimize misunderstandings and mistakes.

It's not even clear to me that this is a widespread problem. In my experience, mutable exports has never, ever been a problem.

Regards,

-- John

— Reply to this email directly or view it on GitHubhttps://github.com/amdjs/amdjs-api/issues/12#issuecomment-17897511 .

unscriptable commented 11 years ago

On May 14, 2013, at 7:00 PM, Jesse Hallett notifications@github.com wrote:

I shared Tobias' confusion. It seems to me that the problem is that the AMD spec describes two require functions but is somewhat vague about whether the behavior of the global require is also implemented by the scoped require.

This line seems clear to me: "An implementation is not required to implement a global require, but if it does, the behavior of global require is similar to the behavior of the local require() function, with the following qualifications..."

Is that bit clear? Maybe we need to do something to make that line easier to find in the spec? Help us out here, please. :)

-- John

The example in question seems to indicate that it is. The text of the specification should be more clear about that.

On Tue, May 14, 2013 at 12:00 PM, John Hann notifications@github.comwrote:

Hi @tlindig https://github.com/tlindig,

While @jrburke https://github.com/jrburke and the original AMD authors did a great job assimilating the CommonJS specifications into AMD, there are plenty of things that CommonJS did not do well (and still does not do well) in an async environment. The async local require, as James states, is necessary for some tasks such as dynamically loading more modules some time after the factory has run.

Just to nitpick, the require as defined by CommonJS is not called the "local require", it is called the "scoped require" or the "free require". Also, imho it is stated quite clearly at the top of the AMD require page< https://github.com/amdjs/amdjs-api/wiki/require>that the require in AMD differs from CJS -- and why. Is there a better way we should illustrate that CommonJS require != AMD require?

So, how you will handle this?

We cannot protect the developer from every mistake they may make. This is true of nearly any language and any environment. We all try to set forth examples that minimize misunderstandings and mistakes.

It's not even clear to me that this is a widespread problem. In my experience, mutable exports has never, ever been a problem.

Regards,

-- John

— Reply to this email directly or view it on GitHub< https://github.com/amdjs/amdjs-api/issues/12#issuecomment-17897511> .

— Reply to this email directly or view it on GitHubhttps://github.com/amdjs/amdjs-api/issues/12#issuecomment-17910820 .

tlindig commented 11 years ago

Hi All,

thank you guys that you have taken the time to explain to me the facts.

James said:

Lots of things can mutate the module export value after the value is first given to the loader, not just the async callback in the require. This is a useful and desirable feature. For instance, a Model module that fetches data from the server and tracks adds/deletes. Modules are not mandated to be frozen.

Sorry, I think I did not write clear what I like to say. Yes, modules shall change there state in there live. To be sure we are talking the same. I discussing about the local require method you only have inside the factory method and if someone need inside this function a asynchronous require. IMHO the factory method is not the right place to fetches data from the server and tracks adds/deletes. After the factory method is done, I would expect to get always an usable instance of the module or an exception.

John said:

The async local require, as James states, is necessary for some tasks such as dynamically loading more modules some time after the factory has run.

And that loading of more modules you like to start in the one time running factory method? If they are necessary for your module, I think you should wait until you get them and so you should load it synchronous. If they are optional, you may could load them as recently as they are needed, but not in the factory.

John said:

We cannot protect the developer from every mistake they may make. This is true of nearly any language and any environment. We all try to set forth examples that minimize misunderstandings and mistakes.

Thats clear. But it would be nice to give developers a chance to write code that is save, to have some guaranteed commitments.

IMHO, one of such a commitment would be a way to get in every case a usable module from the require or an exception. But with an asynchronously require inside a synchronously called factory callback, I can not see this way. May be the result of a require/factory should be a promise ( http://wiki.commonjs.org/wiki/Promises ) so the user of a module can wait, until the module factory is ready.

Thanks again for your patience. May be here is not the right place for discussion about pro and contra. I see you have a group for that: https://groups.google.com/group/amd-implement.

Regards,

Tobias

jrburke commented 11 years ago

Maybe the disconnect is the following: the async local require is not about setting up the actual module exports, but usually used to fetch code based on calculated dependencies:

define(function (require) {
  return {
    nextView: function (viewId, callback, errback) {
      require([viewId], function (viewModule) {
        //Do some internal bookkeeping then call callback
        callback(viewModule);
      }, errback);
    }
  }
});

You could do that newView API with promises too, completely fine.

The larger point is that calculated dependencies (things that cannot be expressed as string literal dependency IDs) can never be a direct module dependency, because their state cannot be known ahead of time, to be fetched before executing the module factory function.

This is actually one of the big things that caused the split between AMD and CommonJS (and how node uses modules). They did not recognize that calculated dependencies needed a callback-based require call (some on the commonjs list saw the need, but nothing was standardized). Modules systems based on synchronous ,local file IO normally do not see the problem. That is not the case on the web though.

Note that promises do not help with issue -- it is about the loader being able to statically analyze dependencies and have them locally available before execution starts. If the ID for a module cannot be statically determined (not a string literal), then a callback-based require is needed to fetch that module, as it may not be locally available.