Better fix for requirejs/r.js#591: introduce nodeIdCompat for more co…

…mprehensive fix
requirejs · Jan 8, 2014 · 96a39ca · 96a39ca
1 parent 87cdf49
commit 96a39ca
Show file tree

Hide file tree

Showing 10 changed files with 61 additions and 6 deletions.
diff --git a/docs/api.html b/docs/api.html
@@ -795,6 +795,8 @@ <h2>
 
 <p id="config-packages"><strong><a href="#config-packages">packages</a></strong>: configures loading modules from CommonJS packages. See the <a href="#packages">packages topic</a> for more information.</p>
 
+<p id="config-nodeIdCompat"><strong><a href="#config-nodeIdCompat">nodeIdCompat</a></strong>: Node treats module ID <code>example.js</code> and <code>example</code> the same. By default these are two different IDs in RequireJS. If you end up using modules installed from npm, then you may need to set this config value to <code>true</code> to avoid resolution issues.</p>
+
 <p id="config-waitSeconds"><strong><a href="#config-waitSeconds">waitSeconds</a></strong>: The number of seconds to wait before giving up on loading a script. Setting it to 0 disables the timeout. The default is 7 seconds.</p>
 
 <p id="config-context"><strong><a href="#config-context">context</a></strong>: A name to give to a loading context. This allows require.js to load multiple versions of modules in a page, as long as each top-level require call specifies a unique context string. To use it correctly, see the <a href="#multiversion">Multiversion Support</a> section.</p>

diff --git a/require.js b/require.js
@@ -288,12 +288,11 @@ var requirejs, require, define;
                     name = name.split('/');
                     lastIndex = name.length - 1;
 
-                    // If inside a package and name ends in .js, strip
-                    // it out, because of node. Have to do this here, and
-                    // not in nameToUrl because node allows either .js or
-                    // non .js to map to same file.
-                    if (getOwn(config.pkgs, normalizedBaseParts[0]) &&
-                        jsSuffixRegExp.test(name[lastIndex])) {
+                    // If wanting node ID compatibility, strip .js from end
+                    // of IDs. Have to do this here, and not in nameToUrl
+                    // because node allows either .js or non .js to map
+                    // to same file.
+                    if (config.nodeIdCompat && jsSuffixRegExp.test(name[lastIndex])) {
                         name[lastIndex] = name[lastIndex].replace(jsSuffixRegExp, '');
                     }
 

diff --git a/tests/all.js b/tests/all.js
@@ -29,6 +29,7 @@ if (hasToString) {
     doh.registerUrl("packagesConfig", "../packages/config/config.html");
     doh.registerUrl("packagesNestedMain", "../packagesNestedMain/packagesNestedMain.html");
     doh.registerUrl("packagesNode", "../packagesNode/packagesNode.html");
+    doh.registerUrl("packagesNodeAdapter", "../packagesNodeAdapter/packagesNodeAdapter.html");
     doh.registerUrl("specialDeps", "../specialDeps/specialDeps.html");
 }
 

diff --git a/tests/packagesNode/packagesNode-tests.js b/tests/packagesNode/packagesNode-tests.js
@@ -1,4 +1,5 @@
 require({
+    nodeIdCompat: true,
     packages: [{
         name: 'foo',
         location: 'node_modules/foo',

diff --git a/tests/packagesNodeAdapter/node_modules/foo.js b/tests/packagesNodeAdapter/node_modules/foo.js
diff --git a/tests/packagesNodeAdapter/node_modules/foo/lib/bar.js b/tests/packagesNodeAdapter/node_modules/foo/lib/bar.js
diff --git a/tests/packagesNodeAdapter/node_modules/foo/lib/baz.js b/tests/packagesNodeAdapter/node_modules/foo/lib/baz.js
diff --git a/tests/packagesNodeAdapter/node_modules/foo/lib/index.js b/tests/packagesNodeAdapter/node_modules/foo/lib/index.js
diff --git a/tests/packagesNodeAdapter/packagesNodeAdapter-tests.js b/tests/packagesNodeAdapter/packagesNodeAdapter-tests.js
@@ -0,0 +1,18 @@
+require({
+    nodeIdCompat: true,
+    baseUrl: 'node_modules'
+}, ['foo'], function (foo) {
+
+    doh.register(
+        'packagesNodeAdapter',
+        [
+            function packagesNodeAdapter(t){
+                t.is('foo', foo.name);
+                t.is('bar', foo.bar.name);
+                t.is('baz', foo.baz.name);
+                t.is('bar', foo.baz.bar.name);
+            }
+        ]
+    );
+    doh.run();
+});
diff --git a/tests/packagesNodeAdapter/packagesNodeAdapter.html b/tests/packagesNodeAdapter/packagesNodeAdapter.html
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>require.js: Packages Node-Style Test</title>
+    <script type="text/javascript" src="../../require.js"></script>
+    <script type="text/javascript" src="../doh/runner.js"></script>
+    <script type="text/javascript" src="../doh/_browserRunner.js"></script>
+    <script type="text/javascript" src="packagesNodeAdapter-tests.js"></script>
+</head>
+<body>
+    <h1>require.js: Packages Node-Style Test</h1>
+    <p>Test package that uses ".js" node-style module ID references inside
+        a package. More info:
+    <a href="https://github.com/jrburke/r.js/pull/591">r.js 591</a></p>
+    <p>Check console for messages</p>
+</body>
+</html>