Reimplement variable deconflicting logic

[lukastaegert]

This PR contains:

• bugfix
• feature
• refactor
• documentation
• other

Are tests included?

• yes (bugfixes and features will not be merged without tests)
• no

Breaking Changes?

• yes (breaking changes will not be merged unless absolutely necessary)
• no

List any relevant issue numbers:
Resolves #2680
Resolves #2683

Description

I know my output is not as much as it used to be at the moment due to my family + day job taking its toll but this is what I have been working on the last weeks. This is basically a complete rework of the name deconflicting logic that lies at the core of scope-hoisting.

That old logic needed to make quite a few assumptions such as certain variable names not appearing (ever wondered why we had $1 and $$1 variables? This was really just a hack to heuristically avoid name clashes). #2683 is a nice example where this went wrong.

But no more!

The new logic already starts in the phase where internal variable objects are associated with variables defined in the code. During this stage, each scope now takes note which variables from outside are used by the code in this scope. To compensate for the additional logic, this noting is also used as caching i.e. when a variable is used more than once in a scope or sub-scope, only the first access will cause a full lookup while the remaining accesses get the cached result.

Now during the actual deconflicting phase, i.e. the phase where all variables receive their final render names, these results are used to deconflict all variables that would otherwise shadow actually used other variables—and only those! As a result, there will now be actually noticeably fewer renames!

Also, variables can now have two "render names": A "base" name and another name. If the base name is present, then this variable is actually rendered as an object member access, i.e. baseName.variableName. In such a situation, of course, only the base name needs to be considered in deconflicting.

Other improvements:

• Formats are taken into account, e.g. with the ESM target, module or exports will usually not be deconflicted automatically as they do not correspond to format specific globals
• Tree-shaking information is taken into account. Now, tree-shaken variables will no longer cause other variables to be renamed
• Reserved words are now taken into account, i.e. a variable will never be called static or await. My hope is that this will resolve Flattening imports MUST take care of javascript reserved words #2680, @matthiasg @gund please tell me if this is not the case.
• Furthermore with the new logic, it was easy to switch some naming priorities. Now inside each chunk, local names take precedence over imported (e.g. external) names. For instance when importing React.Component and locally defining a Component, then the imported one will become a Component$1 while the local one retains its name. My hope is that this change will make the code of libraries nicer overall but please give feedback if I overlooked something here.

This will also form a nice base for future improvements. For instance it would now be quite easy to minify all variable names using this logic. Of course this would probably not be very useful without minifying white-space as well. Another feature I hope to work on soon is merging block scopes into other scopes during rendering, getting rid of some unnecessary curly braces when tree-shaking.

[TrySound]

Awesome!

[thysultan]

Also, variables can now have two "render names": A "base" name and another name. If the base name is present, then this variable is actually rendered as an object member access, i.e. baseName.variableName.

Wouldn't this have a runtime overhead when compared to the previous implementation, or am i reading this wrong?

[lukastaegert]

There should be no qualitative change to the runtime code. This is about cases where we already rewrote variables as object member accesses. The most common example is probably rewriting things like

export let x = 1;
x = 2;

as

exports.x = 1;
exports.x = 2;

to maintain the live binding. There are other examples which I must admit I forgot at the moment🙄 Not too common, though.

[lukastaegert]

Before, the variable would be treated as having the name exports.x which made deconflicting somewhat pointless.

[lukastaegert]

Ah of course, the more important case would be imports in formats other than ESM or SystemJS

[thysultan]

Just to be clear the following won't be affected?

// a.js
export x = 1

// b.js
export x = 2

// main.js
import * as A from a.js
import * as B from b.js

export default A.x + B.x

[TrySound]

@thysultan https://rollupjs.org/repl?version=1.2.0&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmltcG9ydCUyMColMjBhcyUyMEElMjBmcm9tJTIwJy4lMkZhLmpzJyUzQiU1Q25pbXBvcnQlMjAqJTIwYXMlMjBCJTIwZnJvbSUyMCcuJTJGYi5qcyclM0IlNUNuJTVDbmV4cG9ydCUyMGRlZmF1bHQlMjBBLnglMjAlMkIlMjBCLnglMjIlMkMlMjJpc0VudHJ5JTIyJTNBdHJ1ZSU3RCUyQyU3QiUyMm5hbWUlMjIlM0ElMjJhLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmV4cG9ydCUyMGNvbnN0JTIweCUyMCUzRCUyMDIlMjIlMkMlMjJpc0VudHJ5JTIyJTNBZmFsc2UlN0QlMkMlN0IlMjJuYW1lJTIyJTNBJTIyYi5qcyUyMiUyQyUyMmNvZGUlMjIlM0ElMjJleHBvcnQlMjBjb25zdCUyMHglMjAlM0QlMjAxJTIyJTJDJTIyaXNFbnRyeSUyMiUzQWZhbHNlJTdEJTVEJTJDJTIyb3B0aW9ucyUyMiUzQSU3QiUyMmZvcm1hdCUyMiUzQSUyMmNqcyUyMiUyQyUyMm5hbWUlMjIlM0ElMjJteUJ1bmRsZSUyMiUyQyUyMmFtZCUyMiUzQSU3QiUyMmlkJTIyJTNBJTIyJTIyJTdEJTJDJTIyZ2xvYmFscyUyMiUzQSU3QiU3RCU3RCUyQyUyMmV4YW1wbGUlMjIlM0FudWxsJTdE