Skip to content

fix(legacy): modernTargets should set build.target #20393

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sapphi-red
Copy link
Member

Description

modernTargets option is when collecting polyfills for modern chunks. On the other hand, build.target option is used when lowering by esbuild for modern chunks.

This means these two options should be synced. When renderLegacyChunks is true, these two were synced. The value of modernTargets were converted and set to build.target.

But when renderLegacyChunks is false, that did not happen. This causes more / less polyfills to be injected than needed.

This PR makes the legacy plugin to set build.target to the value converted from modernTargets option.
This will cause build.target to be set to a lower version in applications that didn't set modernTargets option (before ['chrome107', 'edge107', 'firefox104', 'safari16'], after ['es2020', 'edge79', 'firefox67', 'chrome64', 'safari12']).

The alternative is to set modernTargets based on build.target. This will make modernTargets option no-op. Also we need to make a function that converts esbuild target syntax to browserslist syntax.

refs #15506

@sapphi-red sapphi-red added plugin: legacy p4-important Violate documented behavior or significantly improves performance (priority) labels Jul 10, 2025
@github-project-automation github-project-automation bot moved this to Discussing in Team Board Jul 10, 2025
@@ -51,7 +51,7 @@ npm add -D terser
- **Type:** `string | string[]`
- **Default:** [`'edge>=79, firefox>=67, chrome>=64, safari>=12, chromeAndroid>=64, iOS>=12'`](https://browsersl.ist/#q=edge%3E%3D79%2C+firefox%3E%3D67%2C+chrome%3E%3D64%2C+safari%3E%3D12%2C+chromeAndroid%3E%3D64%2C+iOS%3E%3D12)

If explicitly set, it's passed on to [`@babel/preset-env`](https://babeljs.io/docs/en/babel-preset-env#targets) when rendering **modern chunks**.
It's passed on to [`@babel/preset-env`](https://babeljs.io/docs/en/babel-preset-env#targets) when collecting polyfills for **modern chunks**.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should also now mention that this field overrides build.target always.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p4-important Violate documented behavior or significantly improves performance (priority) plugin: legacy
Projects
Status: Discussing
Development

Successfully merging this pull request may close these issues.

2 participants