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: exclude missing dependencies from optimization during SSR #5017

Merged
merged 1 commit into from
Sep 23, 2021
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
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 @@ -120,7 +119,7 @@ export function esbuildDepPlugin(
namespace: 'browser-external'
}
}
if (isExternalUrl(resolved) || (ssr && isBuiltin(id))) {
if (isExternalUrl(resolved)) {
ygj6 marked this conversation as resolved.
Show resolved Hide 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