fix(pluginContainer): should not exclude client sourcemap source content

[sun0day]

Description

fix #12916

Since neither client.ts nor env.ts exist in the vite package, their sourcemaps' source content shouldn't be excluded when combining sourcemaps

Additional context

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the PR Title Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[sapphi-red]

Ideally I think we should pass the sourcesContent all along the way and inject null sourcesContent in the end before passing to browsers. I believe that will fix many "missing source files" warning issues.

I don't know why excludeContent was true in the first place (103dec9#diff-071a32aedd2ea59472ebb69fb456e818b103d1a332da632e12c2d54395938ad1).

vite/packages/vite/src/node/utils.ts

Line 735 in a8c7eb2

excludeContent = true,

I seems we are not handling cases like sourcesContent: [null, 'content'] and sourcesContent: ['content'], sources: ['non-existent path']. The former will have null sourcesContent that should be transformed to 'content' by reading from the file, and the latter will output a warning that should not be output because the content already exists.

vite/packages/vite/src/node/server/transformRequest.ts

Lines 272 to 274 in a8c7eb2

	if (map.mappings && !map.sourcesContent) {
	await injectSourcesContent(map, mod.file, logger)
	}

[sun0day]

Would it be possible to replace the non-exist path with null in sources and just keep its 'content' in sourcesContent via injectSourcesContent?

If so, it would be a lot easier to judge whether it's fine to combineSourcemaps with excludeContent=true

[sapphi-red]

Would it be possible to replace the non-exist path with null in sources and just keep its 'content' in sourcesContent via injectSourcesContent?

We should not replace non-existent path with null. The spec says sourcesContent field can contain null but not for sources field. Also browsers/debuggers can show the original path if sources included the non-exist path.

---

The former will have null sourcesContent that should be transformed to 'content' by reading from the file, and the latter will output a warning that should not be output because the content already exists.

Let me elaborate this sentence just in case. I mean

• { "sourcesContent": [null], "sources": ["existent-path"] } should be transformed to { "sourcesContent": ["content from file"], "sources": ["existent-path"] } (currently Vite does not replace null with the actual content)
• { "sourcesContent": ['content'], "sources": ["non-existent-path"] } should not output a warning (currently Vite outputs a warning)

at these lines.

vite/packages/vite/src/node/server/transformRequest.ts

Lines 272 to 274 in a8c7eb2

	if (map.mappings && !map.sourcesContent) {
	await injectSourcesContent(map, mod.file, logger)
	}

[sun0day]

Yeah, I understand what you mean, my question is if we don't replace non-exist-path with null then how to judge whether excludeContent or not in combineSourcemaps

vite/packages/vite/src/node/server/pluginContainer.ts

Lines 543 to 549 in 23096b1

	combinedMap = combineSourcemaps(cleanUrl(this.filename), [
	{
	...(m as RawSourceMap),
	sourcesContent: combinedMap.sourcesContent,
	},
	combinedMap as RawSourceMap,
	]) as SourceMap

[sapphi-red]

excludeContent needs to be false unconditionally to pass sourcesContent all along the way.