vite
vite copied to clipboard
Legacy plugin modern polyfills don't respect targets
Describe the bug
It seems that with modernPolyfills: true
, neither the targets
nor the browserlist config is used at all. Along with polyfilling more than necessary for a given target, I am unable to get it to change the polyfill at all regardless of any targets, however new or old.
The docs say, "Note it is not recommended to use the true value (which uses auto-detection) because core-js@3 is very aggressive in polyfill inclusions due to all the bleeding edge features it supports." I understand that core-js may be aggressive, but I find it hard to believe that it thinks Chrome latest requires a polyfill for every possible feature used. In the reproduction link, notice the target is last 1 chrome version
. Running the build lists a polyfill for Object.fromEntries, and includes it in the generated polyfill file, as well as something as old as Promise. The core-js compat file lists this as compatible in Chrome.
Reproduction
https://stackblitz.com/edit/vitejs-vite-sdqghd?file=counter.js
Steps to reproduce
DEBUG="vite:legacy" npm run build
in linked stackblitz
System Info
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: 9.4.2 - /usr/local/bin/npm
pnpm: 8.6.12 - /usr/local/bin/pnpm
npmPackages:
@vitejs/plugin-legacy: ^4.1.1 => 4.1.1
vite: ^4.4.8 => 4.4.9
Used Package Manager
npm
Logs
~/projects/vitejs-vite-sdqghd 2s
❯ DEBUG="vite:legacy" npm run build
rendering chunks (1)...[@vitejs/plugin-legacy] modern polyfills: Set(4) {
'core-js/modules/es.array.iterator.js',
'core-js/modules/web.dom-collections.iterator.js',
'core-js/modules/es.promise.js',
'core-js/modules/es.object.from-entries.js'
}
dist/index.html 0.53 kB │ gzip: 0.31 kB
dist/assets/javascript-8dac5379.svg 1.00 kB │ gzip: 0.60 kB
dist/assets/index-d9d327fd.css 1.25 kB │ gzip: 0.66 kB
dist/assets/index-2ac35ad6.js 1.48 kB │ gzip: 0.78 kB
dist/assets/polyfills-e21d0655.js 26.24 kB │ gzip: 11.20 kB
✓ built in 1.30s
Validations
- [X] Follow our Code of Conduct
- [X] Read the Contributing Guidelines.
- [X] Read the docs.
- [X] Check that there isn't already an issue 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 instead.
- [X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
- [X] The provided reproduction is a minimal reproducible example of the bug.
Indeed, when I modify this line to instead pass the targets instead of this hard-coded target, things work as expected https://github.com/vitejs/vite/blob/21ae3716927ed8c466235bdd667a45c26468c8c0/packages/plugin-legacy/src/index.ts#L387
await detectPolyfills(raw, options.targets, modernPolyfills);
@C-Higgins I've litterally done the same by patching this logic with yarn patch
.
Can concur that if targets is passed along side modernPolyfills: true
then it should use the targets. This massively reduces the scope of polyfills that it generates.
targets
is the legacy target, so I don't think await detectPolyfills(raw, options.targets, modernPolyfills);
is correct? Maybe we can introduce a new modernTargets
option. Until there's a way for us to safely convert esbuild targets to browserslist.
targets
is the legacy target, so I don't thinkawait detectPolyfills(raw, options.targets, modernPolyfills);
is correct? Maybe we can introduce a newmodernTargets
option. Until there's a way for us to safely convert esbuild targets to browserslist.
I think using the passed targets must be more correct than current behavior, no? Especially if renderLegacyChunks
is false. It's hard to imagine a scenario where the current behavior of modernPolyfills: true
is intended and useful; it seems more like a bug to me.
@C-Higgins are you aware if this was this introduced in a specific release?
Until there's a way for us to safely convert esbuild targets to browserslist.
@bluwy this package may be useful: https://github.com/nihalgonsalves/esbuild-plugin-browserslist
@C-Higgins Making targets
implicitly elevate as modern targets is a bit too automatic, especially that the default targets are set to target older browsers 'last 2 versions and not dead, > 0.3%, Firefox ESR'
. It would also be confusing for projects that dual-setup legacy and modern polyfills. Making a new modernTargets
option, reflecting the polyfills
-> modernPolyfills
pattern feels more predictable to me.
@jroru We need the other way round, esbuild target -> browserslist. The esbuild plugin does the opposite.
Affected by this as well.
Also, cross-linking this comment from #6922 talking about the same problem.
We need the other way round, esbuild target -> browserslist. The esbuild plugin does the opposite.
@bluwy Could you elaborate why this is the case? As far as I understand, targets
is a browserlist query. If a modernTargets
option were to be introduced, I would expect this also to be in browserlist query syntax and hence this needs to be converted to esbuild syntax to set modernTargetsEsbuild
, doesn't it?
@jgosmann If we introduce a new modernTargets
option that's a browserlist query, then yes we can use the package to turn that into esbuild syntax. I was referring before that if we want re-use Vite's build.target
config (esbuild syntax), then we'd need a way to turn that into browserlist.