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

Treeshaking multiple entry modules with cross-referencing creates extra chunks #4962

Closed
tmsns opened this issue Apr 27, 2023 · 3 comments · Fixed by #4989
Closed

Treeshaking multiple entry modules with cross-referencing creates extra chunks #4962

tmsns opened this issue Apr 27, 2023 · 3 comments · Fixed by #4989

Comments

@tmsns
Copy link
Author

tmsns commented Apr 27, 2023

Some more background info:

Possible Cause

We think that Rollup tries to optimize the output bundle and as he notices a module (math.js) with only re-exports, he drills down directly to the referenced inner module (integer-math.js). But as two entries need this file, he will create a new chunk for both entries.

We tried to change the behavior using some of the options of Rollup, but none of them (output.hoistTransitiveImports, preserveentrysignatures, treeshake.*) created the expected bahavior.

Workarounds

As we were surprised by this result, we tried to workaround this behavior. We came up with a number of ways, none of which are ideal.

  • mark math.js as side effect
    When adding a side effect to math.js (eg. void Console, a side effect that doesn't do anything), Rollup seems to create the expected output. Of course we don't like to have extra side effects in a file to just do nothing. This could also be achieved dynamically with a plugin.
  • disable treeshaking
    Of course, we still want to be able to tree shake, so this solution was dismissed.
  • a manualChunk function
    For now we settled with a manualChunk function that will make sure that modules that are reference from an entry module are bundled together. But we considered this more as a workaround than an actual long-term solution.
import { basename } from 'path';

/**
 * Bundles all first level imports of an entry module together with its entry in the same chunk.
 *
 * @type {import('rollup').GetManualChunk}
 */
export function bundleImportsOfEntries(id, { getModuleInfo }) {
  const moduleInfo = getModuleInfo(id);
  if (moduleInfo.isEntry) {
    return basename(id);
  }

  const entryPoint = moduleInfo.importers.find(importer => getModuleInfo(importer).isEntry);

  if (entryPoint) {
    return basename(entryPoint);
  }

  return undefined;
} 

/cc @jorenbroekema @bashmish

@rollup-bot
Copy link
Collaborator

This issue has been resolved via #4989 as part of rollup@3.22.0-0. Note that this is a pre-release, so to test it, you need to install Rollup via npm install rollup@3.22.0-0 or npm install rollup@beta. It will likely become part of a regular release later.

@rollup-bot
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants