preserveModules caused some unnecessary import and export.

[imzue]

Rollup Version

3.2.5

Operating System (or Browser)

macOS 12.6

Node Version (if applicable)

18.9.0

Link To Reproduction

https://github.com/imzue/rollup-preserve-modules-issue-report

Expected Behaviour

// rollup.config.cjs
import { defineConfig } from "rollup";

export default defineConfig({
  input: ["src/index.js"],
  output: [
    {
      format: "esm",
      dir: "es",
      entryFileNames: "[name].js",
      preserveModules: true,
      preserveModulesRoot: "src",
    },
    {
      format: "cjs",
      dir: "lib",
      entryFileNames: "[name].js",
      preserveModules: true,
      preserveModulesRoot: "src",
    },
  ],
});

Source

// src/Foo/index.js
import Foo from "./Foo";

Foo.add = (a, b) => a + b;

const Foo2 = () => {
  return Foo.multiply(10, 10) + 10;
};

export { Foo, Foo2 };

Expected

// es/Foo/index.js
import Foo from "./Foo.js";

Foo.add = (a, b) => a + b;

const Foo2 = () => {
  return Foo.multiply(10, 10) + 10;
};

export { Foo, Foo2 };

Actual Behaviour

// es/Foo/index.js
import Foo from "./Foo.js";
export { default as Foo } from "./Foo.js"; // unnecessary export

Foo.add = (a, b) => a + b;

const Foo2 = () => {
  return Foo.multiply(10, 10) + 10;
};

export { Foo2 };

AND

// es/Bar/index.js
import Bar from "./Bar.js"; // unnecessary import
export { default } from "./Bar.js";

[lukastaegert]

I agree about the unnecessary export in the second bar case. I remember, though, that it was not completely trivial to figure out if it was really needed.

In the first foo case, the output is actually completely equivalent. It is more verbose, though, which is also not nice.

Here is one thing we could try: When generating exports/re-exports, we could check if a re-exported binding is already imported as some name in a module and in that case, write it as a regular export of that name. That would "solve" both issues together.

[TrickyPi]

@lukastaegert I wonder if src/finalisers/es.ts file is a suitable location to fix this bug, because export and import are appended to magicString here.

[lukastaegert]

It depends if we also have similar issues with the other formats that support code-splitting, i.e. cjs, amd, and system. I cannot check at the moment. If that is the case, it might be better to solve the issue in Chunk by tweaking the information sent to the finalizers (which is what es.ts etc. are called here).

[rollup-bot]

This issue has been resolved via #4759 as part of rollup@3.8.0. You can test it via npm install rollup.