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

"duplicate define" errors in 0.7 when mapping multiple paths to the same "package" (actually, "bundle") #140

Closed asilvas closed 11 years ago

asilvas commented 11 years ago

We mix and match modules between stand-alone (mod1.js) and packages* (mod1 in package.js) pretty regularly so we have granular control over which modules get loaded from where. This no longer works in 0.7, unless I'm missing something (hopefully I am!).

[* edited by @unscriptable: in curl.js, we call these "bundles" since "packages" mean something different. (packages are collections of modules whose ids are namespaced under a common name: "packageId/moduleId".)]

It looks something like this:

<script>
  curl = {
    paths: { "mod1": "package", "mod2": "package" }
  }
</script>
<script src="curl.js"></script>

curl(["mod1", "mod2"], function() { });

Where "package.js" looks something like:


// package.js contains more than one module

define("mod1", [], function() { });
define("mod2", [], function() { });

The above code will no longer work as it did previously, instead it will throw a duplicate define error due to curl thinking it needs to load "package" more than once.

Is this breaking change intended, or a bug?

asilvas commented 11 years ago

If it isn't clear why we do it this way, it's so that on a release by release basis, we have control over what modules get loaded from a stand-alone module file or a multi-module package, without the need for any application changes. We simply point the module path to the desired module or package.

asilvas commented 11 years ago

Not resolved in 068-dev branch.

unscriptable commented 11 years ago

What version of curl are you running in production currently?

asilvas commented 11 years ago

v0.6.2

unscriptable commented 11 years ago

Ahhh, I see. There was a change in 0.6.3 that would explain why you're still seeing different behavior.

I just pushed an 062a branch. It incorporates the IE10 fixes (and an obscure IE6 fix). This should solve all of the problems you've seen. Please try it out.

FYI: what you're calling "packages", we're calling "portable module bundles". (Need a shorter name that doesn't conflict with CommonJS's "packages". Just "bundles" maybe? :) )

These will be available in 0.8. We have not settled on a configuration syntax, yet. It'd be nice if you could just change "paths" to "bundles" in the example above, but I'm not sure that'll work, yet.

I'm going to push a new document to the wiki that explains how curl.js resolves ids to urls and that should help explain why we had to break your configuration. It'll also help explain how we plan to support what you're doing going forward.

asilvas commented 11 years ago

Thanks! Hopefully the 0.8 changes wont be too difficult to migrate to when the time comes; breaking changes make me nervous. I'd like to be an early tester for that release, if possible. We have an exhaustive usage of AMD here, and may find issues that your unit tests may not.

I'll pull in 062a, thanks!

unscriptable commented 11 years ago

Yah, we're going to create an official "release candidate" for 0.8 and leave it up for a week -- or as long as people like you need to test it. :)

unscriptable commented 11 years ago

Hey @asilvas, we just pushed "official" 0.6.2a and 0.6.8 releases to fix IE10 issues. The one you're interested in (0.6.2a) is here: https://github.com/cujojs/curl/tree/2683057e4cdf45eec44dd69dfd814ece75b7ae70 It is exactly the same as the one you already pulled except for changes in the README.

unscriptable commented 11 years ago

Please reopen this if you experience any problems. -- J

asilvas commented 11 years ago

Thanks. Is this issue not still applicable to 0.7 and later?