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

Why skip "default" in GetExportedNames and ResolveExport? #948

Closed
benjamn opened this issue Jul 11, 2017 · 4 comments
Closed

Why skip "default" in GetExportedNames and ResolveExport? #948

benjamn opened this issue Jul 11, 2017 · 4 comments

Comments

@benjamn
Copy link
Member

benjamn commented Jul 11, 2017

In section 15.2.1.16.2.7.c.i of the spec, in the implementation of the GetExportedNames method, @jdalton and I recently noticed that exports named default are deliberately skipped.

Echoing this policy, the ResolveExport method also notes that "A default export cannot be provided by an export *" (15.2.1.16.3.6.c).

The git blame for these sections extends beyond the history of this repository, and I haven't been able to find a justification for this skippage on es-discuss (at least as long as I've been subscribed). If I should have looked harder/smarter, please feel free to tell me how.

If I understand the implications of this policy, it seems problematic for several reasons:

  1. It prevents (or at least complicates) using export * from "./original" to create a high-fidelity wrapper module whose export behavior matches the module it wraps. For comparison, in Node/CommonJS, the analogous (if not perfectly equivalent) idiom is to alias module.exports with the original module's exports object:
module.exports = require("./original");

In my experience, this idiom is relatively common in Node applications, and has the benefit of being easy to remember. Imagine a JS developer reading through a CommonJS-to-ECMAScript cheatsheet and stumbling across this ECMAScript equivalent:

export * from "./original";
export { default } from "./original";

To me, this feels like a surprising ergonomic regression.

  1. The policy is currently at odds with what Babel does. While this inconsistency is certainly not the end of the world, it does mean that retracting the policy (if possible) could prevent many bugs that will be exposed in the transition from Babel-transpiled modules to native ES modules. If Babel needs to change its treatment of default exports to match this policy, that's going to be a pretty big deal, so we should make absolutely sure the policy is desirable and necessary—or that my reading of the policy is incorrect!

  2. Since module namespace objects (e.g. import * as namespace from "./original") are created with GetModuleNamespace, which calls GetExportedNames, a strict interpretation of the policy would seem to exclude default exports from namespace objects, so you would have to import OriginalDefault from "./original" separately, because namepace.default is not defined. If I'm missing some part of the spec that ensures namespace objects preserve the .default property, I'd love to be set straight.

  3. Since the dynamic import(...) proposal also uses GetModuleNamespace, and the proposal does not appear to alter anything about the policy in question, a strict interpretation would seem to exclude default exports from module namespace objects imported dynamically. I've separated this case from the previous case because the import OriginalDefault from "./original" trick from above does not help here. With any luck @domenic will have some insight (again, I would be happy if my interpretation is simply wrong).

tl;dr My instinct is that we should consider retracting this policy, provided it does not provide some fundamental benefit that must be preserved.

I'm happy to submit a PR to fix this issue and/or bring it up for discussion at the next TC39 meeting.

Original issue/inspiration: benjamn/reify#184

@domenic
Copy link
Member

domenic commented Jul 11, 2017

@allenwb
Copy link
Member

allenwb commented Jul 11, 2017

@domenic yup. And I believe a search of pre-ES6 release meeting notes would turn of earlier discussions about this.

@benjamn
Copy link
Member Author

benjamn commented Jul 11, 2017

Thanks @domenic and @allenwb.

Of the other people who participated in that conversation (@caridy @dherman @wycats), has your thinking changed at all since November?

It looks like my first two objections were discussed in that conversation. I still consider them legitimate, but at least they are not new. In particular, if this policy stands, Babel-compiled code will break, though I realize not everyone on the committee is moved by that argument.

My second two objections may be mistaken (which is good news to me). Can anyone confirm that module.[[LocalExportEntries]] (as used here) contains default export entries, which is what allows namespace objects to have the .default property?

@benjamn
Copy link
Member Author

benjamn commented Jul 11, 2017

Thanks to everyone who took a look at this. I've implemented the existing policy in my own module compiler project now, so I'm not personally concerned about what Babel does. If the policy turns out to be a problem for Meteor users, I might reopen this issue, but at this point it doesn't sound like a productive use of TC39's time to re-litigate the policy (especially since @caridy tells me he was convinced by the discussion in November).

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

3 participants