fix: __VITE_PUBLIC_ASSET__hash__ in HTML

[nvh95]

Description

• In Vite 3, in index.html, if we refer to a file in publicDir inside url() (e.g: style="background: url(/icon.png)"), it becomes __VITE_PUBLIC_ASSET__hash__ after building (e.g: style="background: url(__VITE_PUBLIC_ASSET__018698dc__)").
• For more details, see [v3.x] - public/fonts/font.woff in index.html contains __VITE_PUBLIC_ASSET__ #9174
• This PR updates the playground/assets to reproduce this bug
• This PR is trying to replace __VITE_PUBLIC_ASSET__hash__ with the actual file in publicDir. (packages/vite/src/node/plugins/html.ts)
• This PR add tests in playground/assets/__tests__/assets.spec.ts to cover changes. (not passed all yet)

Related PR

• feat!: relative base #7644

Additional context

• We are migrating to Vite 3 for bestofjs.org then encounter an issue that fonts inside publicDir does not load Upgrade to vite 3 bestofjs/bestofjs#148 (comment)

You can see the preview at https://bestofjs-68pbasy3a-bestofjs.vercel.app/
There is no file such as https://bestofjs-68pbasy3a-bestofjs.vercel.app/__VITE_PUBLIC_ASSET__03d6a1cf__

Questions

• There is one test not passed yet. However, I tried to build it then it work fine. Not sure what makes this inconsistent yet.

https://github.com/vitejs/vite/pull/9247/files#diff-be0c593011deabf61343720611c7586df51ddc269f187678cdbf85ff72a7973bR370-R373

---

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 Commit 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.
  • but pass with it.
• Discovered another bug in DEV mode fix: __VITE_PUBLIC_ASSET__hash__ in HTML #9247 (comment), @sapphi-red to refer issue number here

[nvh95]

I add a new test to assert that if my source code looks like this

<p class="inline-style-public" style="background: url(/icon.png)">
  inline style
</p>

It should become as bellow after building (since the public dir is /static)

<p class="inline-style-public" style="background: url(/foo/icon.png)">
  inline style
</p>

But it hasn't passed yet and I'm not sure what is the problem. I guess it's from how we setup Vitest.

Build
As expected

Unit test
URL in unit test doesn't have /foo. It supposes to be http://localhost:5173/foo/icon.png

[sapphi-red]

This test is failing because during dev await getBg('.inline-style-public') is "url("http://localhost:5173/icon.png")".
Vite's test runs with dev and then runs with build+preview.

I think there is a different bug here. await getBg('.inline-style-public') should return url("http://localhost:5173/foo/icon.png") and not url("http://localhost:5173/icon.png")" during dev too. (because src="/raw.js"(at assets/index.html line 18) is rewrote to src="/foo/raw.js" during dev.

[nvh95]

Yeah. I guess there is a bug in Vitest configuration here but I'm not familiar so I haven't catched it yet. I will investigate more. But if you spot the bug, please let me know, or just push the change directly to my branch. Much appreciated.

[nvh95]

The weird thing is that if it's class style, it works fine. The test fail with inline style only. So I am nor sure it's a bug in Vitest config or the code implementation 😂

[sapphi-red]

I guess there is a bug in Vitest configuration here but I'm not familiar so I haven't catched it yet.

No, I meant there is a bug in Vite.

The weird thing is that if it's class style, it works fine.

Good finding 👍 I think this is a different bug.
I haven't checked closely yet but I suppose style attributes needs to be rewrote here during dev.

vite/packages/vite/src/node/server/middlewares/indexHtml.ts

Lines 185 to 253 in 2c3d85a

	await traverseHtml(html, htmlPath, (node) => {
	if (node.type !== NodeTypes.ELEMENT) {
	return
	}
	// script tags
	if (node.tag === 'script') {
	const { src, isModule } = getScriptInfo(node)
	if (src) {
	processNodeUrl(src, s, config, htmlPath, originalUrl, moduleGraph)
	} else if (isModule && node.children.length) {
	addInlineModule(node, 'js')
	}
	}
	if (node.tag === 'style' && node.children.length) {
	const children = node.children[0] as TextNode
	styleUrl.push({
	start: children.loc.start.offset,
	end: children.loc.end.offset,
	code: children.content
	})
	}
	// elements with [href/src] attrs
	const assetAttrs = assetAttrsConfig[node.tag]
	if (assetAttrs) {
	for (const p of node.props) {
	if (
	p.type === NodeTypes.ATTRIBUTE &&
	p.value &&
	assetAttrs.includes(p.name)
	) {
	processNodeUrl(p, s, config, htmlPath, originalUrl)
	}
	}
	}
	})
	await Promise.all(
	styleUrl.map(async ({ start, end, code }, index) => {
	const url = `${proxyModulePath}?html-proxy&direct&index=${index}.css`
	// ensure module in graph after successful load
	const mod = await moduleGraph.ensureEntryFromUrl(url, false)
	ensureWatchedFile(watcher, mod.file, config.root)
	const result = await server!.pluginContainer.transform(code, mod.id!)
	s.overwrite(start, end, result?.code || '')
	})
	)
	html = s.toString()
	return {
	html,
	tags: [
	{
	tag: 'script',
	attrs: {
	type: 'module',
	src: path.posix.join(base, CLIENT_PUBLIC_PATH)
	},
	injectTo: 'head-prepend'
	}
	]
	}
	}

If you prefer, I think we could go with commenting out this line for now since it's a different bug. (I'll create a issue or PR later)

[nvh95]

Thank for your help. I will separate it to a different test case then skip it for now. I will push it soon.

[nvh95]

I just re-ran the code again in DEV mode and BUILD mode. I confirm that the url() in inline style is missing base url.

Dev mode
Missing /foo

This is what I get when I visit http://127.0.0.1:5173/icon.png directly.

But I don't understand why it works fine as a background image in a CSS rule

Build mode
Have /foo

So I guess we just discovered another bug.

I also pushed the commit to skip the failed test for DEV mode.