Early exports and typescript.
Yshayy opened this issue · 5 comments
Hi,
We're using curljs for quite some and we recently start migrating our code to typescript.
Typescript generated code is wrapped in AMD by the typescript compiler like this:
define(["require", "exports", 'deps...'], function(require, exports, deps...)
{
}
While this behavior should work fine in RequireJS (according to TS documentation), in curljs it triggers early export (which make sense for cjs modules), and make the deps not available to the module on resolve (which is especially problematic in typescript due to it's classical inheritance features).
Anyway to control/tweak this behavior?
Thanks
Yshay
Hey @Yshayy,
We've been getting a lot of issues related to curl.js's "early exports" feature, which is somewhat broken, atm. It works fine for some dependency cycles, but not all. The problem is that curl.js executes factory functions eagerly. Unless something forces some modules to wait for the next event turn (setTimeout, domReady, etc), curl.js can't resolve the cycle correctly.
I'm going to do a quick assessment to see how much work it will be to defer factory execution. If it's simple, then we'll release 0.9 with deferred factory execution. Otherwise, here's the "Plan B" for 0.9:
- Disable early exports by default. (This should fix your immediate problem unless you have circular dependencies in your code.)
- Add an option to enable early exports
- Turn on early exports by default if using a dojo shim (dojo has a dep cycle).
- Document the early exports option and describe uses cases where it does or does not work.
Thoughts?
-- John
I think that early export behavior is really problematic when using AMD, because it require adding another layer of async handling for resolving resources other than "define", make code more difficult to predict (if we would've use setTimeout/domReady) and manage (if we use resolved promises).
I'm not sure about how AMD designed to handle circular dependencies and i didn't find much documentation on this issue, but I think that if some resource "a" want to early export resource "b", it should be done in the "define" part in a clear way.
It can be indicated by dropping the referenced argument in the factory method -
define(["require", "exports", 'b'], function(require, exports)
{
});
special conventions in resource uri (like plugins) or factory arguments names -
define(["require", "exports", 'early!b'], function(require, exports)
{
});
In either case, since early exporting use cases are probably related to circular dependencies which are themselves a problematic practice, it might be a good idea to require the programmer to explicitly request it.
But since my guess is that this issue is more related to commonjs amd module wrapper and amd spec in general, it would be nice if we can simply disable this feature by configuration.
Right now we solve it by commenting this section in the code:
def.exportsReady = function executeFactory (deps) {
when(isPreload || preload, function () {
// only resolve early if we also use exports (to avoid
// circular dependencies). def.exports will have already
// been set by the getDeps loop before we get here.
/if (def.exports) {
execute(deps);
def.progress(msgFactoryExecuted);
}/
});
};
Of course, we prefer not to patch code in such way,
especially since curljs is very critical component in our project.
Hey @Yshayy,
I was able to defer factory execution. This is the best option, by far. Early exports still happens in most cases, but because factories only execute just-in-time, the exports should never be too early any more.
As I feared, there was a cascade of changes to make this work, but there are other benefits to this refactoring, as well.
I pushed the latest code to the dev branch. Please try it out as soon as you have a chance.
Thanks!
-- John
Have you had a chance to try this, @Yshayy ?
Not yet, I'll try testing it in the next few days.