whatwg / loader

Loader Standard
https://whatwg.github.io/loader/
Creative Commons Zero v1.0 Universal
607 stars 45 forks source link

CommonJS and System.import #19

Closed guybedford closed 9 years ago

guybedford commented 9 years ago

We're typically defining CommonJS modules as the default export for interop.

This means if I use System.import to load a CommonJS module, I will need to manually access the default property:

System.import('cjs-module').then(function(m) {
  var cjsModule = m.default;
});

It would be nice to be able to somehow indicate to the loader that this module is just a default export, and hence that System.import can return the default export directly.

It's possible to do this by overriding System.import with a wrapper function, but I'm not sure if that would carry through to the contextual import as well.

Alternatively if users will just be expected to access the default property that is fine too, but it would be good to know.

arv commented 9 years ago

I don't think that would be a good idea. I think it would be bad if m in the promise callback is not a module instance object.

matthewp commented 9 years ago

@arv Can you explain why you think it would be bad?

caridy commented 9 years ago

The imperative form does not provide a way to implicitly access the default export when importing a module, not today. So, the question here is not about the interpretability with other formats, but whether or not we should consider to have imperative forms that are symmetrical do the declarative forms. Today, we are aiming for the imperative Loader.import() api that is symmetrical to import * as m from "./foo" while the other forms are not available.

System.import('./foo').then(m => {
   m['default']('do something');
});

If we want to stick to that principle, and forget about an api that is symmetric to import foo from "./foo";, then I don't see why we need a way to bypass this invariance.

As for @matthewp's question, having a hook to control the value of m is going to be bad business because you will need a way to differentiate when the module is being imported using the imperative form, vs declarative form. This will be a mess.

matthewp commented 9 years ago

@caridy Superb explanation. Then I agree, it should be the module instance.

arv commented 9 years ago

@matthewp Also, the type of m will change depending on if the module was implemented using CJS/ADM/MMM. Better to always use m.default.

@caridy No need for m['default'], m.default works just as well.

guybedford commented 9 years ago

What about the case of an ES6 module that has just a single default export?

export default {
  some: 'config'
};

Would it not make sense to use the default property here as well on System.import?

caridy commented 9 years ago

no. that will be a huge refactor hazard, if I change my module by just adding export var f = 1;, it will automatically break my app, there is an invariant that we have to maintain in System.import(), also, keep in mind that default is really a convention in ES6 modules, but under the hood it is just an exported name called default.

guybedford commented 9 years ago

Sure, completely agreed - while this will be somewhat inconvenient for CommonJS, AMD and global loading, it is consistent and sensible for a spec.