fix(dev): fix import failure on filenames with hashes

[davidwallacejackson]

close #2346

Description

Credit to @iheyunfei, who took a crack at this first in #4703 -- I started with their work. If it'd be more appropriate to merge this into that PR somehow, just let me know!

Right now, all imports of files with a # character in their path fail because the hash is interpreted as a denoting a postfix. To support hashes in file paths without breaking support for postfix hashes, I'm just trying both -- postfix interpretation first, then file path interpretation. This still won't work for hash postfixes on files that also have hashes in their names, but I'm not sure how common that is.

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.

[davidwallacejackson]

needed to do this because errors in a returned promise in async aren't caught by try unless the promise is awaited -- so files with # still failed even though resolve.ts now handles them

[davidwallacejackson]

Looking at the test failures and could use a little help: I see that the problem comes when we're trying to serve the file inside of a /#/ directory, and I can sort of see why that would be -- it would make sense to me if some system earlier in the process chopped off everything after the hash when the request for the file was coming in over HTTP.

I'm not sure what I should do about that, though. I noticed that the functions in resolve.ts have a targetWeb boolean, which I assume is intended to allow me to address inconsistencies like this, but if I were to, say, urlencode the hash, the path would be invalid if any other code tried to use it to access the filesystem -- which I'm guessing would be a problem? I'm now imagining some scheme wherein the client replaces # characters in module paths with some magic string like __HASH_CHARACTER__ and then the server swaps that out around here: https://github.com/davidwallacejackson/vite/blob/9512fe190495e1f5629973973b9b19152a8be242/packages/vite/src/node/plugins/loadFallback.ts#L10

...but that seems very complicated. Maybe there's an easier way I'm missing?

[huchangfa123]

when can this bug fix : (

[davidwallacejackson]

I am still working on this, but it's proving really tough: there are many places in the code where it's necessary to encode/decode URLs to deal with hashes, and some places where the hash character has a separate established use (as an alternate indicator for queries). The latter means that there are situations in which it's impossible to determine the correct way to parse a URL without just presenting multiple options and trying all of them -- something that doesn't always fit into the way the code is currently structured. For an example, see resolveUrl here -- when a hash appears in what we'd intuitively think of as the "pathname" this code will mangle the path (e.g. /#subdir/index.js becomes /.js#subdir/index.js).

Would appreciate some input from maintainers on this -- I suspect strongly that there's a less troublesome way to do this if I hook into the right parts of the URL-processing pipeline, but it's hard to figure out what those are without context.

[patak-cat]

@davidwallacejackson we just merged #4703, which fixes part of this issue. I don't know if a more complete solution is going to be needed here but if someone wants to continue exploring it we can review it. Encoding and decoding URLs as we discussed because of this PR for example. We could leave this PR open or closed it now that at least #4703 makes this problem a bit less prominent.

[hyf0]

some context.

Yes, if the code is using cjs, writing something like require('./#/foo.js') is totally fine and that's the reason for #4703.
But, Vite is using esm. esm doesn't handleimport './#/foo.js' well, which is more of a bug of browser.

We could see whatwg/html#5162 to get more context.

It seems improper to fix in Vite. The issue consider # in import foo from '/foo#.js' not a dir symbol(which we think it is in Vite view) but a fragment, which we could get the fragment by new URL(import.meta.url).hash // => '#.js'. in javascript file /foo.

So, even the bug was fixed. For import foo from './foo/#/index.js', the script we were requesting actually is './foo/' with fragment #/index.js not ./foo/#/index.js.