From cba08056fa2446713e3d01607e69b2956b1eff4b Mon Sep 17 00:00:00 2001 From: Justice Almanzar Date: Mon, 16 Jan 2023 00:30:31 -0500 Subject: [PATCH 1/8] fix(ssr): load sourcemaps alongside modules (fix: #3288) (#11576) --- packages/vite/src/node/ssr/ssrModuleLoader.ts | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 4cb226de83944e..2b09bc050603ff 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -10,6 +10,7 @@ import { import { transformRequest } from '../server/transformRequest' import type { InternalResolveOptionsWithOverrideConditions } from '../plugins/resolve' import { tryNodeResolve } from '../plugins/resolve' +import { genSourceMapUrl } from '../server/sourcemap' import { ssrDynamicImportKey, ssrExportAllKey, @@ -25,6 +26,16 @@ interface SSRContext { type SSRModule = Record +// eslint-disable-next-line @typescript-eslint/no-empty-function +const AsyncFunction = async function () {}.constructor as typeof Function +let fnDeclarationLineCount = 0 +{ + const body = '/*code*/' + const source = new AsyncFunction('a', 'b', body).toString() + fnDeclarationLineCount = + source.slice(0, source.indexOf(body)).split('\n').length - 1 +} + const pendingModules = new Map>() const pendingImports = new Map() @@ -181,9 +192,17 @@ async function instantiateModule( } } + let sourceMapSuffix = '' + if (result.map) { + const moduleSourceMap = Object.assign({}, result.map, { + // offset the first three lines of the module (function declaration and 'use strict') + mappings: ';'.repeat(fnDeclarationLineCount + 1) + result.map.mappings, + }) + sourceMapSuffix = + '\n//# sourceMappingURL=' + genSourceMapUrl(moduleSourceMap) + } + try { - // eslint-disable-next-line @typescript-eslint/no-empty-function - const AsyncFunction = async function () {}.constructor as typeof Function const initModule = new AsyncFunction( `global`, ssrModuleExportsKey, @@ -191,7 +210,9 @@ async function instantiateModule( ssrImportKey, ssrDynamicImportKey, ssrExportAllKey, - '"use strict";' + result.code + `\n//# sourceURL=${mod.url}`, + '"use strict";\n' + + result.code + + `\n//# sourceURL=${mod.url}${sourceMapSuffix}`, ) await initModule( context.global, From 79a24f15791f32ced913954021b7a5556a2fccf4 Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Sun, 22 Jan 2023 03:06:59 +0900 Subject: [PATCH 2/8] test(ssr): add stacktrace test --- .../ssr-html/__tests__/ssr-html.spec.ts | 27 +++++++++++++++++ playground/ssr-html/package.json | 4 ++- playground/ssr-html/src/error.js | 3 ++ playground/ssr-html/test-stacktrace.js | 29 +++++++++++++++++++ 4 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 playground/ssr-html/src/error.js create mode 100644 playground/ssr-html/test-stacktrace.js diff --git a/playground/ssr-html/__tests__/ssr-html.spec.ts b/playground/ssr-html/__tests__/ssr-html.spec.ts index fa871fce04efb8..2b9da5cadd01c0 100644 --- a/playground/ssr-html/__tests__/ssr-html.spec.ts +++ b/playground/ssr-html/__tests__/ssr-html.spec.ts @@ -1,3 +1,6 @@ +import { execFile } from 'node:child_process' +import { promisify } from 'node:util' +import path from 'node:path' import fetch from 'node-fetch' import { describe, expect, test } from 'vitest' import { port } from './serve' @@ -55,3 +58,27 @@ describe.runIf(isServe)('hmr', () => { }, '[wow]') }) }) + +describe.runIf(isServe)('stacktrace', () => { + const execFileAsync = promisify(execFile) + + for (const sourcemapsEnabled of [false, true]) { + test(`stacktrace is correct when sourcemaps is${ + sourcemapsEnabled ? '' : ' not' + } enabled in Node.js`, async () => { + const testStacktraceFile = path.resolve( + __dirname, + '../test-stacktrace.js', + ) + + const p = await execFileAsync('node', [ + testStacktraceFile, + '' + sourcemapsEnabled, + ]) + const line = p.stdout + .split('\n') + .find((line) => line.includes('Module.error')) + expect(line.trim()).toMatch(/[\\/]src[\\/]error\.js:2:9/) + }) + } +}) diff --git a/playground/ssr-html/package.json b/playground/ssr-html/package.json index 090faa1b578684..f1fb19aaa25488 100644 --- a/playground/ssr-html/package.json +++ b/playground/ssr-html/package.json @@ -6,7 +6,9 @@ "scripts": { "dev": "node server", "serve": "NODE_ENV=production node server", - "debug": "node --inspect-brk server" + "debug": "node --inspect-brk server", + "test-stacktrace:off": "node test-stacktrace false", + "test-stacktrace:on": "node test-stacktrace true" }, "dependencies": {}, "devDependencies": { diff --git a/playground/ssr-html/src/error.js b/playground/ssr-html/src/error.js new file mode 100644 index 00000000000000..fe8eeb20af8f8a --- /dev/null +++ b/playground/ssr-html/src/error.js @@ -0,0 +1,3 @@ +export function error() { + throw new Error('e') +} diff --git a/playground/ssr-html/test-stacktrace.js b/playground/ssr-html/test-stacktrace.js new file mode 100644 index 00000000000000..023c810a9516ab --- /dev/null +++ b/playground/ssr-html/test-stacktrace.js @@ -0,0 +1,29 @@ +import path from 'node:path' +import { fileURLToPath } from 'node:url' +import { createServer } from 'vite' + +const isSourceMapEnabled = process.argv[2] === 'true' +process.setSourceMapsEnabled(isSourceMapEnabled) +console.log('# sourcemaps enabled:', isSourceMapEnabled) + +const __dirname = path.dirname(fileURLToPath(import.meta.url)) +const isTest = process.env.VITEST + +const vite = await createServer({ + root: __dirname, + logLevel: isTest ? 'error' : 'info', + server: { + middlewareMode: true, + }, + appType: 'custom', +}) + +const mod = await vite.ssrLoadModule('/src/error.js') +try { + mod.error() +} catch (e) { + vite.ssrFixStacktrace(e) + console.log(e) +} + +await vite.close() From 788bdcce67fabe5af52203e465a542ffef5befa0 Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Sun, 22 Jan 2023 03:25:56 +0900 Subject: [PATCH 3/8] fix: sourcemap misalignment --- packages/vite/src/node/ssr/ssrModuleLoader.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 2b09bc050603ff..0abae19a6e4880 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -195,8 +195,8 @@ async function instantiateModule( let sourceMapSuffix = '' if (result.map) { const moduleSourceMap = Object.assign({}, result.map, { - // offset the first three lines of the module (function declaration and 'use strict') - mappings: ';'.repeat(fnDeclarationLineCount + 1) + result.map.mappings, + // offset the first three lines of the module (function declaration) + mappings: ';'.repeat(fnDeclarationLineCount) + result.map.mappings, }) sourceMapSuffix = '\n//# sourceMappingURL=' + genSourceMapUrl(moduleSourceMap) @@ -210,7 +210,7 @@ async function instantiateModule( ssrImportKey, ssrDynamicImportKey, ssrExportAllKey, - '"use strict";\n' + + '"use strict";' + result.code + `\n//# sourceURL=${mod.url}${sourceMapSuffix}`, ) From c8b9d7f0c6a2fb5819ad460d459b9a164bb889c5 Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Sun, 22 Jan 2023 03:36:01 +0900 Subject: [PATCH 4/8] chore: add some comments --- packages/vite/src/node/ssr/ssrModuleLoader.ts | 3 ++- playground/ssr-html/test-stacktrace.js | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 0abae19a6e4880..03be7c355debd2 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -195,7 +195,8 @@ async function instantiateModule( let sourceMapSuffix = '' if (result.map) { const moduleSourceMap = Object.assign({}, result.map, { - // offset the first three lines of the module (function declaration) + // currently we need to offset the line + // https://github.com/nodejs/node/issues/43047#issuecomment-1180632750 mappings: ';'.repeat(fnDeclarationLineCount) + result.map.mappings, }) sourceMapSuffix = diff --git a/playground/ssr-html/test-stacktrace.js b/playground/ssr-html/test-stacktrace.js index 023c810a9516ab..01fbcbbe5c418c 100644 --- a/playground/ssr-html/test-stacktrace.js +++ b/playground/ssr-html/test-stacktrace.js @@ -22,6 +22,8 @@ const mod = await vite.ssrLoadModule('/src/error.js') try { mod.error() } catch (e) { + // this is not required + // when running on Node.js "^16.17.0 || >=18.6.0" and sourcemap is enabled vite.ssrFixStacktrace(e) console.log(e) } From 66b8d70709f0e24bca68e43d928a35b692f67e89 Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Sun, 22 Jan 2023 13:37:10 +0900 Subject: [PATCH 5/8] test: should not call fixStacktrace when sourcemap is enabled --- playground/ssr-html/test-stacktrace.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/playground/ssr-html/test-stacktrace.js b/playground/ssr-html/test-stacktrace.js index 01fbcbbe5c418c..3d18d59db02a49 100644 --- a/playground/ssr-html/test-stacktrace.js +++ b/playground/ssr-html/test-stacktrace.js @@ -22,9 +22,12 @@ const mod = await vite.ssrLoadModule('/src/error.js') try { mod.error() } catch (e) { - // this is not required + // this should not be called // when running on Node.js "^16.17.0 || >=18.6.0" and sourcemap is enabled - vite.ssrFixStacktrace(e) + // because the stacktrace is already rewritten + if (isSourceMapEnabled) { + vite.ssrFixStacktrace(e) + } console.log(e) } From 0e13305509696983f7ba280957c67669d9e07987 Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Sun, 22 Jan 2023 13:43:17 +0900 Subject: [PATCH 6/8] test: oops --- playground/ssr-html/test-stacktrace.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/playground/ssr-html/test-stacktrace.js b/playground/ssr-html/test-stacktrace.js index 3d18d59db02a49..234d52e2ef4734 100644 --- a/playground/ssr-html/test-stacktrace.js +++ b/playground/ssr-html/test-stacktrace.js @@ -25,7 +25,7 @@ try { // this should not be called // when running on Node.js "^16.17.0 || >=18.6.0" and sourcemap is enabled // because the stacktrace is already rewritten - if (isSourceMapEnabled) { + if (!isSourceMapEnabled) { vite.ssrFixStacktrace(e) } console.log(e) From 3e20e0cd3e5e0609b45782268b9b9f152408b636 Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Sun, 22 Jan 2023 14:02:01 +0900 Subject: [PATCH 7/8] test: fix Node 14 --- playground/ssr-html/test-stacktrace.js | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/playground/ssr-html/test-stacktrace.js b/playground/ssr-html/test-stacktrace.js index 234d52e2ef4734..d0063858d7c9d1 100644 --- a/playground/ssr-html/test-stacktrace.js +++ b/playground/ssr-html/test-stacktrace.js @@ -6,6 +6,19 @@ const isSourceMapEnabled = process.argv[2] === 'true' process.setSourceMapsEnabled(isSourceMapEnabled) console.log('# sourcemaps enabled:', isSourceMapEnabled) +const version = (() => { + const m = process.version.match(/^v(\d+)\.(\d+)\.\d+$/) + if (!m) throw new Error(`Failed to parse version: ${process.version}`) + + return { major: m[1], minor: m[2] } +})() + +// https://github.com/nodejs/node/pull/43428 +const isFunctionSourceMapSupported = + (version.major === 16 && version.minor >= 17) || + (version.major === 18 && version.minor >= 6) || + version.major >= 19 + const __dirname = path.dirname(fileURLToPath(import.meta.url)) const isTest = process.env.VITEST @@ -23,9 +36,9 @@ try { mod.error() } catch (e) { // this should not be called - // when running on Node.js "^16.17.0 || >=18.6.0" and sourcemap is enabled - // because the stacktrace is already rewritten - if (!isSourceMapEnabled) { + // when sourcemap support for `new Function` is supported and sourcemap is enabled + // because the stacktrace is already rewritten by Node.js + if (!(isSourceMapEnabled && isFunctionSourceMapSupported)) { vite.ssrFixStacktrace(e) } console.log(e) From c28bacb0bc9c8af81b855aa6063670c4854c4b8d Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Sun, 22 Jan 2023 14:11:00 +0900 Subject: [PATCH 8/8] test: fix --- playground/ssr-html/test-stacktrace.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/playground/ssr-html/test-stacktrace.js b/playground/ssr-html/test-stacktrace.js index d0063858d7c9d1..c3ce5e56736799 100644 --- a/playground/ssr-html/test-stacktrace.js +++ b/playground/ssr-html/test-stacktrace.js @@ -10,7 +10,7 @@ const version = (() => { const m = process.version.match(/^v(\d+)\.(\d+)\.\d+$/) if (!m) throw new Error(`Failed to parse version: ${process.version}`) - return { major: m[1], minor: m[2] } + return { major: +m[1], minor: +m[2] } })() // https://github.com/nodejs/node/pull/43428