String.prototype.replaceAll polyfill is not included

[ninoman]

Describe the bug

Hello.

We use '@vitejs/plugin-legacy' to create legacy build and also include polyfills for our modern browsers. We use String.prototype.replaceAll method in our application, but we still miss its polyfill for the modern browsers, though our browserlist query includes modern browsers which do not have replaceAll implementation

Reproduction

https://stackblitz.com/edit/vitejs-vite-pzezqt

System Info

System:
    OS: macOS 11.4
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 248.95 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.14.0 - ~/.nvm/versions/node/v16.14.0/bin/node
    Yarn: 1.22.17 - ~/.yarn/bin/yarn
    npm: 8.3.1 - ~/.nvm/versions/node/v16.14.0/bin/npm
    Watchman: 2022.02.14.00 - /usr/local/bin/watchman
  Browsers:
    Brave Browser: 99.1.36.117
    Chrome: 99.0.4844.83
    Edge: 99.0.1150.52
    Firefox: 98.0.1
    Safari: 14.1.1
  npmPackages:
    @vitejs/plugin-legacy: ^1.6.4 => 1.7.1 
    @vitejs/plugin-react: ^1.1.4 => 1.2.0 
    vite: 2.7.13 => 2.7.13

Used Package Manager

yarn

Logs

No response

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• 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 https://github.com/vuejs/core instead.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[bluwy]

The repro doesn't look like a complete reproduction with browserlist and replaceAll usage. I've forked it and added them: https://stackblitz.com/edit/vitejs-vite-v17uto?file=vite.config.js (without browserlist, but with explicit polyfill, but the issue still persists)

[ninoman]

thank you @bluwy , it seems I forgot to add the replaceAll usage. Here is the updated repo

https://stackblitz.com/edit/vitejs-vite-uaf4oe

[sapphi-red]

Maybe you need to set modernPolyfills to true?

[ninoman]

@sapphi-red I see that it is not recommended as it will include a big chunk, so I wonder why it can't do same autodetection and only include polyfills for the methods that are used, looking into the browser list for the targeting browsers

[sapphi-red]

It does do auto detection. But because core-js cares many edge cases, it leads to including many polyfills.

For example, this project (which you provided for reproduction) will include the following polyfills.

• 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.regexp.exec.js
• core-js/modules/es.string.replace.js
• core-js/modules/esnext.string.replace-all.js

A compat data of es.string.replace and esnext.string.replace-all here maybe interesting.
It shows String::replaceAll is supported with Safari 13.1+ but String::replace is supported only with Safari 14+. This differs from caniuse / MDN browser compat data.

It seems like there is a edge case bug with Safari <13.1 and String::replaceAll polyfill uses String::replace inside.
This is the reason why es.string.replace.js is included.

[ninoman]

I see, it seems the best idea to set moderPolyfills to try, to avoid missing some polyfills

[sapphi-red]

I close this issue because I think it is solved.
Please re-open this if it is not solved.