New amdjs-tests official, all current frameworks passing + Spec Discussion

[Jakob Heuser]

First off, a huge thanks to the Richard Backhouse. We have lsjs working in the new test suite, and we've uncovered a questions around the spec I feel might be worth getting solid answers on.

1. config() must be ran before the initial go() invocation.

Some lsjs tests were timing out due to the config() call occuring after the first go() invocation. As highlighted in another thread in the forum, not all AMD loaders rely on location.href as a fallback.

AMD JS Implication: the global require(Array, cb) should NOT run if the framework is not configured yet, and should throw a helpful error for the developer. This may just be an update to the specification.

2. nesting require statements - allowed?

Should people consider the following code "valid"

go(["require", "foo"], function(require, foo) {

  require(["require", "bar"], function(require, bar) {

    require(["require", "baz"], function(require, baz) {

      // etc...

    });

  });

});

AMD JS Implication: we don't really have many tests that exercise this use case. For now, we've made _reporter a top level module. The basic_require test exercises this somewhat, but circular dependencies are more than likely a missed use case at the moment. For that matter, I'm not even sure this is a pattern we want to be endorsing.

3. One does not simply nest global requires...

go(["foo"], function(foo) {

  go(["bar"], function(bar) {

    // this is bad

  });

});

AMD JS Implication: The AMD spec is pretty vague about how the global require() should work if its exposed. We may want to note that someone implementing code for multiple loaders should only call the global entry point once. Again, I'm not even sure this is a pattern we want to endorse.

--Jakob

[James Burke]

Thanks for getting this in! Notes inline:

On Fri, Mar 15, 2013 at 12:32 PM, Jakob Heuser <ja...@felocity.com> wrote:
> First off, a huge thanks to the Richard Backhouse. We have lsjs working in
> the new test suite, and we've uncovered a questions around the spec I feel
> might be worth getting solid answers on.
>
> 1. config() must be ran before the initial go() invocation.
> Some lsjs tests were timing out due to the config() call occuring after the
> first go() invocation. As highlighted in another thread in the forum, not
> all AMD loaders rely on location.href as a fallback.
>
> AMD JS Implication: the global require(Array, cb) should NOT run if the
> framework is not configured yet, and should throw a helpful error for the
> developer. This may just be an update to the specification.

This seems to be a per-loader choice (have enough defaults that
require() will run before a config call or not), not sure we need to
say anything about it in the spec. For the tests, it seems fine to
allow for running a config() call first.

> 2. nesting require statements - allowed?
> Should people consider the following code "valid"
> go(["require", "foo"], function(require, foo) {
> require(["require", "bar"], function(require, bar) {
> require(["require", "baz"], function(require, baz) {
> // etc...
> });
> });
> });
>
> AMD JS Implication: we don't really have many tests that exercise this use
> case. For now, we've made _reporter a top level module. The basic_require
> test exercises this somewhat, but circular dependencies are more than likely
> a missed use case at the moment. For that matter, I'm not even sure this is
> a pattern we want to be endorsing.

Nested require calls seem fine to allow. In some cases, it can be
useful -- doing a built layer load, doing some other env check, then
doing another layer load. Asking for "require" in the nested require
calls seems excessive -- the user could get by without them, but I
would not expect it to cause harm.

> 3. One does not simply nest global requires...
> go(["foo"], function(foo) {
> go(["bar"], function(bar) {
> // this is bad
> });
> });
>
> AMD JS Implication: The AMD spec is pretty vague about how the global
> require() should work if its exposed. We may want to note that someone
> implementing code for multiple loaders should only call the global entry
> point once. Again, I'm not even sure this is a pattern we want to endorse.

This pattern has shown up in the real world, even used it for an
example project:
https://github.com/requirejs/example-multipage/blob/master/www/js/page1.js

I think there is better performance by parallelizing the load, but
that requires careful crafting of layers for it to work before and
after a build, where the nested call is usually easier to understand
and get right.

I am curious to know other people's opinion on those patterns though.

James

[John Hann]

Hey Jakob, nice work!

I'm rewriting curl.js on top of Promises/A+ promises and should add it to the suite as a verification that it works correctly. :)

> 1. config() must be ran before the initial go() invocation.

> Some lsjs tests were timing out due to the config() call occuring after the
> first go() invocation. As highlighted in another thread in the forum, not
> all AMD loaders rely on location.href as a fallback.

I'm not sure I understand the relationship to go()-before-config() and location.href (and I haven't read the code, either).  Is the call to config() setting paths, packages, etc that affect the loading of modules in the first go() invocation?  If so, this would be problematic, wouldn't it???  

> 2. nesting require statements - allowed?

> Should people consider the following code "valid"
> go(["require", "foo"], function(require, foo) {
>   require(["require", "bar"], function(require, bar) {
>     require(["require", "baz"], function(require, baz) {
>       // etc...
>     });
>   });
> });

I agree with James.  People are using this to load bundles just-in-time (although, not exactly like you've shown :) ).

> 3. One does not simply nest global requires...

> go(["foo"], function(foo) {
>   go(["bar"], function(bar) {
>     // this is bad
>   });
> });

I agree that the typical user should call this exactly once.  However, I've encountered quite a few teams who have multiple departments that each build their own mini-app and another team composes them into combo HTML pages.  (GoDaddy and Disney, for instance)

However, I strongly believe that the case you've shown is probably a coding error.  There is absolutely no use case that mandates that global loader calls be nested.  The local require could be used -- and should be used since it's standardized.  

I am not sure there's a strong enough case to say this scenario should be forbidden in the spec.  But if we were to add language that described this as an anti-pattern, I would be in favor. :)

-- John

--
You received this message because you are subscribed to the Google Groups "amd-implement" group.
To unsubscribe from this group and stop receiving emails from it, send an email to amd-implemen...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Jakob Heuser]

#1 (go-before-config): Usually, it's not a problem. However, if you have ran go(), are now using a local require(), and then call the global config(), what's the expectation? If the framework is creating a local "require" context, it may not be aware that the global configuration has been changed. I don't know why someone would want to change the global config mid-loading, but I think the simplicity of the test's configure-then-run makes a bit more sense than partial_load-then-configure-then-run setup. However, that may be something for the config specification.

#2 (nesting): Since we see some examples of this in the real world, we should consider adding a few more nested tests. I'll see if I can pull some examples together over the next week in the _circular packages that ensure require() nesting is working cleanly.

#3 (nesting globals): lsjs actually implements calling the global require() only once in a loading chain. I actually think (having written it both ways now) that nested go() is an anti-pattern, and developers should be using the local require instead. The reason? Global requires are framework specific, while the local require() has a specific API that is universal across all loaders.

[Richard Backhouse]

#1 If go-before-config is to be supported then it might be worth the spec providing some guidance on what configuration options are modifiable after the initial entrypoint has been called.

#3 I've never been a big fan of the toplevel entrypoint to an AMD loader being called "require". I still deal with confusion over the use of the global vs local require and what they mean. To me its important that if developers use nested global requires with a particular loader then they understand that portability with other loaders could be an issue.

Richard

[Brian Cray]

1. Things that "just work" get more traction. Configuration adds support for certain use cases of course, but for most cases the spec should encourage loaders to work out of the box.

2. I could see conditions where, based on the an expression in required module a, that a user would then need to nest require module b. The pattern is fairly common with ajax calls.

[Jakob Heuser]

Thanks everyone for the feedback.

Around #1 config-before-go I think for the tests that will be the norm (responsible) thing to do. Doesn't require any changes at this point, but libraries that support config mid-run may want tests to account for this "feature"

Regarding #2, we will want some deep-nesting tests. I'll work on and proposed some in a pull request.

Lastly on #3, for module loaders who are providing the global require under the window.require namespace, we should have a rule around how the following code should be handled:

require(['top_level_module'], function(top_level) {

  require(['second_level_module'], function(second_level) {

    // note that top_level_module did not request "require"... so who's require was called?

  });

});

--Jakob

--
You received this message because you are subscribed to a topic in the Google Groups "amd-implement" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/amd-implement/Rt_g2cNTeEI/unsubscribe?hl=en.
To unsubscribe from this group and all its topics, send an email to amd-implemen...@googlegroups.com.

[John Hann]

On Mon, Mar 18, 2013 at 8:38 PM, Jakob Heuser <ja...@felocity.com> wrote:

Thanks everyone for the feedback.

Around #1 config-before-go I think for the tests that will be the norm (responsible) thing to do. Doesn't require any changes at this point, but libraries that support config mid-run may want tests to account for this "feature"

I imagine each AMD tool has its own test suite. :)

 

Lastly on #3, for module loaders who are providing the global require under the window.require namespace, we should have a rule around how the following code should be handled:

require(['top_level_module'], function(top_level) {

  require(['second_level_module'], function(second_level) {

    // note that top_level_module did not request "require"... so who's require was called?

  });

});

Since "global require" is not universal, then I think this is something that each AMD tool lib can test in its own test suite.  Imho, the test suite should just test for compliance with the spec.

(Minor correction: you wrote: "note that top_level_module did not request `require`".  It's actually the top level `require` that needs to request `require`, not the module.  Sorry, I've been spending way too much time dealing with newb confusion lately. :) )

-- J

[Guy Bedford]

I just wanted to add here that I often use a global require in an internal require context. I really don't see an issue with this. Sometimes one wants a global require, not a contextual require.

[Richard Backhouse]

Guy,

Can to give more detail on what you mean by "Sometimes one wants a global require, not a contextual require"

Thanks
Richard

--

You received this message because you are subscribed to the Google Groups "amd-implement" group.

To unsubscribe from this group and stop receiving emails from it, send an email to amd-implemen...@googlegroups.com.

[Guy Bedford]

The contextual require inside a module contains maps specific to that module. For example with a plugin or framework module, one often doesn't want these specific maps but the global require function. So I see no reason not to use the global require as a global variable in this case.

[Richard Backhouse]

I'm sorry I'm not following you on the maps that are contained in the local require. Is this specific to a specific implementation ?

Thanks
Richard

[Guy Bedford]

Module-specific map configuration is in RequireJS.

Another example I came across today is that if the initial baseUrl is relative (starts with './') then require('./some-module.js') will be a very different result to require('./some-module.js'); in the base context.

Yes these are edge cases, but they are quite important edge cases, and for that reason I think it is actually good practise to use the global require function when intending that the require being made is a global require.

In other words, please don't write a spec that says that this is erroneous, as I would need to change large portions of code that would struggle to solve the issues another way currently.

[Guy Bedford]

To be more specific here, this is particularly for plugins, which need access to the global require space.

[John Hann]

Hey Guy,

It seems we're a bit off topic. :)

We weren't suggesting that a global `require` be forbidden.  However, I do believe that using a global `require` within a module is an anti-pattern.  If your custom plugins truly must resolve a url from the baseUrl, then, sure, that might be a valid edge case.  However, it's not a "best practice" for 99.9% of use cases.

Regards,

-- John

[Guy Bedford]

Hi John,

Sorry, didn't mean to pollute the thread. I just noticed it was touched on in this thread and was hoping this wasn't being implemented into an external testing system too strictly.

Thanks for clarifying!