Skip to content

Commit 1ec0176

Browse files
sapphi-redsapphi-red
authoredNov 30, 2022
fix(ssr): preserve require for external node (#11057)
·
v6.3.0-beta.0create-vite@4.0.0-beta.0
1 parent 23d79b8 commit 1ec0176

File tree

16 files changed

+290
-72
lines changed

16 files changed

+290
-72
lines changed
 

‎packages/vite/src/node/optimizer/esbuildDepPlugin.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,10 @@ module.exports = Object.create(new Proxy({}, {
263263

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

281284
build.onResolve({ filter }, (args) => {
282-
if (args.kind === 'require-call') {
285+
// preserve `require` for node because it's more accurate than converting it to import
286+
if (args.kind === 'require-call' && platform !== 'node') {
283287
return {
284288
path: args.path,
285289
namespace: cjsExternalFacadeNamespace

‎packages/vite/src/node/optimizer/index.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,7 @@ export async function runOptimizeDeps(
578578

579579
const plugins = [...pluginsFromConfig]
580580
if (external.length) {
581-
plugins.push(esbuildCjsExternalPlugin(external))
581+
plugins.push(esbuildCjsExternalPlugin(external, platform))
582582
}
583583
plugins.push(esbuildDepPlugin(flatIdDeps, external, config, ssr))
584584

‎playground/ssr-noexternal/__tests__/serve.ts

+52
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// this is automatically detected by playground/vitestSetup.ts and will replace
2+
// the default e2e test serve behavior
3+
4+
import path from 'node:path'
5+
import kill from 'kill-port'
6+
import { hmrPorts, isBuild, ports, rootDir } from '~utils'
7+
8+
export const port = ports['ssr-noexternal']
9+
10+
export async function serve(): Promise<{ close(): Promise<void> }> {
11+
if (isBuild) {
12+
// build first
13+
const { build } = await import('vite')
14+
// server build
15+
await build({
16+
root: rootDir,
17+
logLevel: 'silent',
18+
build: {
19+
ssr: 'src/entry-server.js'
20+
}
21+
})
22+
}
23+
24+
await kill(port)
25+
26+
const { createServer } = await import(path.resolve(rootDir, 'server.js'))
27+
const { app, vite } = await createServer(
28+
rootDir,
29+
isBuild,
30+
hmrPorts['ssr-noexternal']
31+
)
32+
33+
return new Promise((resolve, reject) => {
34+
try {
35+
const server = app.listen(port, () => {
36+
resolve({
37+
// for test teardown
38+
async close() {
39+
await new Promise((resolve) => {
40+
server.close(resolve)
41+
})
42+
if (vite) {
43+
await vite.close()
44+
}
45+
}
46+
})
47+
})
48+
} catch (e) {
49+
reject(e)
50+
}
51+
})
52+
}

‎playground/ssr-noexternal/__tests__/ssr-noexternal.spec.ts

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import { expect, test } from 'vitest'
2+
import { port } from './serve'
3+
import { page } from '~utils'
4+
5+
const url = `http://localhost:${port}`
6+
7+
test('message from require-external-cjs', async () => {
8+
await page.goto(url)
9+
expect(await page.textContent('.require-external-cjs')).toMatch('foo')
10+
})

‎playground/ssr-noexternal/external-cjs/import.mjs

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
throw new Error('shouldnt be loaded')

‎playground/ssr-noexternal/external-cjs/package.json

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"name": "@vitejs/external-cjs",
3+
"private": true,
4+
"version": "0.0.0",
5+
"exports": {
6+
"require": "./require.cjs",
7+
"import": "./import.mjs"
8+
}
9+
}

‎playground/ssr-noexternal/external-cjs/require.cjs

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
module.exports = 'foo'

‎playground/ssr-noexternal/index.html

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<!DOCTYPE html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="UTF-8" />
5+
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
6+
<title>Vite App</title>
7+
</head>
8+
<body>
9+
<div id="app"><!--app-html--></div>
10+
</body>
11+
</html>

‎playground/ssr-noexternal/package.json

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{
2+
"name": "@vitejs/test-ssr-noexternal",
3+
"private": true,
4+
"version": "0.0.0",
5+
"type": "module",
6+
"scripts": {
7+
"dev": "node server",
8+
"build": "vite build --ssr src/entry-server.js",
9+
"serve": "NODE_ENV=production node server",
10+
"debug": "node --inspect-brk server"
11+
},
12+
"dependencies": {
13+
"@vitejs/external-cjs": "file:./external-cjs",
14+
"@vitejs/require-external-cjs": "file:./require-external-cjs",
15+
"express": "^4.18.2"
16+
}
17+
}

‎playground/ssr-noexternal/require-external-cjs/main.js

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
module.exports = require('@vitejs/external-cjs')

‎playground/ssr-noexternal/require-external-cjs/package.json

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"name": "@vitejs/require-external-cjs",
3+
"type": "commonjs",
4+
"private": true,
5+
"version": "0.0.0",
6+
"main": "main.js",
7+
"dependencies": {
8+
"@vitejs/external-cjs": "file:../external-cjs"
9+
}
10+
}

‎playground/ssr-noexternal/server.js

+87
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import fs from 'node:fs'
2+
import path from 'node:path'
3+
import { fileURLToPath } from 'node:url'
4+
import express from 'express'
5+
6+
const __dirname = path.dirname(fileURLToPath(import.meta.url))
7+
8+
const isTest = process.env.VITEST
9+
10+
export async function createServer(
11+
root = process.cwd(),
12+
isProd = process.env.NODE_ENV === 'production',
13+
hmrPort
14+
) {
15+
const resolve = (p) => path.resolve(__dirname, p)
16+
17+
const indexProd = isProd
18+
? fs.readFileSync(resolve('index.html'), 'utf-8')
19+
: ''
20+
21+
const app = express()
22+
23+
/**
24+
* @type {import('vite').ViteDevServer}
25+
*/
26+
let vite
27+
if (!isProd) {
28+
vite = await (
29+
await import('vite')
30+
).createServer({
31+
root,
32+
logLevel: isTest ? 'error' : 'info',
33+
server: {
34+
middlewareMode: true,
35+
watch: {
36+
// During tests we edit the files too fast and sometimes chokidar
37+
// misses change events, so enforce polling for consistency
38+
usePolling: true,
39+
interval: 100
40+
},
41+
hmr: {
42+
port: hmrPort
43+
}
44+
},
45+
appType: 'custom'
46+
})
47+
app.use(vite.middlewares)
48+
}
49+
50+
app.use('*', async (req, res) => {
51+
try {
52+
const url = req.originalUrl
53+
54+
let template, render
55+
if (!isProd) {
56+
// always read fresh template in dev
57+
template = fs.readFileSync(resolve('index.html'), 'utf-8')
58+
template = await vite.transformIndexHtml(url, template)
59+
render = (await vite.ssrLoadModule('/src/entry-server.js')).render
60+
} else {
61+
template = indexProd
62+
// @ts-ignore
63+
render = (await import('./dist/entry-server.js')).render
64+
}
65+
66+
const appHtml = await render(url)
67+
68+
const html = template.replace(`<!--app-html-->`, appHtml)
69+
70+
res.status(200).set({ 'Content-Type': 'text/html' }).end(html)
71+
} catch (e) {
72+
!isProd && vite.ssrFixStacktrace(e)
73+
console.log(e.stack)
74+
res.status(500).end(e.stack)
75+
}
76+
})
77+
78+
return { app, vite }
79+
}
80+
81+
if (!isTest) {
82+
createServer().then(({ app }) =>
83+
app.listen(5173, () => {
84+
console.log('http://localhost:5173')
85+
})
86+
)
87+
}

‎playground/ssr-noexternal/src/entry-server.js

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import requireExternalCjs from '@vitejs/require-external-cjs'
2+
3+
export async function render(url) {
4+
let html = ''
5+
6+
html += `\n<p class="require-external-cjs">message from require-external-cjs: ${requireExternalCjs}</p>`
7+
8+
return html + '\n'
9+
}

‎playground/ssr-noexternal/vite.config.js

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import module from 'node:module'
2+
import { defineConfig } from 'vite'
3+
4+
export default defineConfig({
5+
ssr: {
6+
noExternal: ['@vitejs/require-external-cjs'],
7+
external: ['@vitejs/external-cjs'],
8+
optimizeDeps: {
9+
disabled: false
10+
}
11+
},
12+
build: {
13+
target: 'esnext',
14+
minify: false,
15+
rollupOptions: {
16+
external: ['@vitejs/external-cjs']
17+
},
18+
commonjsOptions: {
19+
include: []
20+
}
21+
}
22+
})

‎playground/test-utils.ts

+9-7
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,11 @@ export const ports = {
2424
'legacy/client-and-ssr': 9523,
2525
'ssr-deps': 9600,
2626
'ssr-html': 9601,
27-
'ssr-pug': 9602,
28-
'ssr-react': 9603,
29-
'ssr-vue': 9604,
30-
'ssr-webworker': 9605,
27+
'ssr-noexternal': 9602,
28+
'ssr-pug': 9603,
29+
'ssr-react': 9604,
30+
'ssr-vue': 9605,
31+
'ssr-webworker': 9606,
3132
'css/postcss-caching': 5005,
3233
'css/postcss-plugins-different-dir': 5006,
3334
'css/dynamic-import': 5007
@@ -36,9 +37,10 @@ export const hmrPorts = {
3637
'optimize-missing-deps': 24680,
3738
'ssr-deps': 24681,
3839
'ssr-html': 24682,
39-
'ssr-pug': 24683,
40-
'ssr-react': 24684,
41-
'ssr-vue': 24685
40+
'ssr-noexternal': 24683,
41+
'ssr-pug': 24684,
42+
'ssr-react': 24685,
43+
'ssr-vue': 24686
4244
}
4345

4446
const hexToNameMap: Record<string, string> = {}

‎pnpm-lock.yaml

+44-62
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)
Please sign in to comment.