vitejs / vite

Next generation frontend tooling. It's fast!
http://vitejs.dev
MIT License
66.36k stars 5.94k forks source link

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

Open Menci opened 2 years ago

Menci commented 2 years ago

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 commented 1 year ago

Any update?

Menci commented 1 year ago

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 https://github.com/vitejs/vite/issues/6922#issuecomment-1246314773, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACWRBA3WYHUUQ6GFNQQZHO3V6FX4LANCNFSM5OMHRFCQ . You are receiving this because you authored the thread.Message ID: @.***>

Tal500 commented 1 year ago

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 commented 1 year ago

OK. I will.

On Thu, Sep 29, 2022 at 18:56 Tal500 @.***> wrote:

I'm waiting for Vite maintainers' opinions before I could create a PR for this. … <#m2926676529368159112> On Wed, Sep 14, 2022 at 14:44 Season Chen @.> wrote: Any update? — Reply to this email directly, view it on GitHub <#6922 (comment) https://github.com/vitejs/vite/issues/6922#issuecomment-1246314773>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACWRBA3WYHUUQ6GFNQQZHO3V6FX4LANCNFSM5OMHRFCQ 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.

— Reply to this email directly, view it on GitHub https://github.com/vitejs/vite/issues/6922#issuecomment-1262112849, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACWRBAZSWDB2C26KZEVQCLDWAVYVBANCNFSM5OMHRFCQ . You are receiving this because you authored the thread.Message ID: @.***>

MetRonnie commented 1 year ago

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 commented 11 months ago

Any follow-up to this issue?