main.js should have a `define` instead (or in addition to) `require` #26

Closed
brianmhunt opened this Issue Jul 31, 2012 · 5 comments

2 participants

@brianmhunt

The example code can result in a race condition whereby require does not work as designed.

Here is a post on StackOverflow detailing the issue. Basically the order demanded by require() was not being honoured when main.js (or its equivalent) uses require. In other words, require(['main'], code) did not load the dependencies of main before calling code.

To solve the problem in the example, unless I misunderstand something, main.js should essentially become:

require.config({
  paths: {
     cs: '../../cs',
    'coffee-script': '../../coffee-script'
  }
});

define(['cs!csmain']);

I hope this is a helpful suggestion/comment. I certainly spent quite a lot of time tracking down the problem. :)

@jrburke
requirejs member

Just to confirm, this is with requirejs 2.0.4 and after using the 2.0.4 r.js optimizer?

I will want to set up a test case, but require() should work in that context. In particular, define() does not seem like the right choice. It is either a misconfiguration or a bug in require.js. I will post back more when I have a test up and running.

If you have a link to a test that I can try directly with the failure, that would be even better, but no worries if that is not possible.

@brianmhunt

Thanks for the quick reply. I am using requirejs 2.0.4, and the problem occurs only when using the r.js optimizer.

I unfortunately do not have a test case, but I imagine a naive setup to be along the lines of:

test.html

<!doctype html> 
<html>
  <head>
    <script src="require-jquery.js"></script>
    <script>
      require.config({
        paths: {
          "main": "main-built", // <--- the 'optimized' result of `$ r.js build.js`
        }
      });
      alert("Hey");
      require(["main"], function () {
        alert("Running the inline function that requires main");
        // this executes without any regard to whether 'main' is loaded.
        // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        // also:
        // require('cs!csmain') throws an exception because "cs!csmain" has not been
        // loaded for context: '_'.
      });
    </script>
  </head>
  <body>Test of the script.</body>
</html>

main.js

define(['cs!csmain']);

csmain.coffee

define [], -> 
  alert "The csmain has been loaded."
  return {}

Then running a simple build script to produce main-built.js.

A well-placed sleep or putting the loading at the end of the HTML may help reproduce the problem- I am not sure.

Thanks - I hope the above is helpful, and I look forward to your thoughts on this.

Brian

@jrburke
requirejs member

Ah sorry, yes I misunderstood, previously.

By doing this in the HTML file:

require(['main'], function () {
   //I want this to not be called until anything that main depends on is loaded
});

If main.js does not call define, just uses require, then the module loader treats main.js's module value as just define('main', {}); and does not wait for the require(['main']); call in main.js to finish. So, to properly set up the relationships, and so that any require() call on 'main' waits for main and its dependencies to load, you are correct, main.js needs to use define().

So this is working as designed. Sorry I got confused when I parsed the example.

@jrburke jrburke closed this Aug 7, 2012
@jrburke
requirejs member

The trick is that normally the HTML page does not need to do the inline require and wait for it to load. Normally people just use data-main="main" and have all the main logic inside that file.

@brianmhunt

Thanks for looking at this issue. It's a relief to know that it's working as designed/expected. I just posted the issue to make sure that (a) it wasn't a broader issue (unlikely) and (b) I was using the library correctly (distinctly possible).

I would give a really good bottle of scotch to be able to do what people normally do ... :)

Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment