fix: consider # as a valid dir symbol (fix #4701)

[hyf0]

fix #4701.

Description

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 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.

[Shinigami92]

It should fix #2346 🤔, does it?

[hyf0]

@Shinigami92 Yes, It's the same problem. I also found related PR #2432 but It looks like a different approach to fix the problem. Should I close mine?

[patak-dev]

Maybe you could fork #2432, and start from there? The PR is staled, it wasnt merged because of this requested change #2432 (comment)

[hyf0]

@patak-js I'm not sure the fix in #2432 is appropriate. After digging into codes, It seems that simply considering # as valid dir symbol fixes the problem.

[Shinigami92]

Okay, sadly it looks like there are failing tests caused on these PR changes 🙁

[hyf0]

It looks like there are many edge cases for symbol #. There are situations that # could not consider a valid dir symbol.

vite/packages/playground/assets/__tests__/assets.spec.ts

Lines 129 to 135 in ff50c22

	if (isBuild) {
	test('preserve postfix query/hash', () => {
	expect(findAssetFile(/\.css$/, 'foo')).toMatch(`woff2?#iefix`)
	})
	}
	})

I think the strategy @patak-js said is indeed a good strategy. I might still need to dig deep for a while.

[Shinigami92]

It looks like there are many edge cases for symbol #. There are situations that # could not consider a valid dir symbol.

vite/packages/playground/assets/__tests__/assets.spec.ts

Lines 129 to 135 in ff50c22

	if (isBuild) {
	test('preserve postfix query/hash', () => {
	expect(findAssetFile(/\.css$/, 'foo')).toMatch(`woff2?#iefix`)
	})
	}
	})

I think the strategy @patak-js said is indeed a good strategy. I might still need to dig deep for a while.

Thank you for tackling this issue
I think it is not allowed in query parameters, but in path 🤔
Could you try to implement a check for this?

[hyrious]

I think it is not allowed in query parameters, but in path 🤔

Does it mean # is maybe a dir symbol only if it is before ??

//a.com/b#c?d      /b#c may be a valid file (so we have 2 fs tests here)
//a.com/b?c#d      only /b has to be processed

Also, consider this:

//a.com/#c/d#/#/#/f#g

Assume we have to check from the right side, wow...

/#c/d#/#/#/f#g
/#c/d#/#/#/f
/#c/d#/#/(index)
/#c/d#/(index)
/#c/d
/(index)

Here we must make sure that trailing slash is necessary for referencing /index,
otherwice the list above will be longer.

From the left side, it is obviously wrong, take /ext/#/abc for example,

/ext/(index)      maybe exist!, but /ext/#/abc should be considered first

[Shinigami92]

I think it is not allowed in query parameters, but in path 🤔

Does it means # is maybe a dir symbol only if it is before ??

//a.com/b#c?d      /b#c may be a valid file (so we have 2 fs tests here)
//a.com/b?c#d      only /b has to be processed

Also, consider this:

//a.com/#c/d#/#/#/f#g

I assume yes, but could be wrong

[hyf0]

Kind of busy these days :(, I will look deeper this weekend.

[hyf0]

It's harder than I think :(. I'm not sure there is a way to pre-determine whether # is a dir symbol or a special query unless we actually try to resolve it.

I found the reason tests fail is because of processing CSS files.

vite/packages/playground/assets/css/icons.css

Lines 12 to 21 in ff50c22

	.icon-clock {
	background: url(../nested/fragment-bg.svg#icon-clock-view) no-repeat;
	}
	.icon-heart {
	background: url(../nested/fragment-bg.svg#icon-heart-view) no-repeat;
	}
	.icon-arrow-right {
	background: url(../nested/fragment-bg.svg#icon-arrow-right-view) no-repeat;

Simply considering # as a valid dir symbol will make vite could not find the correct SVG file imported from css, but <img /> is ok 🤔.

vite/packages/playground/assets/index.html

Lines 85 to 89 in ff50c22

	<img
	class="svg-frag-img"
	src="./nested/fragment.svg#icon-clock-view"
	alt=""
	/>

[bowencool]

Is there any update?

[hyf0]

The solution is an implementation of #2432 (comment). It should work, I just need to refactor it for better maintenance, then it should be ok to be reviewed.

[hyf0]

So, is this plugin never working 🤣 ?

[hyf0]

@Shinigami92 @patak-js Hi, the code should be ok to fix the issue. Sorry for pending so long.

[patak-dev]

Let's merge this one once we kick the beta for 2.7 so we get some extra testing

[patak-dev]

For reference, this only solves part of the issues with # and ? in paths. But after discussing with @iheyunfei we are not sure if a more complete solution is desired here, or we should push for the ecosystem to stop using these characters in their path names.