From bfc70e232a30dca5294e0d31426a47175d457fc3 Mon Sep 17 00:00:00 2001 From: bluwy Date: Fri, 11 Nov 2022 21:54:40 +0800 Subject: [PATCH 1/3] fix(ssr): improve missing file error --- .../vite/src/node/plugins/importAnalysis.ts | 2 +- .../vite/src/node/server/transformRequest.ts | 17 +++++--------- .../fixtures/modules/has-invalid-import.js | 4 ++++ .../node/ssr/__tests__/ssrLoadModule.spec.ts | 23 +++++++++++++++++++ 4 files changed, 34 insertions(+), 12 deletions(-) create mode 100644 packages/vite/src/node/ssr/__tests__/fixtures/modules/has-invalid-import.js create mode 100644 packages/vite/src/node/ssr/__tests__/ssrLoadModule.spec.ts diff --git a/packages/vite/src/node/plugins/importAnalysis.ts b/packages/vite/src/node/plugins/importAnalysis.ts index d26b0e219085d9..8950ce968bca28 100644 --- a/packages/vite/src/node/plugins/importAnalysis.ts +++ b/packages/vite/src/node/plugins/importAnalysis.ts @@ -9,7 +9,6 @@ import { parse as parseJS } from 'acorn' import type { Node } from 'estree' import { findStaticImports, parseStaticImport } from 'mlly' import { makeLegalIdentifier } from '@rollup/pluginutils' -import { getDepOptimizationConfig } from '..' import type { ViteDevServer } from '..' import { CLIENT_DIR, @@ -44,6 +43,7 @@ import { unwrapId, wrapId } from '../utils' +import { getDepOptimizationConfig } from '../config' import type { ResolvedConfig } from '../config' import type { Plugin } from '../plugin' import { diff --git a/packages/vite/src/node/server/transformRequest.ts b/packages/vite/src/node/server/transformRequest.ts index f2b78516e63670..3f4d27ad9661a2 100644 --- a/packages/vite/src/node/server/transformRequest.ts +++ b/packages/vite/src/node/server/transformRequest.ts @@ -215,18 +215,13 @@ async function loadAndTransform( } } if (code == null) { - if (checkPublicFile(url, config)) { - throw new Error( - `Failed to load url ${url} (resolved id: ${id}). ` + - `This file is in /public and will be copied as-is during build without ` + - `going through the plugin transforms, and therefore should not be ` + - `imported from source code. It can only be referenced via HTML tags.` - ) - } else { - return null - } + const msg = checkPublicFile(url, config) + ? `This file is in /public and will be copied as-is during build without ` + + `going through the plugin transforms, and therefore should not be ` + + `imported from source code. It can only be referenced via HTML tags.` + : `Does the file exist?` + throw new Error(`Failed to load url ${url} (resolved id: ${id}). ${msg}`) } - // ensure module in graph after successful load const mod = await moduleGraph.ensureEntryFromUrl(url, ssr) ensureWatchedFile(watcher, mod.file, root) diff --git a/packages/vite/src/node/ssr/__tests__/fixtures/modules/has-invalid-import.js b/packages/vite/src/node/ssr/__tests__/fixtures/modules/has-invalid-import.js new file mode 100644 index 00000000000000..81ab2b0fab83c9 --- /dev/null +++ b/packages/vite/src/node/ssr/__tests__/fixtures/modules/has-invalid-import.js @@ -0,0 +1,4 @@ +// eslint-disable-next-line node/no-missing-import +import { foo } from './non-existent.js' + +export const hello = 'world' diff --git a/packages/vite/src/node/ssr/__tests__/ssrLoadModule.spec.ts b/packages/vite/src/node/ssr/__tests__/ssrLoadModule.spec.ts new file mode 100644 index 00000000000000..1df5f2b128da5d --- /dev/null +++ b/packages/vite/src/node/ssr/__tests__/ssrLoadModule.spec.ts @@ -0,0 +1,23 @@ +import { fileURLToPath } from 'node:url' +import { expect, test } from 'vitest' +import { createServer } from '../../server' + +const root = fileURLToPath(new URL('./', import.meta.url)) + +async function createDevServer() { + const server = await createServer({ configFile: false, root }) + server.pluginContainer.buildStart({}) + return server +} + +test('ssrLoad', async () => { + expect.assertions(1) + const server = await createDevServer() + try { + await server.ssrLoadModule('/fixtures/modules/has-invalid-import.js') + } catch (e) { + expect(e.message).toBe( + 'Failed to load url ./non-existent.js (resolved id: ./non-existent.js). Does the file exist?' + ) + } +}) From 4213003ea9df6600e2a399838a367b2964ba1f00 Mon Sep 17 00:00:00 2001 From: bluwy Date: Fri, 11 Nov 2022 22:45:18 +0800 Subject: [PATCH 2/3] feat: add strict option --- packages/vite/src/node/plugins/importAnalysis.ts | 4 ++-- packages/vite/src/node/server/transformRequest.ts | 12 +++++++++++- packages/vite/src/node/ssr/ssrModuleLoader.ts | 2 +- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/vite/src/node/plugins/importAnalysis.ts b/packages/vite/src/node/plugins/importAnalysis.ts index 8950ce968bca28..8f7ae8c9d41699 100644 --- a/packages/vite/src/node/plugins/importAnalysis.ts +++ b/packages/vite/src/node/plugins/importAnalysis.ts @@ -712,9 +712,9 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin { // These requests will also be registered in transformRequest to be awaited // by the deps optimizer if (config.server.preTransformRequests && staticImportedUrls.size) { - staticImportedUrls.forEach(({ url, id }) => { + staticImportedUrls.forEach(({ url }) => { url = removeImportQuery(url) - transformRequest(url, server, { ssr }).catch((e) => { + transformRequest(url, server, { ssr, strict: true }).catch((e) => { if (e?.code === ERR_OUTDATED_OPTIMIZED_DEP) { // This are expected errors return diff --git a/packages/vite/src/node/server/transformRequest.ts b/packages/vite/src/node/server/transformRequest.ts index 3f4d27ad9661a2..0c0c19b5a5a307 100644 --- a/packages/vite/src/node/server/transformRequest.ts +++ b/packages/vite/src/node/server/transformRequest.ts @@ -37,6 +37,11 @@ export interface TransformResult { export interface TransformOptions { ssr?: boolean html?: boolean + /** + * For non-fatal errors, set true to throw an error, otherwise set false to + * return null instead + */ + strict?: boolean } export function transformRequest( @@ -161,6 +166,7 @@ async function loadAndTransform( const { root, logger } = config const prettyUrl = isDebug ? prettifyUrl(url, config.root) : '' const ssr = !!options.ssr + const strict = !!options.strict const file = cleanUrl(id) @@ -215,7 +221,11 @@ async function loadAndTransform( } } if (code == null) { - const msg = checkPublicFile(url, config) + const isPublicFile = checkPublicFile(url, config) + if (!isPublicFile && !strict) { + return null + } + const msg = isPublicFile ? `This file is in /public and will be copied as-is during build without ` + `going through the plugin transforms, and therefore should not be ` + `imported from source code. It can only be referenced via HTML tags.` diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 0e4e163570a575..bea362ff7833b5 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -85,7 +85,7 @@ async function instantiateModule( } const result = mod.ssrTransformResult || - (await transformRequest(url, server, { ssr: true })) + (await transformRequest(url, server, { ssr: true, strict: true })) if (!result) { // TODO more info? is this even necessary? throw new Error(`failed to load module for ssr: ${url}`) From 32ce26513c988da39bfa8294dd6d12986b00f61d Mon Sep 17 00:00:00 2001 From: bluwy Date: Fri, 11 Nov 2022 23:14:12 +0800 Subject: [PATCH 3/3] fix: try error handling --- .../vite/src/node/plugins/importAnalysis.ts | 2 +- .../src/node/server/middlewares/transform.ts | 6 +++++- .../vite/src/node/server/transformRequest.ts | 18 ++++++++---------- packages/vite/src/node/ssr/ssrModuleLoader.ts | 2 +- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/packages/vite/src/node/plugins/importAnalysis.ts b/packages/vite/src/node/plugins/importAnalysis.ts index 8f7ae8c9d41699..80ee54e501a26e 100644 --- a/packages/vite/src/node/plugins/importAnalysis.ts +++ b/packages/vite/src/node/plugins/importAnalysis.ts @@ -714,7 +714,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin { if (config.server.preTransformRequests && staticImportedUrls.size) { staticImportedUrls.forEach(({ url }) => { url = removeImportQuery(url) - transformRequest(url, server, { ssr, strict: true }).catch((e) => { + transformRequest(url, server, { ssr }).catch((e) => { if (e?.code === ERR_OUTDATED_OPTIMIZED_DEP) { // This are expected errors return diff --git a/packages/vite/src/node/server/middlewares/transform.ts b/packages/vite/src/node/server/middlewares/transform.ts index 929c6856d34748..831ea2c2139a2b 100644 --- a/packages/vite/src/node/server/middlewares/transform.ts +++ b/packages/vite/src/node/server/middlewares/transform.ts @@ -18,7 +18,7 @@ import { unwrapId } from '../../utils' import { send } from '../send' -import { transformRequest } from '../transformRequest' +import { ERR_LOAD_URL, transformRequest } from '../transformRequest' import { isHTMLProxy } from '../../plugins/html' import { DEP_VERSION_RE, @@ -224,6 +224,10 @@ export function transformMiddleware( // error but a normal part of the missing deps discovery flow return } + if (e?.code === ERR_LOAD_URL) { + // Let other middleware handle if we can't load the url via transformRequest + return next() + } return next(e) } diff --git a/packages/vite/src/node/server/transformRequest.ts b/packages/vite/src/node/server/transformRequest.ts index 0c0c19b5a5a307..b7bed216348504 100644 --- a/packages/vite/src/node/server/transformRequest.ts +++ b/packages/vite/src/node/server/transformRequest.ts @@ -21,6 +21,9 @@ import { getDepsOptimizer } from '../optimizer' import { injectSourcesContent } from './sourcemap' import { isFileServingAllowed } from './middlewares/static' +export const ERR_LOAD_URL = 'ERR_LOAD_URL' +export const ERR_LOAD_PUBLIC_URL = 'ERR_LOAD_PUBLIC_URL' + const debugLoad = createDebugger('vite:load') const debugTransform = createDebugger('vite:transform') const debugCache = createDebugger('vite:cache') @@ -37,11 +40,6 @@ export interface TransformResult { export interface TransformOptions { ssr?: boolean html?: boolean - /** - * For non-fatal errors, set true to throw an error, otherwise set false to - * return null instead - */ - strict?: boolean } export function transformRequest( @@ -166,7 +164,6 @@ async function loadAndTransform( const { root, logger } = config const prettyUrl = isDebug ? prettifyUrl(url, config.root) : '' const ssr = !!options.ssr - const strict = !!options.strict const file = cleanUrl(id) @@ -222,15 +219,16 @@ async function loadAndTransform( } if (code == null) { const isPublicFile = checkPublicFile(url, config) - if (!isPublicFile && !strict) { - return null - } const msg = isPublicFile ? `This file is in /public and will be copied as-is during build without ` + `going through the plugin transforms, and therefore should not be ` + `imported from source code. It can only be referenced via HTML tags.` : `Does the file exist?` - throw new Error(`Failed to load url ${url} (resolved id: ${id}). ${msg}`) + const err: any = new Error( + `Failed to load url ${url} (resolved id: ${id}). ${msg}` + ) + err.code = isPublicFile ? ERR_LOAD_PUBLIC_URL : ERR_LOAD_URL + throw err } // ensure module in graph after successful load const mod = await moduleGraph.ensureEntryFromUrl(url, ssr) diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index bea362ff7833b5..0e4e163570a575 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -85,7 +85,7 @@ async function instantiateModule( } const result = mod.ssrTransformResult || - (await transformRequest(url, server, { ssr: true, strict: true })) + (await transformRequest(url, server, { ssr: true })) if (!result) { // TODO more info? is this even necessary? throw new Error(`failed to load module for ssr: ${url}`)