Skip to content

Commit

Permalink
fix: exclude missing dependencies from optimization during SSR (vitej…
Browse files Browse the repository at this point in the history
…s#5017)

Co-authored-by: ygj6 <chieffochen2021>
  • Loading branch information
ygj6 authored and aleclarson committed Nov 8, 2021
1 parent 0f900ea commit 4a50fce
Show file tree
Hide file tree
Showing 14 changed files with 180 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { port } from './serve'
import fetch from 'node-fetch'
import { untilUpdated } from '../../testUtils'

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

test('*', async () => {
await page.goto(url)
// reload page to get optimized missing deps
await page.reload()
await untilUpdated(() => page.textContent('div'), 'Client')

// raw http request
const aboutHtml = await (await fetch(url)).text()
expect(aboutHtml).toMatch('Server')
})
36 changes: 36 additions & 0 deletions packages/playground/optimize-missing-deps/__test__/serve.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// @ts-check
// this is automtically detected by scripts/jestPerTestSetup.ts and will replace
// the default e2e test serve behavior

const path = require('path')

const port = (exports.port = 9529)

/**
* @param {string} root
* @param {boolean} isProd
*/
exports.serve = async function serve(root, isProd) {
const { createServer } = require(path.resolve(root, 'server.js'))
const { app, vite } = await createServer(root, isProd)

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)
}
})
}
11 changes: 11 additions & 0 deletions packages/playground/optimize-missing-deps/index.html
Original file line number Diff line number Diff line change
@@ -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>
<!--app-html-->
</body>
</html>
3 changes: 3 additions & 0 deletions packages/playground/optimize-missing-deps/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { sayName } from 'missing-dep'

export const name = sayName()
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { name } from 'multi-entry-dep'

export function sayName() {
return name
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "missing-dep",
"version": "0.0.0",
"main": "index.js",
"dependencies": {
"multi-entry-dep": "file:../multi-entry-dep"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
exports.name = 'Client'
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const path = require('path')

exports.name = path.normalize('./Server')
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "multi-entry-dep",
"version": "0.0.0",
"main": "index.js",
"browser": {
"./index.js": "./index.browser.js"
}
}
17 changes: 17 additions & 0 deletions packages/playground/optimize-missing-deps/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"name": "optimize-missing-deps",
"private": true,
"version": "0.0.0",
"scripts": {
"dev": "node server"
},
"workspaces": {
"packages": [
"./*"
]
},
"dependencies": {
"missing-dep": "file:./missing-dep",
"multi-entry-dep": "file:./multi-entry-dep"
}
}
60 changes: 60 additions & 0 deletions packages/playground/optimize-missing-deps/server.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// @ts-check
const fs = require('fs')
const path = require('path')
const express = require('express')

const isTest = process.env.NODE_ENV === 'test' || !!process.env.VITE_TEST_BUILD

async function createServer(root = process.cwd()) {
const resolve = (p) => path.resolve(__dirname, p)

const app = express()

/**
* @type {import('vite').ViteDevServer}
*/
const vite = await require('vite').createServer({
root,
logLevel: isTest ? 'error' : 'info',
server: { middlewareMode: 'ssr' }
})
app.use(vite.middlewares)

app.use('*', async (req, res) => {
try {
let template = fs.readFileSync(resolve('index.html'), 'utf-8')
template = await vite.transformIndexHtml(req.originalUrl, template)

// this will import missing deps nest built-in deps that should not be optimized
const { name } = await vite.ssrLoadModule('./main.js')

// this will import missing deps that should be optimized correctly
const appHtml = `<div id="app">${name}</div>
<script type='module'>
import { name } from './main.js'
document.getElementById('app').innerText = name
</script>`

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

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

return { app, vite }
}

if (!isTest) {
createServer().then(({ app }) =>
app.listen(3000, () => {
console.log('http://localhost:3000')
})
)
}

// for test use
exports.createServer = createServer
5 changes: 2 additions & 3 deletions packages/vite/src/node/optimizer/esbuildDepPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ import {
isRunningWithYarnPnp,
flattenId,
normalizePath,
isExternalUrl,
isBuiltin
isExternalUrl
} from '../utils'
import { browserExternalId } from '../plugins/resolve'
import { ExportsData } from '.'
Expand Down Expand Up @@ -121,7 +120,7 @@ export function esbuildDepPlugin(
namespace: 'browser-external'
}
}
if (isExternalUrl(resolved) || (ssr && isBuiltin(id))) {
if (isExternalUrl(resolved)) {
return {
path: resolved,
external: true
Expand Down
3 changes: 2 additions & 1 deletion packages/vite/src/node/plugins/resolve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,8 @@ export function tryNodeResolve(
importer?.includes('node_modules') ||
exclude?.includes(pkgId) ||
exclude?.includes(id) ||
SPECIAL_QUERY_RE.test(resolved)
SPECIAL_QUERY_RE.test(resolved) ||
ssr
) {
// excluded from optimization
// Inject a version query to npm deps so that the browser
Expand Down
8 changes: 8 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5552,6 +5552,11 @@ minimist@^1.1.1, minimist@^1.2.0, minimist@^1.2.5:
resolved "https://registry.npmjs.org/minimist/-/minimist-1.2.5.tgz#67d66014b66a6a8aaa0c083c5fd58df4e4e97602"
integrity sha512-FM9nNUYrRBAELZQT3xeZQ7fmMOBg6nWNmJKTcgsJeaLstP/UODVpGsr5OhXhhXg6f+qtJ8uiZ+PUxkDWcgIXLw==

"missing-dep@file:./packages/playground/optimize-missing-deps/missing-dep":
version "0.0.0"
dependencies:
multi-entry-dep "file:../../../Library/Caches/Yarn/v6/npm-missing-dep-0.0.0-c3d33c60-1540-4e68-9410-6849e432ed4c-1632279325025/node_modules/multi-entry-dep"

mkdirp@1.0.4, mkdirp@~1.0.4:
version "1.0.4"
resolved "https://registry.npmjs.org/mkdirp/-/mkdirp-1.0.4.tgz#3eb5ed62622756d79a5f0e2a221dfebad75c2f7e"
Expand Down Expand Up @@ -5592,6 +5597,9 @@ ms@^2.1.1:
resolved "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz#574c8138ce1d2b5861f0b44579dbadd60c6615b2"
integrity sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==

"multi-entry-dep@file:./packages/playground/optimize-missing-deps/multi-entry-dep", "multi-entry-dep@file:packages/playground/optimize-missing-deps/multi-entry-dep":
version "0.0.0"

mustache@^4.2.0:
version "4.2.0"
resolved "https://registry.npmjs.org/mustache/-/mustache-4.2.0.tgz#e5892324d60a12ec9c2a73359edca52972bf6f64"
Expand Down

0 comments on commit 4a50fce

Please sign in to comment.