Incorrect output with default exports & circular dependencies

[jakearchibald]

Rollup Version

2.52.7

Operating System (or Browser)

REPL

Node Version (if applicable)

No response

Link To Reproduction

https://rollupjs.org/repl/?version=2.52.7&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmltcG9ydCUyMGZvbyUyMGZyb20lMjAnLiUyRm1vZHVsZS5qcyclM0IlNUNuJTVDbmZvbygpJTNCJTVDbiU1Q25mdW5jdGlvbiUyMGhlbGxvKCklMjAlN0IlNUNuJTIwJTIwY29uc29sZS5sb2coJ2hlbGxvJyklM0IlNUNuJTdEJTVDbiU1Q25leHBvcnQlMjBkZWZhdWx0JTIwaGVsbG8lM0IlMjIlMkMlMjJpc0VudHJ5JTIyJTNBdHJ1ZSU3RCUyQyU3QiUyMm5hbWUlMjIlM0ElMjJtb2R1bGUuanMlMjIlMkMlMjJjb2RlJTIyJTNBJTIyaW1wb3J0JTIwaGVsbG8lMjBmcm9tJTIwJy4lMkZtYWluLmpzJyUzQiU1Q24lNUNuaGVsbG8oKSUzQiU1Q24lNUNuZnVuY3Rpb24lMjBmb28oKSUyMCU3QiU1Q24lMjAlMjBjb25zb2xlLmxvZygnZm9vJyklM0IlNUNuJTdEJTVDbiU1Q25leHBvcnQlMjBkZWZhdWx0JTIwZm9vJTNCJTIyJTJDJTIyaXNFbnRyeSUyMiUzQWZhbHNlJTdEJTVEJTJDJTIyb3B0aW9ucyUyMiUzQSU3QiUyMmZvcm1hdCUyMiUzQSUyMmVzJTIyJTJDJTIybmFtZSUyMiUzQSUyMm15QnVuZGxlJTIyJTJDJTIyYW1kJTIyJTNBJTdCJTIyaWQlMjIlM0ElMjIlMjIlN0QlMkMlMjJnbG9iYWxzJTIyJTNBJTdCJTdEJTdEJTJDJTIyZXhhbXBsZSUyMiUzQW51bGwlN0Q=

Expected Behaviour

This should result in code that throws because hello is accessed before it's initialised. This is due to how default export whatever is treated as an expression.

The output should be something like:

_helloExport();

function foo() {
  console.log('foo');
}

let _fooExport = foo;

_fooExport();

function hello() {
  console.log('hello');
}

let _helloExport = hello;

export default hello;

Actual Behaviour

Produces runnable code.

[jakearchibald]

Rollup gets it right in this slightly modified case.

[lukastaegert]

So you are saying that Rollup should either assign the default export to a variable in the right position or otherwise write the default export itself in the correct place?
Also if I understand you correctly, this would be an even simpler reproduction of the issue: https://rollupjs.org/repl/?version=2.52.7&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmV4cG9ydCUyMGRlZmF1bHQlMjBoZWxsbyUzQiU1Q24lNUNuY29uc3QlMjBoZWxsbyUyMCUzRCUyMCdoZWxsbyclM0IlMjIlMkMlMjJpc0VudHJ5JTIyJTNBdHJ1ZSU3RCU1RCUyQyUyMm9wdGlvbnMlMjIlM0ElN0IlMjJmb3JtYXQlMjIlM0ElMjJlcyUyMiUyQyUyMm5hbWUlMjIlM0ElMjJteUJ1bmRsZSUyMiUyQyUyMmFtZCUyMiUzQSU3QiUyMmlkJTIyJTNBJTIyJTIyJTdEJTJDJTIyZ2xvYmFscyUyMiUzQSU3QiU3RCU3RCUyQyUyMmV4YW1wbGUlMjIlM0FudWxsJTdE ?

[jakearchibald]

So you are saying that Rollup should either assign the default export to a variable in the right position or otherwise write the default export itself in the correct place?

I think so. V8 folks tell me that:

export default thing;

…is equivalent to:

let _defaultExport;
export { _defaultExport as default };
_defaultExport = thing;

Where _defaultExport is hidden. But there are probably shortcuts that produce the same behaviour.

Also if I understand you correctly, this would be an even simpler reproduction of the issue: link

That's definitely a bug, but it isn't clear to be if it's the same bug or not.

Here's another. The original code will log 'changed' but the Rollup'd version will log 'initial'. Again, I don't know if that's a different bug or the same bug.

export { identifier as default } exposes a live binding, whereas export default identifier evaluates identifier as an expression, so they behave differently, and Rollup shouldn't translate from one to the other unless it's sure it doesn't matter. Fwiw I wrote a post on this weird behaviour https://jakearchibald.com/2021/export-default-thing-vs-thing-as-default/

[lukastaegert]

It is definitely a larger complex. If you export a mutable value, there is a logic to snapshot correctly, but obviously it fails in the case you listed: https://rollupjs.org/repl/?version=2.52.7&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmxldCUyMGhlbGxvJTIwJTNEJTIwMSUzQiU1Q24lNUNuZXhwb3J0JTIwZGVmYXVsdCUyMGhlbGxvJTNCJTVDbiU1Q25oZWxsbyUyMCUzRCUyMDIlM0IlMjIlMkMlMjJpc0VudHJ5JTIyJTNBdHJ1ZSU3RCU1RCUyQyUyMm9wdGlvbnMlMjIlM0ElN0IlMjJmb3JtYXQlMjIlM0ElMjJlcyUyMiUyQyUyMm5hbWUlMjIlM0ElMjJteUJ1bmRsZSUyMiUyQyUyMmFtZCUyMiUzQSU3QiUyMmlkJTIyJTNBJTIyJTIyJTdEJTJDJTIyZ2xvYmFscyUyMiUzQSU3QiU3RCU3RCUyQyUyMmV4YW1wbGUlMjIlM0FudWxsJTdE

[lukastaegert]

Most of this except the original issue with circular dependencies will be resolved in #4182

[jirutka]

I think so. V8 folks tell me that:

export default thing;

…is equivalent to:

let _defaultExport;
export { _defaultExport as default };
_defaultExport = thing;

V8 is not the only JavaScript engine in existence. The important question is: is this behaviour described in the ECMAScript standard?

Relevant to #4182 (comment)

[jakearchibald]

V8 is not the only JavaScript engine in existence.

We're all fully aware of that.

The important question is: is this behaviour described in the ECMAScript standard?

Yes. See the details in the blog post I linked to earlier https://jakearchibald.com/2021/export-default-thing-vs-thing-as-default/

[imtaotao]

Is this problem solved? When I use rollup to build semver, I run into a bug that exports circular references by default. causes semver.subset to be unavailable.