Legacy plugin produces broken build with renderModernChunks=false & polyfills=false

[k-yle]

Describe the bug

plugins: [
      react(),
      legacy({
        renderModernChunks: false,
        polyfills: false,
      }),
    ],

When using the legacy plugin with these two options, the compiled output is broken because SystemJS is not injected.

Example - note how dist/index.html refers to System, but SystemJS is not imported.

I'm happy to make a PR to fix this, we are currently bypassing this bug by adding this snippet to packages/plugin-legacy/src/index.ts:594:

      // 6. Inject SystemJS
      if (!genModern && !options.polyfills) {
        const systemJsRuntime = fs
          .readFileSync(require.resolve('systemjs/dist/s.min.js'), 'utf8')
          .split("//# sourceMappingURL")[0]
        html = html.replace(/<\/head>/g, `<script>${systemJsRuntime}</script></head>`)
      }

Reproduction

https://github.com/k-yle/vite-legacy-broken-build

Steps to reproduce

• clone the example repo
• npm i
• npm run build
• try to open index.html - note how SystemJS is missing.

System Info

System:
    OS: Windows 10 10.0.*****
    CPU: (*) *******
    Memory: ***********
  Binaries:
    Node: 18.16.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - C:\Program Files\nodejs\yarn.CMD
    npm: 9.5.1 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.****.****.*), Chromium (116.0.****.*)
    Internet Explorer: 11.0.****.*
  npmPackages:
    @vitejs/plugin-legacy: ^4.1.1 => 4.1.1 
    @vitejs/plugin-react: ^4.0.4 => 4.0.4 
    vite: ^4.4.9 => 4.4.9

Used Package Manager

npm

Logs

Click to expand!

warning package.json: No license field
$ vite build
vite v4.4.9 building for production...
✓ 32 modules transformed.
dist/index.html                             0.42 kB │ gzip:  0.26 kB
dist/assets/file2-legacy-4d4e3654.js        0.09 kB │ gzip:  0.10 kB
dist/assets/polyfills-legacy-a26809ba.js   91.53 kB │ gzip: 36.33 kB
dist/assets/index-legacy-c4fde549.js      142.03 kB │ gzip: 45.52 kB
✓ built in 6.45s
Done in 7.28s.

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 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.

[sapphi-red]

SystemJS is included in the polyfill chunk. But this condition is skipping the polyfill chunk generation.

vite/packages/plugin-legacy/src/index.ts

Line 258 in 4fb617f

if (legacyPolyfills.size) {

I guess simply removing this condition would fix it.