Skip to content
This repository

Config paths not checked if id matches jsExtRegExp #200

Closed
absoludity opened this Issue March 05, 2012 · 5 comments

2 participants

Michael Nelson James Burke
Michael Nelson

As per line:
https://github.com/jrburke/requirejs/blob/master/require.js#L1483

the require config is not even checked if the given id is something like 'expect.js'. Looking at the code, I don't see a reason why the order couldn't be changed here (ie. only check the jsExtRegExp if a path isn't defined in the config)?

Note: I'm only hitting this issue because I want to run my tests - which use expect.js - in both the browser and in node. The shim I'm using for expect.js in the browser (as expect.js isn't using amd) can't be used in node (at least, I can't find expect anywhere in the shim function when running from nodejs:

define(["expect_js_orig"], function() {
    debugger
    return window.expect;                                                                                                      
});

as expect.js only exports itself if it detects module/export, which leaves me currently in the position that when running my tests via nodejs, I need to let nodejs' require load the module, which means I can't specify a path at all in my nodejs runner config, while in my browser config, need to specify a path for the id 'expect.js' which breaks as per #L1483 above. Or am I missing something obvious? (likely).

Thanks for the great work!

James Burke
Owner

In AMD module IDs should not include a ".js" suffix as it is added according to a loader config. RequireJS allows loading non-AMD scripts using "web page rules" and uses this convention (as you pointed to in your require.js link):

  • an ID that has a protocol in it
  • It ends in ".js"

These indicate to requirejs that "web page loading rules" should be used -- mainly that the IDs are urls and if they are relative, are relative to the document, not something that uses the requirejs config for module names.

I'm unclear why this is a problem for using expect.js. It seems the bigger problem is that expect.js will not export a value unless "module" and "exports" is available in the JS environment. In that case, you will want to load it with Node's require, which requirejs via r.js in Node delegates to if it cannot find the module with its config.

So maybe more info on the project setup would be useful to figure out how to fix the problem, and maybe a link to the expect.js you are using too.

Michael Nelson

Hi James,

Yes, the real issues are that:

  • expect.js isn't exporting anything unless window/module/exports are defined, so although I can use it fine with requirejs in the browser (using a shim or use plugin), I am not able to use expect.js via requirejs in node (I tried the latest from https://github.com/LearnBoost/expect.js as a standalone script and 0.1.2 as a node module), unless I remove any requirejs config path for it and install the expect.js node module.
  • Node packages the module as node_modules/expect.js/expect.js, and so I need to require(['expect.js'],...) (ie. that's not the file extension, but the node module directory name). So if I want my test with that require(['expect.js'], ...) to work both in the browser and in nodejs, I need it to work in the browser with the name 'expect.js' - as per the bug.

But that's not your issue - and if there's a reason why you're matching jsExtRegExp before checking the paths, that's fine, close this off. I can work around this issue at the moment by ln -s node_modules/expect.js/ node_modules/expect_js/ and requiring that instead. The real solution would be for expect.js to be AMD compat., or be loadable with a shim without the nodejs loader I guess. Thanks!

James Burke
Owner

Ouch, yeah, naming the project 'expect.js' is awkward, sorry I did not fully understand that part. I'm not sure yet how to deal with that. I'm actually not sure that allowing 'browser url rules' are so great, particularly the '.js' rule, so I'm open to thinking about a different way to do that code. I'll put this in for consideration for 1.1, but not sure how it will shake out yet.

On a side note:

You can use volo if you want a command line tool that can add a define() wrapper to a JS file that is written in node style. volo amdify is the command to run.

James Burke
Owner
jrburke commented May 19, 2012

For 2.0 I will not be changing the ".js" resolution rules. Looking more at expect.js, it looks like if you have it at lib/expect.js/, then the actual dependency is at lib/expect.js/expect.js, so if lib is the baseurl, then the AMD module ID can just be "expects.js/expect". I also think that will work in Node.

James Burke jrburke closed this May 19, 2012
Michael Nelson

Yes, that makes sense. Thanks James.

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.