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] finer control for modern/legacy target polyfills #6922

Open
4 tasks done
Menci opened this issue Feb 14, 2022 · 6 comments
Open
4 tasks done

[plugin-legacy] finer control for modern/legacy target polyfills #6922

Menci opened this issue Feb 14, 2022 · 6 comments
Labels
p2-to-be-discussed Enhancement under consideration (priority) plugin: legacy

Comments

@Menci
Copy link
Contributor

Menci commented Feb 14, 2022

Clear and concise description of the problem

The current implementation of plugin-legacy forces dynamic-import as the modern target (which means, browser supporting dynamic-import will load the modern bundle and others will load the legacy bundle).

But the proportion of browsers that support newer feature is gradually increasing so we can choose a higher feature as the modern target to decrease the bundle size of "modern" target. For the browsers have no support for the given feature, let them load the legacy bundle.

Additionally, we uses @babel/preset-env which generates too many polyfills (even for modern build). For example, if we used new URL in default target (esmodules: true), it generates URL polyfill since Safari < 14 implements URLSearchParams.prototype.delete wrongly. But in most cases we won't care of this so we won't need a 100% right polyfill.

Suggested solution

We already generate code to test legacy/modern browsers. We can change to provide an option to customize it:

interface Options {
  /**
   * JS code to test if the current running browser should use the modern build. Should throw exception if not.
   *
   * @default `"import('data:text/javascript,')"`
   */
  modernFeatureTestCode?: string

  /**
   * A browserslist of target we build the modern bundle for. Used for modern polyfills generation.
   *
   * @default A browserslist with support for ES6 dynamic import.
   */
  modernTargets?: string;
}

Then generate the testing and gate-keeping code like:

const defaultModernFeatureTestCode = "import('data:text/javascript,')"
const getFallbackInlineCode = (featureTestCode) => `!function(){try{new Function("",${JSON.stringify(featureTestCode)})()}catch(o){console.warn("vite: loading legacy build because required features are unsupported, errors above should be ignored");var e=document.getElementById("${legacyPolyfillId}"),n=document.createElement("script");n.src=e.src,n.onload=function(){${systemJSInlineCode}},document.body.appendChild(n)}}();`
const getModernGatekeepingCode = (featureTestCode) => `export function __vite_legacy_guard(){${featureTestCode}};__vite_legacy_guard();`

For example, if I want to target browsers supporting Object.fromEntries, we could use:

{
  modernTargets: browsersWithSupportForFeatures( // From package `browserslist-generator`
    "es6-module-dynamic-import",
    "javascript.builtins.Object.fromEntries"
  ),
  modernFeatureTestCode: "import('data:text/javascript,');Object.fromEntries([])"
}

Additionally, we could accept two functions to filter-out the polyfills we don't want to exclude:

interface Options {
  // Pass a function for auto detection, which returns false to exclude a polyfill.
  polyfills?: boolean | string[] | ((polyfill: string) => boolean)
  modernPolyfills?: boolean | string[] | ((polyfill: string) => boolean)
}

Alternative

Change the way of "testing if the client browser should use modern build" to just test if it matches a browserslist. But it may be hard to do this in a code snippet in index.html.

Additional context

I have implemented all I mentioned features and ready for creating a pull request.

Validations

@ccqgithub
Copy link

Any update?

@Menci
Copy link
Contributor Author

Menci commented Sep 14, 2022 via email

@Tal500
Copy link
Contributor

Tal500 commented Sep 29, 2022

I'm waiting for Vite maintainers' opinions before I could create a PR for this.

On Wed, Sep 14, 2022 at 14:44 Season Chen @.> wrote: Any update? — Reply to this email directly, view it on GitHub <#6922 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACWRBA3WYHUUQ6GFNQQZHO3V6FX4LANCNFSM5OMHRFCQ . You are receiving this because you authored the thread.Message ID: @.>

I suggest you to send first the PR if you already implement it, they give more opinions while they see the implementation details in my experience.

@Menci
Copy link
Contributor Author

Menci commented Sep 29, 2022 via email

@MetRonnie
Copy link

MetRonnie commented Apr 5, 2023

Am I right in saying that, without this feature implemented, @vitejs/plugin-legacy currently cannot automatically inject the correct polyfills to the modern build?

E.g.

// vite.config.js
  plugins: {
    legacy({
      modernPolyfills: true,
      renderLegacyChunks: false,
      targets: 'chrome > 100',
      ignoreBrowserslistConfig: true
    })
  }

ignores the chrome > 100 target and just adds 81 completely arbitrary looking polyfills e.g. core-js/modules/es.array.push.js (which is ancient!?)

@jiadesen
Copy link
Contributor

jiadesen commented Aug 9, 2023

Any follow-up to this issue?

@patak-dev patak-dev added p2-to-be-discussed Enhancement under consideration (priority) and removed enhancement: pending triage labels Feb 21, 2024
@bluwy bluwy moved this from Discussing to Approved in Team Board Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-to-be-discussed Enhancement under consideration (priority) plugin: legacy
Projects
Status: Approved
Development

No branches or pull requests

7 participants