From 2204afa34482384460507aa0149730ab05f99e31 Mon Sep 17 00:00:00 2001 From: ygj6 <7699524+ygj6@users.noreply.github.com> Date: Thu, 23 Sep 2021 14:05:11 +0800 Subject: [PATCH] fix: exclude missing dependencies from optimization during SSR (#5017) Co-authored-by: ygj6 --- .../__test__/optimize-missing-deps.spec.ts | 16 +++++ .../optimize-missing-deps/__test__/serve.js | 36 +++++++++++ .../optimize-missing-deps/index.html | 11 ++++ .../playground/optimize-missing-deps/main.js | 3 + .../missing-dep/index.js | 5 ++ .../missing-dep/package.json | 8 +++ .../multi-entry-dep/index.browser.js | 1 + .../multi-entry-dep/index.js | 3 + .../multi-entry-dep/package.json | 8 +++ .../optimize-missing-deps/package.json | 17 ++++++ .../optimize-missing-deps/server.js | 60 +++++++++++++++++++ .../src/node/optimizer/esbuildDepPlugin.ts | 5 +- packages/vite/src/node/plugins/resolve.ts | 3 +- yarn.lock | 8 +++ 14 files changed, 180 insertions(+), 4 deletions(-) create mode 100644 packages/playground/optimize-missing-deps/__test__/optimize-missing-deps.spec.ts create mode 100644 packages/playground/optimize-missing-deps/__test__/serve.js create mode 100644 packages/playground/optimize-missing-deps/index.html create mode 100644 packages/playground/optimize-missing-deps/main.js create mode 100644 packages/playground/optimize-missing-deps/missing-dep/index.js create mode 100644 packages/playground/optimize-missing-deps/missing-dep/package.json create mode 100644 packages/playground/optimize-missing-deps/multi-entry-dep/index.browser.js create mode 100644 packages/playground/optimize-missing-deps/multi-entry-dep/index.js create mode 100644 packages/playground/optimize-missing-deps/multi-entry-dep/package.json create mode 100644 packages/playground/optimize-missing-deps/package.json create mode 100644 packages/playground/optimize-missing-deps/server.js diff --git a/packages/playground/optimize-missing-deps/__test__/optimize-missing-deps.spec.ts b/packages/playground/optimize-missing-deps/__test__/optimize-missing-deps.spec.ts new file mode 100644 index 00000000000000..dd776daeceadbf --- /dev/null +++ b/packages/playground/optimize-missing-deps/__test__/optimize-missing-deps.spec.ts @@ -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') +}) diff --git a/packages/playground/optimize-missing-deps/__test__/serve.js b/packages/playground/optimize-missing-deps/__test__/serve.js new file mode 100644 index 00000000000000..9f293024f83913 --- /dev/null +++ b/packages/playground/optimize-missing-deps/__test__/serve.js @@ -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) + } + }) +} diff --git a/packages/playground/optimize-missing-deps/index.html b/packages/playground/optimize-missing-deps/index.html new file mode 100644 index 00000000000000..13e9831870aa18 --- /dev/null +++ b/packages/playground/optimize-missing-deps/index.html @@ -0,0 +1,11 @@ + + + + + + Vite App + + + + + diff --git a/packages/playground/optimize-missing-deps/main.js b/packages/playground/optimize-missing-deps/main.js new file mode 100644 index 00000000000000..93f3e1221298bf --- /dev/null +++ b/packages/playground/optimize-missing-deps/main.js @@ -0,0 +1,3 @@ +import { sayName } from 'missing-dep' + +export const name = sayName() diff --git a/packages/playground/optimize-missing-deps/missing-dep/index.js b/packages/playground/optimize-missing-deps/missing-dep/index.js new file mode 100644 index 00000000000000..f5d61c545d080a --- /dev/null +++ b/packages/playground/optimize-missing-deps/missing-dep/index.js @@ -0,0 +1,5 @@ +import { name } from 'multi-entry-dep' + +export function sayName() { + return name +} diff --git a/packages/playground/optimize-missing-deps/missing-dep/package.json b/packages/playground/optimize-missing-deps/missing-dep/package.json new file mode 100644 index 00000000000000..5d91f3518a5f5b --- /dev/null +++ b/packages/playground/optimize-missing-deps/missing-dep/package.json @@ -0,0 +1,8 @@ +{ + "name": "missing-dep", + "version": "0.0.0", + "main": "index.js", + "dependencies": { + "multi-entry-dep": "file:../multi-entry-dep" + } +} diff --git a/packages/playground/optimize-missing-deps/multi-entry-dep/index.browser.js b/packages/playground/optimize-missing-deps/multi-entry-dep/index.browser.js new file mode 100644 index 00000000000000..969e7564ad52e9 --- /dev/null +++ b/packages/playground/optimize-missing-deps/multi-entry-dep/index.browser.js @@ -0,0 +1 @@ +exports.name = 'Client' diff --git a/packages/playground/optimize-missing-deps/multi-entry-dep/index.js b/packages/playground/optimize-missing-deps/multi-entry-dep/index.js new file mode 100644 index 00000000000000..0717b87c27c2d8 --- /dev/null +++ b/packages/playground/optimize-missing-deps/multi-entry-dep/index.js @@ -0,0 +1,3 @@ +const path = require('path') + +exports.name = path.normalize('./Server') diff --git a/packages/playground/optimize-missing-deps/multi-entry-dep/package.json b/packages/playground/optimize-missing-deps/multi-entry-dep/package.json new file mode 100644 index 00000000000000..bf9ac7212de92c --- /dev/null +++ b/packages/playground/optimize-missing-deps/multi-entry-dep/package.json @@ -0,0 +1,8 @@ +{ + "name": "multi-entry-dep", + "version": "0.0.0", + "main": "index.js", + "browser": { + "./index.js": "./index.browser.js" + } +} diff --git a/packages/playground/optimize-missing-deps/package.json b/packages/playground/optimize-missing-deps/package.json new file mode 100644 index 00000000000000..b108c58bc41466 --- /dev/null +++ b/packages/playground/optimize-missing-deps/package.json @@ -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" + } +} diff --git a/packages/playground/optimize-missing-deps/server.js b/packages/playground/optimize-missing-deps/server.js new file mode 100644 index 00000000000000..b9422feb622584 --- /dev/null +++ b/packages/playground/optimize-missing-deps/server.js @@ -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 = `
${name}
+` + + const html = template.replace(``, 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 diff --git a/packages/vite/src/node/optimizer/esbuildDepPlugin.ts b/packages/vite/src/node/optimizer/esbuildDepPlugin.ts index 56ea6ac1311d53..9854beb0936d83 100644 --- a/packages/vite/src/node/optimizer/esbuildDepPlugin.ts +++ b/packages/vite/src/node/optimizer/esbuildDepPlugin.ts @@ -6,8 +6,7 @@ import { isRunningWithYarnPnp, flattenId, normalizePath, - isExternalUrl, - isBuiltin + isExternalUrl } from '../utils' import { browserExternalId } from '../plugins/resolve' import { ExportsData } from '.' @@ -121,7 +120,7 @@ export function esbuildDepPlugin( namespace: 'browser-external' } } - if (isExternalUrl(resolved) || (ssr && isBuiltin(id))) { + if (isExternalUrl(resolved)) { return { path: resolved, external: true diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index e0a99dd81d3b2b..5fd2c38ba99b50 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -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 diff --git a/yarn.lock b/yarn.lock index 2ad9822a24323f..503ab461bb5221 100644 --- a/yarn.lock +++ b/yarn.lock @@ -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" @@ -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"