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

Default import/export is not treated as live binding #1078

Closed
asapach opened this issue Oct 24, 2016 · 19 comments

Comments

@asapach
Copy link

commented Oct 24, 2016

Rollup doesn't treat default import/export as a live binding, meaning that changes to the export value is not reflected in the import. Example:

// --- calc.js ---
var x = 0
export {x as default}
export function inc() {
    x++
}

// --- main.js ---
import {default as x, inc} from './calc.js'
console.log(x)
inc()
console.log(x)

Here's a repro: link


Whereas named export works fine:

// --- calc.js ---
export var x = 0
export function inc() {
    x++
}

// --- main.js ---
import {x, inc} from './calc.js'
console.log(x)
inc()
console.log(x)

Here's a repro: link

@pygy

This comment has been minimized.

Copy link

commented Dec 18, 2016

Here's a minimal repro:

var foo;
foo = 5
export default foo;

Becomes (as ES2015)

var foo;
foo = 5;
var foo$1 = foo;

export default foo$1;
@asapach

This comment has been minimized.

Copy link
Author

commented Dec 18, 2016

@pygy, in your example it exports a value instead of the binding. It's equivalent to:

export default 5;

To fix it it needs to be:

var foo;
foo = 5
export {foo as default};

This works in babel (e.g. babel-register), systemjs and webpack2. It even works in Edge browser. Rollup is the only ES6-aware tool that doesn't support it.

@pygy

This comment has been minimized.

Copy link

commented Dec 18, 2016

Oooh indeed, my bad. Fixed sample.

@Rich-Harris

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2016

The current behaviour is deliberate – my understanding (cc @eventualbuddha, who is better at this stuff than me) is that default exports are not live, because they're expressions rather than bindings.

@Rich-Harris

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2016

(Or, more accurately, export default does export a binding, but it exports a binding to a synthetic name called *default* which can't be written to inside the exporting module – in other words by changing the value of x in the top example you're not actually changing the value of the default export.)

@kzc

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2016

Babel generates a live binding for the default:

$ cat calc.js

var x = 0
export {x as default}
export function inc() {
    x++
}

$ babel calc.js

"use strict";

Object.defineProperty(exports, "__esModule", {
    value: true
});
exports.inc = inc;
var x = 0;
exports["default"] = x;

function inc() {
    exports["default"] = x += 1;
}

$ cat main.js

import {default as x, inc} from './calc.js'
console.log(x)
inc()
console.log(x)

$ babel main.js

'use strict';

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { 'default': obj }; }

var _calcJs = require('./calc.js');

var _calcJs2 = _interopRequireDefault(_calcJs);

console.log(_calcJs2['default']);
(0, _calcJs.inc)();
console.log(_calcJs2['default']);
@eventualbuddha

This comment has been minimized.

Copy link
Member

commented Dec 19, 2016

According to this StackOverflow answer, default exports are sometimes bound, but not always. I've been confused about this for ages…

@Rich-Harris

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2016

Oh for the love of–

Why? Why would it be designed that way? It means that import foo from 'foo'; has two completely different meanings depending on the nature of 'foo'.

I give up.

@asapach

This comment has been minimized.

Copy link
Author

commented Dec 19, 2016

I've done some research on this: *default* local name is used in these cases (export default ...):

  • undefined
  • Literals (e.g. null, 42, 'foo', etc.)
  • Identifiers (e.g. export default foo)
  • Anonymous function or class declarations/expressions (and generators)
  • Other expressions

The only remaining case is named function and class declarations - and it mostly works correctly in Rollup: when functions are re-declared the last one wins (example). There are edge cases when these named declarations get re-assigned, but they can be ignored for the sake of simplicity.

However export { ... as default } needs to be treated like a named export - in this instance default is a special name, but not a special case, and follows the same rules. *default* local name is never used here.

@Victorystick

This comment has been minimized.

Copy link
Member

commented Dec 20, 2016

The default binding should be treated exactly the same as any other. The only difference between it and other bindings lies in the syntactic sugar that is export default.

The ECMAScript committee decided that there had to be some "easy" syntax, and thus the default import/export was born. (I can't find exactly where they talked about it over on https://esdiscuss.org, but this is somewhat related.) I'm starting to wonder if we wouldn't have been better off without it.

The default declaration

export default 1337;

is basically just a shorthand for

// This variable is called *default* in the spec, just as asapach@ says.
var implicitVariable = 1337;
export { implicitVariable as default };

It's syntactic sugar in the same way as

import foo from 'foo';

is sugar for

import {default as foo} from 'foo';

Note that named function and class declarations are exceptions to this rule! They are not assigned to an intermediate variable, but exported as is. Compare

export default class {};
// becomes
var implicit = class {};
export { implicit as default };

with

export default class A {}
// becomes
class A {}
export { A as default };

The same thing applies for function declarations, which is why they can be referenced within the module.

@Rich-Harris

This comment has been minimized.

Copy link
Contributor

commented Dec 20, 2016

I'm starting to wonder if we wouldn't have been better off without it.

We absolutely would have been. Default exports have caused no end of problems. People get desperately confused by all the different forms of import/export declaration – imagine if we could teach people that you either import { names } or * as namespace, and that you can export either names or declarations. As it stands, it feels like there's a ton of different variations you have to understand.

Plus the confusion that arises over whether default exports are live or not. I've spent more time learning about ES modules than anyone should reasonably be expected to, and I had no idea that the situation is as you've described. (Marked this issue as a bug, btw.)

And then there's the interop headaches. Ostensibly, privileged default exports were meant to make adoption easier for a community that's familiar with Node modules, which is ironic as nonsense likemodule.exports.default has probably caused more friction than any other aspect of ES modules. I'm sure we could have come up with a better way of importing single-export CommonJS modules. (Though we shouldn't really call them CommonJS modules – CommonJS modules can only have named exports!)

Unfortunately, we're stuck with it.

@Victorystick

This comment has been minimized.

Copy link
Member

commented Dec 20, 2016

I hear you. Knew Node didn't really follow the CommonJS standard, but not that CommonJS was limited to only named exports. Would have made the whole thing much easier though.

The solution is quite easy though: stop treating default differently from the other bindings. Instead of

$ = 'default' in $ ? $['default'] : $;

$( 'body' ).html( '<h1>Hello world!</h1>' );

it'll be straight on

$.default( 'body' ).html( '<h1>Hello world!</h1>' );

Rich-Harris added a commit that referenced this issue Dec 21, 2016

Rich-Harris added a commit that referenced this issue Dec 21, 2016

Merge pull request #1166 from rollup/gh-1078
[BREAKING] fix behaviour of export { foo as default }

Rich-Harris added a commit that referenced this issue Dec 21, 2016

Rich-Harris added a commit that referenced this issue Dec 21, 2016

Merge pull request #1167 from rollup/gh-1078
deshadow destructured parameters with assignments
@Rich-Harris

This comment has been minimized.

Copy link
Contributor

commented Dec 22, 2016

This is fixed in 0.38

asapach added a commit to asapach/babel-plugin-rewire-exports that referenced this issue Dec 22, 2016

@objectkit

This comment has been minimized.

Copy link

commented May 6, 2017

Unfortunately I am encountering a similar issue, BUT... it is an uncertain edge case. I have taken the advice from above to use export { TARGET as default } rather than export default TARGET, but name mangling is still happening when I import by module name.

Parent.js:

class Parent  {
    
}

export { Parent as default }

Child.js:

import "/Parent" // import by module name

class Child extends Parent {

}

export { Child as default }

then with rollup...

class Parent$1  {
    
}

class Child extends Parent {

}

export { Child, Parent$1 as Parent };

However, If I change the import toimport Parent from "/Parent", I get what I would have expected:

class Parent  {
    
}

class Child extends Parent {

}

export { Child, Parent };

I would have expected that importing by module name would have the same affect as importing by default member, yet importing by module name results in name mangling. Am I mistaken?

@asapach

This comment has been minimized.

Copy link
Author

commented May 6, 2017

import "/Parent" is not the same as import Parent from "/Parent"
In the first case you're just importing the module without any bindings - this is usually done for side effects.

@objectkit

This comment has been minimized.

Copy link

commented May 6, 2017

Cheers for the heads up! Rollup without name mangling was the side effect I was hoping for actually... I guess implicit bindings are not part of the es specification? Or is it an issue with Rollup?

@asapach

This comment has been minimized.

Copy link
Author

commented May 6, 2017

Nope, there's no such thing as implicit bindings. In real ES modules this would throw an error. You can actually try in latest Safari or Edge under some flags.

@objectkit

This comment has been minimized.

Copy link

commented May 6, 2017

OK! Thank you! I was most definitely confused. I suspected it was a long shot. Context was a dependency resolution plugin I was considering using btw. Default imports work fine, and keep within ES spec, so Rollup === OK. Processing empty imports without name mangling would be nice-to-have though.
Thanks @asapach 👍

@wearhere

This comment has been minimized.

Copy link

commented Oct 22, 2018

I think this regressed #2524 I misunderstood the intended behavior, all good. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.