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

export default x live binding scenarios #2000

Closed
guybedford opened this issue Feb 21, 2018 · 3 comments · Fixed by #4182
Closed

export default x live binding scenarios #2000

guybedford opened this issue Feb 21, 2018 · 3 comments · Fixed by #4182

Comments

@guybedford
Copy link
Contributor

Currently we output export default x when we have default exports instead of export { x as default }.

For example - https://rollupjs.org/repl?version=0.56.0&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMnZhciUyMHglMjAlM0QlMjAxMCUzQiU1Q24lNUNuZXhwb3J0JTIwZnVuY3Rpb24lMjB1cGRhdGUlMjAoKSUyMCU3QiU1Q24lNUN0eCUyMCUzRCUyMDExJTNCJTVDbiU3RCU1Q24lNUNuZXhwb3J0JTIwJTdCJTIweCUyMGFzJTIwZGVmYXVsdCUyMCU3RCUyMiU3RCU1RCUyQyUyMm9wdGlvbnMlMjIlM0ElN0IlMjJmb3JtYXQlMjIlM0ElMjJlcyUyMiUyQyUyMm5hbWUlMjIlM0ElMjJteUJ1bmRsZSUyMiUyQyUyMmdsb2JhbHMlMjIlM0ElN0IlN0QlMkMlMjJhbWQlMjIlM0ElN0IlMjJpZCUyMiUzQSUyMiUyMiU3RCU3RCUyQyUyMmV4YW1wbGUlMjIlM0FudWxsJTdE

The issue is that export default x is no longer a live binding, so it would actually be better to have export { x as default } to ensure the live binding!

This case came up considering live binding across chunking interfaces which all works out, except for default exports in this way.

Of course in scenarios where the default is a temporary variable created by Rollup, we know we can use export default as there is no live binding update in play.

For example with - https://rollupjs.org/repl?version=0.56.0&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMnZhciUyMHglMjAlM0QlMjAxMCUzQiU1Q24lNUNuZXhwb3J0JTIwZnVuY3Rpb24lMjB1cGRhdGUlMjAoKSUyMCU3QiU1Q24lNUN0eCUyMCUzRCUyMDExJTNCJTVDbiU3RCU1Q24lNUNuZXhwb3J0JTIwZGVmYXVsdCUyMHglM0IlMjIlN0QlNUQlMkMlMjJvcHRpb25zJTIyJTNBJTdCJTIyZm9ybWF0JTIyJTNBJTIyZXMlMjIlMkMlMjJuYW1lJTIyJTNBJTIybXlCdW5kbGUlMjIlMkMlMjJnbG9iYWxzJTIyJTNBJTdCJTdEJTJDJTIyYW1kJTIyJTNBJTdCJTIyaWQlMjIlM0ElMjIlMjIlN0QlN0QlMkMlMjJleGFtcGxlJTIyJTNBbnVsbCU3RA==

Perhaps we should work on distinguishing these cases and outputting appropriately.

@wearhere
Copy link

wearhere commented Oct 22, 2018

The issue is that export default x is no longer a live binding

I think that it should be a live binding, per e05bf77. I do see that it is not currently a live binding, but I think that's a regression. Filed as #2524.

Put another way, if we fix that regression, then it won't matter whether Rollup outputs export { x as default } or export default x, so we can close this issue.

@shellscape
Copy link
Contributor

Hey folks. This is a saved-form message, but rest assured we mean every word. The Rollup team is attempting to clean up the Issues backlog in the hopes that the active and still-needed, still-relevant issues bubble up to the surface. With that, we're closing issues that have been open for an eon or two, and have gone stale like pirate hard-tack without activity.

We really appreciate the folks have taken the time to open and comment on this issue. Please don't confuse this closure with us not caring or dismissing your issue, feature request, discussion, or report. The issue will still be here, just in a closed state. If the issue pertains to a bug, please re-test for the bug on the latest version of Rollup and if present, please tag @shellscape and request a re-open, and we'll be happy to oblige.

@lukastaegert
Copy link
Member

I think this is bug is important enough to be fixed for spec compliance, reopening.

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

Successfully merging a pull request may close this issue.

4 participants