curl.js loads wrong module if used with paths and all modules define in one file

cujojs / curl

curl.js is small, fast, extensible module loader that handles AMD, CommonJS Modules/1.1, CSS, HTML/text, and legacy scripts.

https://github.com/cujojs/curl/wiki

Other

1.89k stars 216 forks source link

curl.js loads wrong module if used with paths and all modules define in one file #145

Closed fafhrd91 closed 11 years ago

fafhrd91 commented 11 years ago

Here is example

test.js:


define('test1', function() {return {test: 1}})
define('test2', function() {return {test: 2}})
define('test3', function() {return {test: 3}})

index.html:

<html>
<body>
<h1>Test</h1>

<script src="/curl.js"> </script>
<script type="text/javascript">
  var amd_mods = {
     'test1': '/test.js',
     'test2': '/test.js',
     'test3': '/test.js',
  }
  curl({dontAddFileExt:'.', paths:amd_mods}, 
       ['test1', 'test2', 'test3'], 
       function(m1, m2, m3) {
           console.log("Should be 1 got "+m1.test)
           console.log("Should be 2 got "+m2.test)
           console.log("Should be 3 got "+m3.test)
       }
)
</script>

</body>
</html>

here is output with master curl.js:

[16:40:03.861] Should be 1 got 1
[16:40:03.861] Should be 2 got 1
[16:40:03.861] Should be 3 got 1

fafhrd91 commented 11 years ago

here is patch for this problem:


diff --git a/src/curl.js b/src/curl.js
index d6c1508..765e2d3 100644
--- a/src/curl.js
+++ b/src/curl.js
@@ -368,7 +368,7 @@
                        // before resolving
                        def.resolve = function resolve (deps) {
                                when(isPreload || preload, function () {
-                                       origResolve((cache[def.id] = urlCache[def.url] = execute(deps)));
+                                       origResolve((cache[def.id] = urlCache[[def.id,def.url]] = execute(deps)));
                                });
                        };

@@ -972,15 +972,15 @@
                        if (mainId in cache) {
                                def = cache[mainId];
                        }
-                       else if (pathInfo.url in urlCache) {
-                               def = cache[mainId] = urlCache[pathInfo.url];
+                       else if ([mainId,pathInfo.url] in urlCache) {
+                               def = cache[mainId] = urlCache[[mainId,pathInfo.url]];
                        }
                        else {
                                def = core.createResourceDef(pathInfo.config, mainId, isPreload);
                                // TODO: can this go inside createResourceDef?
                                // TODO: can we pass pathInfo.url to createResourceDef instead?
                                def.url = core.checkToAddJsExt(pathInfo.url, pathInfo.config);
-                               cache[mainId] = urlCache[pathInfo.url] = def;
+                               cache[mainId] = urlCache[[mainId,pathInfo.url]] = def;
                                core.fetchResDef(def);
                        }

fafhrd91 commented 11 years ago

with this patch it works, but i get "Error: duplicate define: test1" this exception

asilvas commented 11 years ago

Looks related to #140.

unscriptable commented 11 years ago

Hey @fafhrd91! Thanks for finding this and submitting a patch! If you want to have a working local branch, just remove all references to urlCache. That should fix it.

Your proposed fix will break some other peoples' code. TBH, I don't want to support those use cases. However, I'm working on a solution that should fix all use cases.

Correct, @asilvas. It's related.

fafhrd91 commented 11 years ago

thanks. i'm already using local branch, so no problem. i can wait 0.8

naltatis commented 10 years ago

This issue doesn't seem to be fixed in the current version.

With the above code and version 0.8.1 I still get

Should be 1 got 1
Should be 2 got 1
Should be 3 got 1

When I apply the patch from @fafhrd91 I get the correct result but the the duplicate define error

Should be 1 got 1
Should be 2 got 2
Should be 3 got 3

Uncaught Error: duplicate define: test1

Any ideas on how to fix this?