From 63cc25089b989c97bd207e35e18ca192b390805b Mon Sep 17 00:00:00 2001 From: Kyle Hensel Date: Sun, 24 Sep 2023 15:36:10 +1300 Subject: [PATCH 1/2] fix(legacy): fix broken build when renderModernChunks=false & polyfills=false --- packages/plugin-legacy/src/index.ts | 36 +++++++++---------- .../no-polyfills/no-polyfills.spec.ts | 20 +++++++++++ playground/legacy/no-polyfills.html | 3 ++ playground/legacy/no-polyfills.js | 1 + playground/legacy/package.json | 1 + playground/legacy/vite.config-no-polyfills.js | 29 +++++++++++++++ 6 files changed, 72 insertions(+), 18 deletions(-) create mode 100644 playground/legacy/__tests__/no-polyfills/no-polyfills.spec.ts create mode 100644 playground/legacy/no-polyfills.html create mode 100644 playground/legacy/no-polyfills.js create mode 100644 playground/legacy/vite.config-no-polyfills.js diff --git a/packages/plugin-legacy/src/index.ts b/packages/plugin-legacy/src/index.ts index 4fcdb2c11ddc16..f7bf484f64dd75 100644 --- a/packages/plugin-legacy/src/index.ts +++ b/packages/plugin-legacy/src/index.ts @@ -258,7 +258,7 @@ function viteLegacyPlugin(options: Options = {}): Plugin[] { } // legacy bundle - if (legacyPolyfills.size) { + if (options.polyfills !== false) { // check if the target needs Promise polyfill because SystemJS relies on it // https://github.com/systemjs/systemjs#ie11-support await detectPolyfills( @@ -266,26 +266,26 @@ function viteLegacyPlugin(options: Options = {}): Plugin[] { targets, legacyPolyfills, ) + } - isDebug && - console.log( - `[@vitejs/plugin-legacy] legacy polyfills:`, - legacyPolyfills, - ) - - await buildPolyfillChunk( - config.mode, + isDebug && + console.log( + `[@vitejs/plugin-legacy] legacy polyfills:`, legacyPolyfills, - bundle, - facadeToLegacyPolyfillMap, - // force using terser for legacy polyfill minification, since esbuild - // isn't legacy-safe - config.build, - 'iife', - opts, - options.externalSystemJS, ) - } + + await buildPolyfillChunk( + config.mode, + legacyPolyfills, + bundle, + facadeToLegacyPolyfillMap, + // force using terser for legacy polyfill minification, since esbuild + // isn't legacy-safe + config.build, + 'iife', + opts, + options.externalSystemJS, + ) }, } diff --git a/playground/legacy/__tests__/no-polyfills/no-polyfills.spec.ts b/playground/legacy/__tests__/no-polyfills/no-polyfills.spec.ts new file mode 100644 index 00000000000000..9563c569830c8f --- /dev/null +++ b/playground/legacy/__tests__/no-polyfills/no-polyfills.spec.ts @@ -0,0 +1,20 @@ +import { test } from 'vitest' +import { isBuild, page, untilUpdated, viteTestUrl } from '~utils' + +test('should load and execute the JS file', async () => { + await page.goto(viteTestUrl + '/no-polyfills.html') + await untilUpdated(() => page.textContent('main'), '👋', true) +}) + +test.runIf(isBuild)('includes a script tag for SystemJS', async () => { + await untilUpdated( + () => page.getAttribute('#vite-legacy-polyfill', 'src'), + /.\/assets\/polyfills-legacy-(.+)\.js/, + true, + ) + await untilUpdated( + () => page.getAttribute('#vite-legacy-entry', 'data-src'), + /.\/assets\/index-legacy-(.+)\.js/, + true, + ) +}) diff --git a/playground/legacy/no-polyfills.html b/playground/legacy/no-polyfills.html new file mode 100644 index 00000000000000..a91e4764055276 --- /dev/null +++ b/playground/legacy/no-polyfills.html @@ -0,0 +1,3 @@ + +
+ diff --git a/playground/legacy/no-polyfills.js b/playground/legacy/no-polyfills.js new file mode 100644 index 00000000000000..c806e68363501b --- /dev/null +++ b/playground/legacy/no-polyfills.js @@ -0,0 +1 @@ +document.querySelector('main').innerHTML = '👋' diff --git a/playground/legacy/package.json b/playground/legacy/package.json index d7aed1215a29db..053e33506c708d 100644 --- a/playground/legacy/package.json +++ b/playground/legacy/package.json @@ -8,6 +8,7 @@ "build": "vite build --debug legacy", "build:custom-filename": "vite --config ./vite.config-custom-filename.js build --debug legacy", "build:multiple-output": "vite --config ./vite.config-multiple-output.js build", + "build:no-polyfills": "vite --config ./vite.config-no-polyfills.js build", "debug": "node --inspect-brk ../../packages/vite/bin/vite", "preview": "vite preview" }, diff --git a/playground/legacy/vite.config-no-polyfills.js b/playground/legacy/vite.config-no-polyfills.js new file mode 100644 index 00000000000000..7498e4d25f19ad --- /dev/null +++ b/playground/legacy/vite.config-no-polyfills.js @@ -0,0 +1,29 @@ +import path from 'node:path' +import legacy from '@vitejs/plugin-legacy' +import { defineConfig } from 'vite' + +export default defineConfig({ + base: './', + plugins: [ + legacy({ + renderModernChunks: false, + polyfills: false, + }), + { + name: 'remove crossorigin attribute', + transformIndexHtml: (html) => html.replaceAll('crossorigin', ''), + enforce: 'post', + }, + ], + + build: { + rollupOptions: { + input: { + index: path.resolve(__dirname, 'no-polyfills.html'), + }, + }, + }, + testConfig: { + baseRoute: '/no-polyfills/', + }, +}) From 83b5a85446f91bf6abf6d47d8cfb594c92d4f170 Mon Sep 17 00:00:00 2001 From: Kyle Hensel Date: Tue, 26 Sep 2023 19:18:48 +1300 Subject: [PATCH 2/2] fix: don't emit an empty bundle --- packages/plugin-legacy/src/index.ts | 34 ++++++++++--------- .../no-polyfills-no-systemjs.spec.ts | 16 +++++++++ .../legacy/no-polyfills-no-systemjs.html | 3 ++ playground/legacy/no-polyfills-no-systemjs.js | 1 + playground/legacy/package.json | 1 + .../vite.config-no-polyfills-no-systemjs.js | 30 ++++++++++++++++ 6 files changed, 69 insertions(+), 16 deletions(-) create mode 100644 playground/legacy/__tests__/no-polyfills-no-systemjs/no-polyfills-no-systemjs.spec.ts create mode 100644 playground/legacy/no-polyfills-no-systemjs.html create mode 100644 playground/legacy/no-polyfills-no-systemjs.js create mode 100644 playground/legacy/vite.config-no-polyfills-no-systemjs.js diff --git a/packages/plugin-legacy/src/index.ts b/packages/plugin-legacy/src/index.ts index f7bf484f64dd75..4fb6e55b9baee0 100644 --- a/packages/plugin-legacy/src/index.ts +++ b/packages/plugin-legacy/src/index.ts @@ -268,24 +268,26 @@ function viteLegacyPlugin(options: Options = {}): Plugin[] { ) } - isDebug && - console.log( - `[@vitejs/plugin-legacy] legacy polyfills:`, + if (legacyPolyfills.size || !options.externalSystemJS) { + isDebug && + console.log( + `[@vitejs/plugin-legacy] legacy polyfills:`, + legacyPolyfills, + ) + + await buildPolyfillChunk( + config.mode, legacyPolyfills, + bundle, + facadeToLegacyPolyfillMap, + // force using terser for legacy polyfill minification, since esbuild + // isn't legacy-safe + config.build, + 'iife', + opts, + options.externalSystemJS, ) - - await buildPolyfillChunk( - config.mode, - legacyPolyfills, - bundle, - facadeToLegacyPolyfillMap, - // force using terser for legacy polyfill minification, since esbuild - // isn't legacy-safe - config.build, - 'iife', - opts, - options.externalSystemJS, - ) + } }, } diff --git a/playground/legacy/__tests__/no-polyfills-no-systemjs/no-polyfills-no-systemjs.spec.ts b/playground/legacy/__tests__/no-polyfills-no-systemjs/no-polyfills-no-systemjs.spec.ts new file mode 100644 index 00000000000000..609c45c0749b64 --- /dev/null +++ b/playground/legacy/__tests__/no-polyfills-no-systemjs/no-polyfills-no-systemjs.spec.ts @@ -0,0 +1,16 @@ +import { expect, test } from 'vitest' +import { isBuild, page, untilUpdated, viteTestUrl } from '~utils' + +test.runIf(isBuild)('includes only a single script tag', async () => { + await page.goto(viteTestUrl + '/no-polyfills-no-systemjs.html') + + await untilUpdated( + () => page.getAttribute('#vite-legacy-entry', 'data-src'), + /.\/assets\/index-legacy-(.+)\.js/, + true, + ) + + expect(await page.locator('script').count()).toBe(1) + expect(await page.locator('#vite-legacy-polyfill').count()).toBe(0) + expect(await page.locator('#vite-legacy-entry').count()).toBe(1) +}) diff --git a/playground/legacy/no-polyfills-no-systemjs.html b/playground/legacy/no-polyfills-no-systemjs.html new file mode 100644 index 00000000000000..878b86e5027e0c --- /dev/null +++ b/playground/legacy/no-polyfills-no-systemjs.html @@ -0,0 +1,3 @@ + +
+ diff --git a/playground/legacy/no-polyfills-no-systemjs.js b/playground/legacy/no-polyfills-no-systemjs.js new file mode 100644 index 00000000000000..c806e68363501b --- /dev/null +++ b/playground/legacy/no-polyfills-no-systemjs.js @@ -0,0 +1 @@ +document.querySelector('main').innerHTML = '👋' diff --git a/playground/legacy/package.json b/playground/legacy/package.json index 053e33506c708d..f1646c8a5f19b7 100644 --- a/playground/legacy/package.json +++ b/playground/legacy/package.json @@ -9,6 +9,7 @@ "build:custom-filename": "vite --config ./vite.config-custom-filename.js build --debug legacy", "build:multiple-output": "vite --config ./vite.config-multiple-output.js build", "build:no-polyfills": "vite --config ./vite.config-no-polyfills.js build", + "build:no-polyfills-no-systemjs": "vite --config ./vite.config-no-polyfills-no-systemjs.js build", "debug": "node --inspect-brk ../../packages/vite/bin/vite", "preview": "vite preview" }, diff --git a/playground/legacy/vite.config-no-polyfills-no-systemjs.js b/playground/legacy/vite.config-no-polyfills-no-systemjs.js new file mode 100644 index 00000000000000..6908a5298077cd --- /dev/null +++ b/playground/legacy/vite.config-no-polyfills-no-systemjs.js @@ -0,0 +1,30 @@ +import path from 'node:path' +import legacy from '@vitejs/plugin-legacy' +import { defineConfig } from 'vite' + +export default defineConfig({ + base: './', + plugins: [ + legacy({ + renderModernChunks: false, + polyfills: false, + externalSystemJS: true, + }), + { + name: 'remove crossorigin attribute', + transformIndexHtml: (html) => html.replaceAll('crossorigin', ''), + enforce: 'post', + }, + ], + + build: { + rollupOptions: { + input: { + index: path.resolve(__dirname, 'no-polyfills-no-systemjs.html'), + }, + }, + }, + testConfig: { + baseRoute: '/no-polyfills-no-systemjs/', + }, +})