Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ssr): preserve require for external node #11057

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions packages/vite/src/node/optimizer/esbuildDepPlugin.ts
Expand Up @@ -263,7 +263,10 @@ module.exports = Object.create(new Proxy({}, {

// esbuild doesn't transpile `require('foo')` into `import` statements if 'foo' is externalized
// https://github.com/evanw/esbuild/issues/566#issuecomment-735551834
export function esbuildCjsExternalPlugin(externals: string[]): Plugin {
export function esbuildCjsExternalPlugin(
externals: string[],
platform: 'node' | 'browser'
): Plugin {
return {
name: 'cjs-external',
setup(build) {
Expand All @@ -279,7 +282,8 @@ export function esbuildCjsExternalPlugin(externals: string[]): Plugin {
})

build.onResolve({ filter }, (args) => {
if (args.kind === 'require-call') {
// preserve `require` for node because it's more accurate than converting it to import
if (args.kind === 'require-call' && platform !== 'node') {
return {
path: args.path,
namespace: cjsExternalFacadeNamespace
Expand Down
2 changes: 1 addition & 1 deletion packages/vite/src/node/optimizer/index.ts
Expand Up @@ -578,7 +578,7 @@ export async function runOptimizeDeps(

const plugins = [...pluginsFromConfig]
if (external.length) {
plugins.push(esbuildCjsExternalPlugin(external))
plugins.push(esbuildCjsExternalPlugin(external, platform))
}
plugins.push(esbuildDepPlugin(flatIdDeps, external, config, ssr))

Expand Down
52 changes: 52 additions & 0 deletions playground/ssr-noexternal/__tests__/serve.ts
@@ -0,0 +1,52 @@
// this is automatically detected by playground/vitestSetup.ts and will replace
// the default e2e test serve behavior

import path from 'node:path'
import kill from 'kill-port'
import { hmrPorts, isBuild, ports, rootDir } from '~utils'

export const port = ports['ssr-noexternal']

export async function serve(): Promise<{ close(): Promise<void> }> {
if (isBuild) {
// build first
const { build } = await import('vite')
// server build
await build({
root: rootDir,
logLevel: 'silent',
build: {
ssr: 'src/entry-server.js'
}
})
}

await kill(port)

const { createServer } = await import(path.resolve(rootDir, 'server.js'))
const { app, vite } = await createServer(
rootDir,
isBuild,
hmrPorts['ssr-noexternal']
)

return new Promise((resolve, reject) => {
try {
const server = app.listen(port, () => {
resolve({
// for test teardown
async close() {
await new Promise((resolve) => {
server.close(resolve)
})
if (vite) {
await vite.close()
}
}
})
})
} catch (e) {
reject(e)
}
})
}
10 changes: 10 additions & 0 deletions playground/ssr-noexternal/__tests__/ssr-noexternal.spec.ts
@@ -0,0 +1,10 @@
import { expect, test } from 'vitest'
import { port } from './serve'
import { page } from '~utils'

const url = `http://localhost:${port}`

test('message from require-external-cjs', async () => {
await page.goto(url)
expect(await page.textContent('.require-external-cjs')).toMatch('foo')
})
1 change: 1 addition & 0 deletions playground/ssr-noexternal/external-cjs/import.mjs
@@ -0,0 +1 @@
throw new Error('shouldnt be loaded')
9 changes: 9 additions & 0 deletions playground/ssr-noexternal/external-cjs/package.json
@@ -0,0 +1,9 @@
{
"name": "@vitejs/external-cjs",
"private": true,
"version": "0.0.0",
"exports": {
"require": "./require.cjs",
"import": "./import.mjs"
}
}
1 change: 1 addition & 0 deletions playground/ssr-noexternal/external-cjs/require.cjs
@@ -0,0 +1 @@
module.exports = 'foo'
11 changes: 11 additions & 0 deletions playground/ssr-noexternal/index.html
@@ -0,0 +1,11 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Vite App</title>
</head>
<body>
<div id="app"><!--app-html--></div>
</body>
</html>
17 changes: 17 additions & 0 deletions playground/ssr-noexternal/package.json
@@ -0,0 +1,17 @@
{
"name": "@vitejs/test-ssr-noexternal",
"private": true,
"version": "0.0.0",
"type": "module",
"scripts": {
"dev": "node server",
"build": "vite build --ssr src/entry-server.js",
"serve": "NODE_ENV=production node server",
"debug": "node --inspect-brk server"
},
"dependencies": {
"@vitejs/external-cjs": "file:./external-cjs",
"@vitejs/require-external-cjs": "file:./require-external-cjs",
"express": "^4.18.2"
}
}
1 change: 1 addition & 0 deletions playground/ssr-noexternal/require-external-cjs/main.js
@@ -0,0 +1 @@
module.exports = require('@vitejs/external-cjs')
10 changes: 10 additions & 0 deletions playground/ssr-noexternal/require-external-cjs/package.json
@@ -0,0 +1,10 @@
{
"name": "@vitejs/require-external-cjs",
"type": "commonjs",
"private": true,
"version": "0.0.0",
"main": "main.js",
"dependencies": {
"@vitejs/external-cjs": "file:../external-cjs"
}
}
87 changes: 87 additions & 0 deletions playground/ssr-noexternal/server.js
@@ -0,0 +1,87 @@
import fs from 'node:fs'
import path from 'node:path'
import { fileURLToPath } from 'node:url'
import express from 'express'

const __dirname = path.dirname(fileURLToPath(import.meta.url))

const isTest = process.env.VITEST

export async function createServer(
root = process.cwd(),
isProd = process.env.NODE_ENV === 'production',
hmrPort
) {
const resolve = (p) => path.resolve(__dirname, p)

const indexProd = isProd
? fs.readFileSync(resolve('index.html'), 'utf-8')
: ''

const app = express()

/**
* @type {import('vite').ViteDevServer}
*/
let vite
if (!isProd) {
vite = await (
await import('vite')
).createServer({
root,
logLevel: isTest ? 'error' : 'info',
server: {
middlewareMode: true,
watch: {
// During tests we edit the files too fast and sometimes chokidar
// misses change events, so enforce polling for consistency
usePolling: true,
interval: 100
},
hmr: {
port: hmrPort
}
},
appType: 'custom'
})
app.use(vite.middlewares)
}

app.use('*', async (req, res) => {
try {
const url = req.originalUrl

let template, render
if (!isProd) {
// always read fresh template in dev
template = fs.readFileSync(resolve('index.html'), 'utf-8')
template = await vite.transformIndexHtml(url, template)
render = (await vite.ssrLoadModule('/src/entry-server.js')).render
} else {
template = indexProd
// @ts-ignore
render = (await import('./dist/entry-server.js')).render
}

const appHtml = await render(url)

const html = template.replace(`<!--app-html-->`, appHtml)

res.status(200).set({ 'Content-Type': 'text/html' }).end(html)
} catch (e) {
!isProd && vite.ssrFixStacktrace(e)
console.log(e.stack)
res.status(500).end(e.stack)
}
})

return { app, vite }
}

if (!isTest) {
createServer().then(({ app }) =>
app.listen(5173, () => {
console.log('http://localhost:5173')
})
)
}
9 changes: 9 additions & 0 deletions playground/ssr-noexternal/src/entry-server.js
@@ -0,0 +1,9 @@
import requireExternalCjs from '@vitejs/require-external-cjs'

export async function render(url) {
let html = ''

html += `\n<p class="require-external-cjs">message from require-external-cjs: ${requireExternalCjs}</p>`

return html + '\n'
}
22 changes: 22 additions & 0 deletions playground/ssr-noexternal/vite.config.js
@@ -0,0 +1,22 @@
import module from 'node:module'
import { defineConfig } from 'vite'

export default defineConfig({
ssr: {
noExternal: ['@vitejs/require-external-cjs'],
external: ['@vitejs/external-cjs'],
optimizeDeps: {
disabled: false
}
},
build: {
target: 'esnext',
minify: false,
rollupOptions: {
external: ['@vitejs/external-cjs']
},
commonjsOptions: {
include: []
}
}
})
16 changes: 9 additions & 7 deletions playground/test-utils.ts
Expand Up @@ -24,10 +24,11 @@ export const ports = {
'legacy/client-and-ssr': 9523,
'ssr-deps': 9600,
'ssr-html': 9601,
'ssr-pug': 9602,
'ssr-react': 9603,
'ssr-vue': 9604,
'ssr-webworker': 9605,
'ssr-noexternal': 9602,
'ssr-pug': 9603,
'ssr-react': 9604,
'ssr-vue': 9605,
'ssr-webworker': 9606,
'css/postcss-caching': 5005,
'css/postcss-plugins-different-dir': 5006,
'css/dynamic-import': 5007
Expand All @@ -36,9 +37,10 @@ export const hmrPorts = {
'optimize-missing-deps': 24680,
'ssr-deps': 24681,
'ssr-html': 24682,
'ssr-pug': 24683,
'ssr-react': 24684,
'ssr-vue': 24685
'ssr-noexternal': 24683,
'ssr-pug': 24684,
'ssr-react': 24685,
'ssr-vue': 24686
}

const hexToNameMap: Record<string, string> = {}
Expand Down