fix(vite-node): fix source map of inlined node_modules #7557

hi-ogawa · 2025-02-25T03:25:30Z

Description

Currently ssr transform tries to preserve lines by best effort vitejs/vite#19004, but this is not guaranteed since user plugin can also change lines and also there are some not-so-edge cases ssr transform has't handle it yet (I added it as a test case).

Considering Vite SSR / module runner is also injecting source map (regardless of user code or inlined deps), I think we should allow this for Vitest / vite-node as well.

Also as an important context, preserving lines for transform is hard for the ecosystem for the following aspects:

transformer: vite ssrTransform oxc-project/oxc#9169
- with rolldown/oxc, we might do more transform on plain js not only ssr, but it becomes harder to preserve lines.
fix(ssr): hoist export to handle cyclic import better vitejs/vite#18983
- when try to preserve lines for ssr transform, it'll end up many code squashed into a single line, but that tends to break coverage tooling.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
Ideally, include a test that fails without this PR but passes with it.
Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Tests

Run the tests with pnpm test:ci.

Documentation

If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

netlify · 2025-02-25T05:55:43Z

✅ Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`d0f42b0`
🔍 Latest deploy log	https://app.netlify.com/sites/vitest-dev/deploys/67c28669b698970008e11732
😎 Deploy Preview	https://deploy-preview-7557--vitest-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

hi-ogawa · 2025-02-25T06:26:05Z

test/cli/deps/error/index.js

@@ -0,0 +1,11 @@
+import 'node:path'
+
+export function testStack() {


Currently ssr transform doesn't preserve lines with export, so this repro fails on main.

packages/vite-node/src/server.ts

hi-ogawa · 2025-02-25T07:52:57Z

New test is not running on CI due to #7558. We need to merge that first.

This reverts commit 8f360cf.

sheremet-va

What happens if file already has a sourcermapUrl that points to a sourcemap file. What ends up running?

hi-ogawa · 2025-03-01T05:26:58Z

What happens if file already has a sourcermapUrl that points to a sourcemap file. What ends up running?

Vite's transform pipeline starts with extractSourcemapFromFile and it will strip all sourceMappingURL, so Vitest won't see sourceMappingURL comment on our end. I'll add a test case to verify the correctness of collapsed sourcemap from node_modules.

hi-ogawa · 2025-03-01T06:25:54Z

test/cli/test/__snapshots__/stacktraces.test.ts.snap

+ FAIL  error-in-package.test.js > transpiled
+Error: __TEST_STACK_TRANSPILED__
+ ❯ innerTestStack (NODE_MODULES)/@vitest/test-dep-error/transpiled.ts:22:9
+ ❯ testStack (NODE_MODULES)/@vitest/test-dep-error/transpiled.ts:12:3
+ ❯ error-in-package.test.js:16:22
+     14| 
+     15| test('transpiled', () => {
+     16|   testStackTranspiled()
+       |                      ^
+     17| })
+     18| 
+
+⎯⎯[3/4]⎯
+
+ FAIL  error-in-package.test.js > transpiled inline
+Error: __TEST_STACK_TRANSPILED_INLINE__
+ ❯ innerTestStack (NODE_MODULES)/@vitest/test-dep-error/transpiled-inline.ts:22:9
+ ❯ testStack (NODE_MODULES)/@vitest/test-dep-error/transpiled-inline.ts:12:3
+ ❯ error-in-package.test.js:20:28
+     18| 
+     19| test('transpiled inline', () => {
+     20|   testStackTranspiledInline()
+       |                            ^
+     21| })
+     22| 
+
+⎯⎯[4/4]⎯


Error frames don't show the original ts file content because it's not found by getModuleById.

vitest/packages/vitest/src/node/error.ts

Lines 127 to 135 in bbfcf9f

const nearest

= error instanceof TypeCheckError

? error.stacks[0]

: stacks.find((stack) => {

try {

return (

project.server

&& project.getModuleById(stack.file)

&& existsSync(stack.file)

After removing getModuleById check, frame gets shown properly like below, but this probably doesn't matter since published packages wouldn't ship .ts file, so existsSync would fail anyways for actual case.

FAIL stacktraces/error-in-package.test.js > transpiled Error: __TEST_STACK_TRANSPILED__ ❯ innerTestStack ../../../node_modules/.pnpm/@vitest+test-dep-error@file+test+cli+deps+error/node_modules/@vitest/test-dep-error/transpiled.ts:22:9 20| */ 21| function innerTestStack() { 22| throw new Error('__TEST_STACK_TRANSPILED__') | ^ 23| } 24| ❯ testStack ../../../node_modules/.pnpm/@vitest+test-dep-error@file+test+cli+deps+error/node_modules/@vitest/test-dep-error/transpiled.ts:12:3 ❯ stacktraces/error-in-package.test.js:16:22

sheremet-va

Thank you! This looks good to me 👍🏻

hi-ogawa added 2 commits February 25, 2025 12:17

fix(vite-node): fix source map of inlined node_modules

34b87c0

test: add test

8f7b281

test: cleanup

f1264fb

hi-ogawa commented Feb 25, 2025

View reviewed changes

AriPerkkio reviewed Feb 25, 2025

View reviewed changes

packages/vite-node/src/server.ts Show resolved Hide resolved

test: use snapshot

bf9ec27

hi-ogawa added 5 commits February 25, 2025 17:12

Merge branch 'main' into fix-inline-node-modules-source-map

683b063

test: cleanup

8f360cf

Revert "test: cleanup"

edca9bb

This reverts commit 8f360cf.

test: update snapshot

2bd5534

Merge branch 'main' into fix-inline-node-modules-source-map

6159c8c

hi-ogawa marked this pull request as ready for review February 26, 2025 04:34

hi-ogawa added the p2-to-be-discussed Enhancement under consideration (priority) label Feb 26, 2025

sheremet-va reviewed Feb 28, 2025

View reviewed changes

sheremet-va added p3-minor-bug An edge case that only affects very specific usage (priority) and removed p2-to-be-discussed Enhancement under consideration (priority) labels Feb 28, 2025

hi-ogawa added 2 commits March 1, 2025 12:54

Merge branch 'main' into fix-inline-node-modules-source-map

c406c72

Merge branch 'main' into fix-inline-node-modules-source-map

d0f42b0

test: add more test

bbfcf9f

hi-ogawa commented Mar 1, 2025

View reviewed changes

hi-ogawa requested a review from sheremet-va March 1, 2025 06:30

Merge branch 'main' into fix-inline-node-modules-source-map

4a53951

This was referenced Mar 7, 2025

fix(coverage): exclude injected functions without mapping #7192

Closed

chore: test Vite PR 18983 #7621

Closed

fix(ssr): hoist export to handle cyclic import better vitejs/vite#18983

Merged

sheremet-va approved these changes Mar 7, 2025

View reviewed changes

sheremet-va merged commit 34aa322 into vitest-dev:main Mar 7, 2025
9 of 13 checks passed

hi-ogawa deleted the fix-inline-node-modules-source-map branch March 7, 2025 08:52

tvanier mentioned this pull request Mar 17, 2025

chore(deps-dev): bump vitest from 3.0.8 to 3.0.9 tvanier/sport-visioner#584

Merged

renovate bot mentioned this pull request Mar 27, 2025

Update all non-major dependencies fluent-vue/unplugin-fluent-vue#123

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(vite-node): fix source map of inlined node_modules #7557

fix(vite-node): fix source map of inlined node_modules #7557

hi-ogawa commented Feb 25, 2025 •

edited

Loading

netlify bot commented Feb 25, 2025 •

edited

Loading

hi-ogawa Feb 25, 2025

hi-ogawa commented Feb 25, 2025

sheremet-va left a comment

hi-ogawa commented Mar 1, 2025

hi-ogawa Mar 1, 2025 •

edited

Loading

sheremet-va left a comment

		@@ -0,0 +1,11 @@
		import 'node:path'

		export function testStack() {

	const nearest
	= error instanceof TypeCheckError
	? error.stacks[0]
	: stacks.find((stack) => {
	try {
	return (
	project.server
	&& project.getModuleById(stack.file)
	&& existsSync(stack.file)

fix(vite-node): fix source map of inlined node_modules #7557

fix(vite-node): fix source map of inlined node_modules #7557

Conversation

hi-ogawa commented Feb 25, 2025 • edited Loading

Description

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

Tests

Documentation

Changesets

netlify bot commented Feb 25, 2025 • edited Loading

✅ Deploy Preview for vitest-dev ready!

hi-ogawa Feb 25, 2025

Choose a reason for hiding this comment

hi-ogawa commented Feb 25, 2025

sheremet-va left a comment

Choose a reason for hiding this comment

hi-ogawa commented Mar 1, 2025

hi-ogawa Mar 1, 2025 • edited Loading

Choose a reason for hiding this comment

sheremet-va left a comment

Choose a reason for hiding this comment

hi-ogawa commented Feb 25, 2025 •

edited

Loading

netlify bot commented Feb 25, 2025 •

edited

Loading

hi-ogawa Mar 1, 2025 •

edited

Loading