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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: tree-shake modulepreload polyfill #9531

Merged
merged 1 commit into from Aug 8, 2022

Conversation

caoxiemeihao
Copy link
Contributor

@caoxiemeihao caoxiemeihao commented Aug 4, 2022

Description

This is a build optimization PR, which will recuce the bundle size.

Before

before-polyfill

After

after-polyfill

Additional context

Reproduce

This is often used for Electron App builds.
My Electron App 馃憠 electron-vite-vue.

export default {
  build: {
    minify: false,
    rollupOptions: {
      output: {
        // Here
        format: 'cjs',
      },
    },
  },
}

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@sapphi-red
Copy link
Member

Do you have a reproduction? I checked with playground/legacy, but it seems to work without this PR.

@sapphi-red sapphi-red changed the title chore: optimize polyfill-string feat: tree-shake modulepreload polyfill Aug 4, 2022
@caoxiemeihao
Copy link
Contributor Author

caoxiemeihao commented Aug 4, 2022

@sapphi-red You can set the value of format to 'cjs' to reproduce. This is often used for Electron App builds.

export default {
  build: {
    minify: false,
    rollupOptions: {
      output: {
        // Here
        format: 'cjs',
      },
    },
  },
}

@sapphi-red
Copy link
Member

sapphi-red commented Aug 4, 2022

This was because when you disable minify, tree-shaking won't be enabled for esbuild.
esbuild repl without --tree-shaking
esbuild repl with --tree-shaking

Currently it seems it's not possible to set esbuild.treeShaking: true and build.minify: false at the same time.

Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be fine doing this way for now to take advantage of Rollup treeshaking instead of esbuild's

@sapphi-red
Copy link
Member

__VITE_IS_MODERN__ is replaced in renderChunk hook. So it is not tree-shaked by rollup.
With this PR, it will be removed by esbuild's constant propergation (esbuild repl).

@bluwy
Copy link
Member

bluwy commented Aug 6, 2022

Oh interesting, so esbuild has some sort of partial treeshaking by default, which seems to remove false && foo, but not if (false) foo (example). In our case this would be fine then.

@patak-dev patak-dev changed the title feat: tree-shake modulepreload polyfill fix: tree-shake modulepreload polyfill Aug 8, 2022
@patak-dev patak-dev merged commit 1f11a70 into vitejs:main Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants