Exports moved unexpectedly with preserveModules

[jakearchibald]

• Rollup Version: 2.7.6
• Operating System (or Browser): OSX
• Node Version (if applicable): 12.16.1
• Link to reproduction (IMPORTANT, read below): https://repl.it/repls/SpanishImmaculatePrediction

Input

index.js

import bar, { foo } from "./lib";
console.log(bar, foo);

lib.js

export default SOME_VARIABLE;
export const foo = SOME_OTHER_VARIABLE;

Expected Behavior

SOME_VARIABLE and SOME_OTHER_VARIABLE remain within lib.js.

Actual Behavior

index.js

import { foo } from './lib.js';

console.log(SOME_VARIABLE, foo);

This change happened after updating from Rollup 2.2.0. In 2.2.0 it behaves as expected, but I'm not sure exactly which version introduced the behaviour change.

This transform feels like it violates preserveModules.

[lukastaegert]

SOME_VARIABLE appears to be a global, so no reason to import it. Otherwise preserveModules tries to preserve exports if they are used, but imports will be replaced by direct imports if possible. I.e. reexports will be replaced by what they reexport, and if they reexport a global, it will be short-cut. As for globals I see now that this maybe too eager as of course the default export takes a snapshot and if the global changes, the default export will not update while the global will. So maybe this should be changed. For local variables, Rollup usually knows if a default exported variable is ever updated.

[lukastaegert]

In general as for preserveModules, only the full signatures of entry modules are preserved. Other modules are subject to tree-shaking which is actually a feature here.

[jakearchibald]

In this case I was using SOME_VARIABLE as a placeholder that I was replacing in renderChunk. I thought, due to preserveModules, the placeholder wouldn't move.

Is there a better way to create placeholders? In the end I changed my placeholder to SOME_VARIABLE(), which prevented movement.

[lukastaegert]

If you want to leave tree-shaking on, a call to an unknown global seems like a good choice for a side-effect to me for now. Though as the return value is not used, there is again no reason to keep the export even if it is retained for now.

The safest way would be to either promote all such modules to entry chunks or use emitFile with their ids. Additionally, adding the side-effect of calling an unknown global will definitely make sure nothing is moved here. This should also remain stable in the future.

[lukastaegert]

Thinking about it, import.meta.myProperty could also be a very good choice for a placeholder with virtually no danger of name conflicts, and there is already a custom hook to replace it separately for each output: https://rollupjs.org/guide/en/#resolveimportmeta

[jakearchibald]

Thinking about it, import.meta.myProperty could also be a very good choice for a placeholder with virtually no danger of name conflicts

Boom! Yes, that works perfectly. I guess you're sharing that 'namespace' with browser things and other plugins, but the same goes for globals.

[jakearchibald]

Although, one substantial gotcha of using import.meta.* for flags, is the final value doesn't impact the file hash.

[lukastaegert]

@jakearchibald I still hope to tackle this issue rather soon. There is just one other big refactoring I want to do first which is moving the chunking from the build to the output generation phase. The reason is that this makes many things a lot easier as it allows to safely store transient state on the chunks without influencing other outputs.

As for this issue, there is now actually a fix: #3550
This addresses an underlying bug in the logic that you uncovered, namely that global variables could be reassigned, in which case we do not want a live-binding to the default export. As a side-effect, the global variable will now be rendered solely in the exporting chunk when preserving modules.