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(ssr): load sourcemaps alongside modules #11780

Merged
merged 8 commits into from Apr 4, 2023
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
28 changes: 25 additions & 3 deletions packages/vite/src/node/ssr/ssrModuleLoader.ts
Expand Up @@ -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,
Expand All @@ -25,6 +26,16 @@ interface SSRContext {

type SSRModule = Record<string, any>

// 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<string, Promise<SSRModule>>()
const pendingImports = new Map<string, string[]>()

Expand Down Expand Up @@ -181,17 +192,28 @@ async function instantiateModule(
}
}

let sourceMapSuffix = ''
if (result.map) {
const moduleSourceMap = Object.assign({}, result.map, {
// currently we need to offset the line
// https://github.com/nodejs/node/issues/43047#issuecomment-1180632750
mappings: ';'.repeat(fnDeclarationLineCount) + 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,
ssrImportMetaKey,
ssrImportKey,
ssrDynamicImportKey,
ssrExportAllKey,
'"use strict";' + result.code + `\n//# sourceURL=${mod.url}`,
'"use strict";' +
result.code +
`\n//# sourceURL=${mod.url}${sourceMapSuffix}`,
)
await initModule(
context.global,
Expand Down
27 changes: 27 additions & 0 deletions 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'
Expand Down Expand Up @@ -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/)
})
}
})
4 changes: 3 additions & 1 deletion playground/ssr-html/package.json
Expand Up @@ -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": {
Expand Down
3 changes: 3 additions & 0 deletions playground/ssr-html/src/error.js
@@ -0,0 +1,3 @@
export function error() {
throw new Error('e')
}
47 changes: 47 additions & 0 deletions playground/ssr-html/test-stacktrace.js
@@ -0,0 +1,47 @@
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)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work if something called process.setSourceMapsEnabled after this line. (I mean the isSourceMapEnabled will not be synced with the actual value.)

I've created a feature request to Node to fix this: nodejs/node#46304


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

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) {
// this should not be called
// 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)
}

await vite.close()