Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

define (['require'], function(require) {}) halts on require('dep') #467

Closed
anodynos opened this Issue · 3 comments

2 participants

@anodynos

I am trying to iron out some semantics of require() on web/requirejs, for my soon to be released AMD-to-UMD converter.

This works fine

define [], (require)->
  console.log 'started a'
  b = require 'b/b' 
  console.log 'got b'
  return a: b

but if I add 'require' on the dependencies

define ['require'], (require)->
  console.log 'started a'
  b = require 'b/b' #if 'require' is present in deps, requirejs halts here
  console.log 'got b'
  return a: b

it halts on the require() call, without any error message.

The problem is I want to add more dependencies on my define [], but still be able to use require() in the commonjs way. Is this a bug ? Using version 2.0.6

PS: I hope you dont mind the coffeescript dialect :-)

@jrburke
Owner

If you use the dependency array, then the factory function is not scanned for require('') calls to pull out dependencies, so in your second example require('b/b') is not found and preloaded/executed.

It is a "one or the other" approach: either list all dependencies in the dependency array, or do not list any, do not even pass an array to the define call, just pass in a function that expects a require argument, and list the dependencies needed as require('stringvalue') calls:

define(function(require){
  require('a');
  require('b');
});

The following works too, but I find it too verbose, having to mention the dependencies twice, once in the dependency array and again inside the factory function with a require('') call:

define(['require', 'a', 'b'], function(require){
  require('a');
  require('b');
});

PS: I prefer getting issues in JS, since I do not know how the coffeescript will be converted to JS, and need to be sure the underlying JS is correctly. If it is a simple thing, I can normally muddle through, but do not be surprised if I ask for the JS version of the code. Put another way, I cannot be expected to know all languages that compile down to JS, and how they are translated to JS.

@jrburke jrburke closed this
@anodynos

@jrburke thank you so much for the thorough explanation.

If I understand it right, this is to enable the optimization process to signal at runtime that no scanning is needed and save execution time. I see that r.js scans each module and if no dependency array exists, it gathers all dep's of require('dep') during the code analysis step and properly builds the dependency array, adding 'require' as the first dependency.

Nevertheless, when developing this can be a rough edge and it can discourage those not aware of the one or the other behaviour, which to me at least was not so obvious.

Do you see fit that this could change in future versions, if the signaling semantics change a bit ? For instance, if the developer has explicitly declared 'require' as the first dependency (along with other deps), then scanning is enabled. Then the optimizer could add a 'require!' as a first dependency to signal the dont-scan optimization.

If you think this is not going to happen, I suggest that at least it get explicitly documented in common errors or elsewhere. Also, shouldn't there be an error message at runtime, such as "module x not yet loaded for context" ?

PS: The good thing about coffeescript, is that its just javascript, less verbose. Unless you use sexy things like comprehensions, destructuring assignments etc, the translation is 1:1. Maybe you should give it a try!

@jrburke
Owner

I do not think we'll do any changes to the AMD spec on this, it would be more about maybe providing error feedback, maybe via tools like xrayquire for this case. I am hopeful that the next big rev for modules will just be ECMAScript harmony modules built into the language.

I did update the error page (see above commit), which will go live with the 2.1 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.