fix(build): ensure amd bundles request require to be injected (#20861)

Co-authored-by: sapphi-red <[email protected]>
Co-authored-by: Bjorn Lu <[email protected]>

Author: 3 people

packages/vite/src/node/__tests__/plugins/completeAmdWrap.spec.ts

@@ -0,0 +1,46 @@
+import { describe, expect, test } from 'vitest'
+import { completeAmdWrapPlugin } from '../../plugins/completeAmdWrap'
+
+async function createCompleteAmdWrapPluginRenderChunk() {
+  const instance = completeAmdWrapPlugin()
+
+  return async (code: string) => {
+    // @ts-expect-error transform.handler should exist
+    const result = await instance.renderChunk.call(instance, code, 'foo.ts', {
+      format: 'amd',
+    })
+    return result?.code || result
+  }
+}
+
+describe('completeAmdWrapPlugin', async () => {
+  const renderChunk = await createCompleteAmdWrapPluginRenderChunk()
+
+  describe('adds require parameter', async () => {
+    test('without other dependencies', async () => {
+      expect(
+        await renderChunk('define((function() { } ))'),
+      ).toMatchInlineSnapshot(`"define(["require"], (function(require) { } ))"`)
+    })
+
+    test('with other dependencies', async () => {
+      expect(
+        await renderChunk(
+          'define(["vue", "vue-router"], function(vue, vueRouter) { } ))',
+        ),
+      ).toMatchInlineSnapshot(
+        `"define(["require", "vue", "vue-router"], (function(require, vue, vueRouter) { } ))"`,
+      )
+    })
+
+    test("only if require isn't injected already", async () => {
+      expect(
+        await renderChunk('define(["require"], function(require) { } ))'),
+      ).toMatchInlineSnapshot(`"define(["require"], (function(require) { } ))"`)
+
+      expect(
+        await renderChunk(`define(['require'], function(require) { } ))`),
+      ).toMatchInlineSnapshot(`"define(['require'], (function(require) { } ))"`)
+    })
+  })
+})

packages/vite/src/node/build.ts

@@ -65,6 +65,7 @@ import {
   resolveChokidarOptions,
   resolveEmptyOutDir,
 } from './watch'
+import { completeAmdWrapPlugin } from './plugins/completeAmdWrap'
 import { completeSystemWrapPlugin } from './plugins/completeSystemWrap'
 import { webWorkerPostPlugin } from './plugins/worker'
 import { getHookHandler } from './plugins'
@@ -477,6 +478,7 @@ export async function resolveBuildPlugins(config: ResolvedConfig): Promise<{
 }> {
   return {
     pre: [
+      completeAmdWrapPlugin(),
       completeSystemWrapPlugin(),
       ...(!config.isWorker ? [prepareOutDirPlugin()] : []),
       perEnvironmentPlugin('commonjs', (environment) => {

packages/vite/src/node/plugins/completeAmdWrap.ts

@@ -0,0 +1,30 @@
+import type { Plugin } from '../plugin'
+
+/**
+ * ensure amd bundles request `require` to be injected
+ */
+export function completeAmdWrapPlugin(): Plugin {
+  const AmdWrapRE =
+    /\bdefine\((?:\s*\[([^\]]*)\],)?\s*(?:\(\s*)?function\s*\(([^)]*)\)\s*\{/g
+
+  return {
+    name: 'vite:force-amd-wrap-require',
+    renderChunk(code, _chunk, opts) {
+      if (opts.format !== 'amd') return
+
+      return {
+        code: code.replace(AmdWrapRE, (_, deps, params) => {
+          if (deps?.includes(`"require"`) || deps?.includes(`'require'`)) {
+            return `define([${deps}], (function(${params}) {`
+          }
+
+          const newDeps = deps ? `"require", ${deps}` : '"require"'
+          const newParams = params.trim() ? `require, ${params}` : 'require'
+
+          return `define([${newDeps}], (function(${newParams}) {`
+        }),
+        map: null, // no need to generate sourcemap as no mapping exists for the wrapper
+      }
+    },
+  }
+}

playground/amd/__tests__/amd.spec.ts

@@ -0,0 +1,17 @@
+import { describe, expect, test } from 'vitest'
+import { browserLogs, getBg, isBuild, page } from '~utils'
+
+describe.runIf(isBuild)('build', () => {
+  test('should have no 404s', async () => {
+    await page.waitForLoadState('networkidle')
+    browserLogs.forEach((msg) => {
+      expect(msg).not.toMatch('404')
+    })
+  })
+
+  test('asset url is correct with `base: "."`', async () => {
+    await expect
+      .poll(() => getBg('.assets'))
+      .toMatch(/\/assets\/asset-[-\w]{8}\.png/)
+  })
+})

playground/amd/asset.png

@@ -0,0 +1,5 @@
+import asset from './asset.png'
+
+export default function pluginMain() {
+  return asset
+}

playground/amd/index.ts

@@ -0,0 +1,14 @@
+{
+  "name": "@vitejs/test-amd",
+  "private": true,
+  "version": "0.0.0",
+  "type": "module",
+  "scripts": {
+    "dev": "vite",
+    "build": "vite build",
+    "preview": "vite preview"
+  },
+  "dependencies": {
+    "requirejs": "^2.3.7"
+  }
+}

playground/amd/package.json

@@ -0,0 +1,30 @@
+<!doctype html>
+<html lang="en">
+  <head>
+    <meta charset="UTF-8" />
+    <title>amd</title>
+  </head>
+  <body>
+    <h1>AMD</h1>
+    <p>An image should be shown below</p>
+    <div
+      class="assets"
+      style="background-size: cover; height: 100px; width: 100px"
+    ></div>
+
+    <script src="/npm/requirejs.js"></script>
+    <script type="text/javascript">
+      requirejs(
+        ['assets/plugin.js'],
+        (plugin) => {
+          const url = plugin()
+          document.querySelector('.assets').style.backgroundImage =
+            `url(${url})`
+        },
+        (err) => {
+          console.error('Error loading plugin', err)
+        },
+      )
+    </script>
+  </body>
+</html>

playground/amd/public/index.html

@@ -0,0 +1,53 @@
+import fs from 'node:fs/promises'
+import path from 'node:path'
+import { type Connect, defineConfig } from 'vite'
+
+export default defineConfig({
+  base: './',
+  build: {
+    outDir: 'dist/nested',
+    rollupOptions: {
+      preserveEntrySignatures: 'strict',
+      input: {
+        plugin: path.resolve(import.meta.dirname, './index.ts'),
+      },
+      output: {
+        format: 'amd',
+        entryFileNames: 'assets/[name].js',
+      },
+    },
+  },
+  plugins: [
+    {
+      name: 'serve-npm-code-directly',
+      configureServer({ middlewares }) {
+        middlewares.use(serveNpmCodeDirectlyMiddleware)
+      },
+      configurePreviewServer({ middlewares }) {
+        middlewares.use(serveNpmCodeDirectlyMiddleware)
+      },
+    },
+  ],
+  appType: 'mpa', // to cause 404 for incorrect URLs
+})
+
+const npmDirectServeConfig = {
+  '/npm/requirejs.js': 'requirejs/require.js',
+}
+const serveNpmCodeDirectlyMiddleware: Connect.NextHandleFunction = async (
+  req,
+  res,
+  next,
+) => {
+  for (const [url, file] of Object.entries(npmDirectServeConfig)) {
+    if (req.originalUrl === url) {
+      const code = await fs.readFile(
+        new URL(`./node_modules/${file}`, import.meta.url),
+      )
+      res.setHeader('Content-Type', 'text/javascript')
+      res.end(code)
+      return
+    }
+  }
+  next()
+}