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

plugin-legacy: modernTargets not respected when renderLegacyChunks is false #15788

Closed
jgosmann opened this issue Feb 2, 2024 · 2 comments · Fixed by #15789
Closed

plugin-legacy: modernTargets not respected when renderLegacyChunks is false #15788

jgosmann opened this issue Feb 2, 2024 · 2 comments · Fixed by #15789

Comments

@jgosmann
Copy link
Contributor

jgosmann commented Feb 2, 2024

Describe the bug

I'm using @vitejs/plugin-legacy with this configuration:

export default defineConfig({
  plugins: [
    legacy({
      modernTargets: ['Firefox >= 68', 'Chrome >= 72', 'Edge >= 91'],
      modernPolyfills: true,
      renderLegacyChunks: false,
    }),
  ],
});

The modernTargets option is only respected if I also set renderLegacyChunks to true.

Reproduction

https://stackblitz.com/edit/vitejs-vite-nagbo9?file=vite.config.js

Steps to reproduce

A (fairly) minimal reproduction is provided under the Stackblitz link. Run DEBUG=vite:legacy npm run build and you will see that the modernTargets option is not logged. In the generated bundle (dist/assets/index-[hash].js), you will be able to find a null coalescing operation (??) that isn't compatible with the browsers given in modernTargets.

If you change renderLegacyChunks to true, you get the expected log output:

plugin-legacy 'modernTargets' option overrode the builtin targets of modern chunks. Some versions of browsers between legacy and modern may not be supported.
[@vitejs/plugin-legacy] targets: last 2 versions and not dead, > 0.3%, Firefox ESR
[@vitejs/plugin-legacy] modernTargets: [ 'Firefox >= 68', 'Chrome >= 72', 'Edge >= 91' ]

Also, in the generated bundle the null coalescing operator does not longer exist and has been replaced with code compatible with older browsers.

System Info

# Stackblitz environment


  System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.18.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.2.3 - /usr/local/bin/npm
    pnpm: 8.14.0 - /usr/local/bin/pnpm
  npmPackages:
    @vitejs/plugin-legacy: ^5.3.0 => 5.3.0 
    vite: ^5.0.12 => 5.0.12 

Local environment

  System:
    OS: macOS 14.3
    CPU: (12) arm64 Apple M2 Max
    Memory: 6.17 GB / 64.00 GB
    Shell: 5.9 - /opt/local/bin/zsh
  Binaries:
    Node: 20.11.0 - /opt/local/bin/node
    npm: 10.4.0 - /opt/local/bin/npm
  Browsers:
    Chrome: 121.0.6167.139
    Edge: 121.0.2277.98
    Safari: 17.3
  npmPackages:
    @vitejs/plugin-legacy: ^5.3.0 => 5.3.0
    @vitejs/plugin-react: ^4.2.1 => 4.2.1
    vite: ^5.0.12 => 5.0.12


### Used Package Manager

npm

### Logs

_No response_

### Validations

- [X] Follow our [Code of Conduct](https://github.com/vitejs/vite/blob/main/CODE_OF_CONDUCT.md)
- [X] Read the [Contributing Guidelines](https://github.com/vitejs/vite/blob/main/CONTRIBUTING.md).
- [X] Read the [docs](https://vitejs.dev/guide).
- [X] Check that there isn't [already an issue](https://github.com/vitejs/vite/issues) that reports the same bug to avoid creating a duplicate.
- [X] Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to [vuejs/core](https://github.com/vuejs/core) instead.
- [X] Check that this is a concrete bug. For Q&A open a [GitHub Discussion](https://github.com/vitejs/vite/discussions) or join our [Discord Chat Server](https://chat.vitejs.dev/).
- [X] The provided reproduction is a [minimal reproducible example](https://stackoverflow.com/help/minimal-reproducible-example) of the bug.
Copy link

stackblitz bot commented Feb 2, 2024

Fix this issue in StackBlitz Codeflow Start a new pull request in StackBlitz Codeflow.

jgosmann added a commit to jgosmann/vite that referenced this issue Feb 2, 2024
If `modernPolyfills` is set to `true`, but `renderLegacyChunks` is set
to false, `genLegacy` will be false and we would return early. If this
return happens before `modernTargets` is initialized, the modern
polyfills will be generated with the wrong (default?) target instead of
the specified `modernTargets`.
jgosmann added a commit to jgosmann/vite that referenced this issue Feb 2, 2024
If `modernPolyfills` is set to `true`, but `renderLegacyChunks` is set
to false, `genLegacy` will be false and we would return early. If this
return happens before `modernTargets` is initialized, the modern
polyfills will be generated with the wrong (default?) target instead of
the specified `modernTargets`.
jgosmann added a commit to jgosmann/vite that referenced this issue Feb 3, 2024
If `modernPolyfills` is set to `true`, but `renderLegacyChunks` is set
to false, `genLegacy` will be false and we would return early. If this
return happens before `modernTargets` is initialized, the modern
polyfills will be generated with the wrong (default?) target instead of
the specified `modernTargets`.

Also, config.build.target must be set from the `modernTargets`, even if
`genLegacy` is to set.
jgosmann added a commit to jgosmann/vite that referenced this issue Feb 8, 2024
If `modernPolyfills` is set to `true`, but `renderLegacyChunks` is set
to false, `genLegacy` will be false and we would return early. If this
return happens before `modernTargets` is initialized, the modern
polyfills will be generated with the wrong (the default?) target instead
of the specified `modernTargets`.
@jgosmann
Copy link
Contributor Author

jgosmann commented Feb 8, 2024

After pondering this comment in the review I realized that the bug described here isn't quite accurate. The syntax transforms are applied correctly if config.build.target is properly configured in the Vite configuration. (Though, the documentation on this could probably be improved.) However, there is still a bug as modernTargets is not correctly initialized before detecting the necessary polyfills.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant