Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Early exports and typescript. #265

Open
Yshayy opened this issue Apr 3, 2014 · 5 comments
Open

Early exports and typescript. #265

Yshayy opened this issue Apr 3, 2014 · 5 comments

Comments

@Yshayy
Copy link

Yshayy commented Apr 3, 2014

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

@unscriptable
Copy link
Member

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:

  1. Disable early exports by default. (This should fix your immediate problem unless you have circular dependencies in your code.)
  2. Add an option to enable early exports
  3. Turn on early exports by default if using a dojo shim (dojo has a dep cycle).
  4. Document the early exports option and describe uses cases where it does or does not work.

Thoughts?

-- John

@Yshayy
Copy link
Author

Yshayy commented Apr 8, 2014

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.

@unscriptable
Copy link
Member

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

@unscriptable
Copy link
Member

Have you had a chance to try this, @Yshayy ?

@Yshayy
Copy link
Author

Yshayy commented May 14, 2014

Not yet, I'll try testing it in the next few days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants