perf(resolve): reduce vite client path checks

[patak-dev]

Description

@vite/client is an alias replaced by the resolved path to the vite client /root/.../client.mjs. This id reaches the resolve plugin first and it is then tested against root here. So tryFsResolve is called with /root/root/.../client.mjs, resulting in failed checks (like /root/root/.../client.mjs.ts, and all the default extensions).

This PR modifies the alias to use /@fs/root/.../client.mjs, so the resolve plugin will directly try this path and resolve it.

If #12450 is merged, then there will not even be a fs check for this path.

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[patak-dev]

/ecosystem-ci run

[vite-ecosystem-ci]

📝 Ran ecosystem CI: Open

suite	result
astro	✅ success
histoire	❌ failure
iles	✅ success
ladle	✅ success
laravel	✅ success
marko	✅ success
nuxt	❌ failure
previewjs	✅ success
qwik	✅ success
rakkas	✅ success
sveltekit	✅ success
vite-plugin-ssr	✅ success
vite-plugin-react	✅ success
vite-plugin-react-pages	✅ success
vite-plugin-react-swc	✅ success
vite-plugin-svelte	✅ success
vite-plugin-vue	✅ success
vite-setup-catalogue	✅ success
vitepress	✅ success
vitest	✅ success
windicss	✅ success

[bluwy]

Given resolve.alias is usually configured to map to absolute paths when the user configures it, can we optimize for all case of that? We probably need a cheap heuristic to tell whether a /... path is root-relative or absolute, I think that will help projects that uses aliases a lot.

(Currently trying some things out locally)

[patak-dev]

The problem is what happens if you alias to {root}/file.js and your project has a /{root}/{root}/file.js. How resolve works now is that an absolute URL, starting from the root should win here. It makes sense to avoid clashes with the file system if there are /{root}/a.js and /a.js in the fs for example. Maybe if the absolute path starts with /{root} we could do an exception, but I'm afraid of the edge cases it may bring.

The other issue is that aliases may not be resolved, so we could no longer do #12450 (maybe we need a new ?resolved concept to bring that back, but sheremet said that Vitest also expects /@fs/ to be resolved).

[bluwy]

The problem is what happens if you alias to {root}/file.js and your project has a /{root}/{root}/file.js

Yeah I was thinking we can do a one-time pass to verify if /{root}/{root} exist, and perform that root-relative resolve if it exists. In most projects I don't think it does so we can optimize that.

The other issue is that aliases may not be resolved, so we could no longer do #12450 (maybe we need a new ?resolved concept to bring that back, but sheremet said that Vitest also expects /@fs/ to be resolved).

I don't think it would affect #12450 since the idea doesn't interfere with @fs 🤔 Import analysis would resolve the absolute path to the actual file before prepending the @fs here

[bluwy]

Through some test, optimizing alias that maps to absolute paths is doable, but makes the code a bit complex. They are usually resolved under 1ms too, so I don't think it's worth it for now. Since this Vite client path is something we control, I think we can try this 👍

[bluwy]

I wonder if we should also clean up and do path.posix.join instead. It's done at ,

vite/packages/vite/src/node/plugins/asset.ts

Line 253 in 1a8af8d

rtn = path.posix.join(FS_PREFIX, id)

vite/packages/vite/src/node/plugins/importAnalysis.ts

Line 348 in 1a8af8d

url = path.posix.join(FS_PREFIX, resolved.id)

but not at

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

Line 228 in 1a8af8d

const url = `${FS_PREFIX}${file}`

[patak-dev]

Switched to path.posix.join in vitejs/vite@656e5d2 (#12471)

Should we add isFsPathId and fsPathToId in another PR and use them everywhere?

export function isFsPathId(id: string): boolean {
  return id.startsWith(FS_PREFIX)
}

export function fsPathToId(fsPath: string): string {
  return path.posix.join(FS_PREFIX, normalizePath(fsPath))
}

export function fsPathFromId(fsPath: string): string {

[bluwy]

Forgot to reply 😅 I don't really have a hard opinion if we want to abstract it, but usually I lean on not abstracting things until it becomes a problem.

---

Also maybe we can go ahead with this PR too now.

[patak-dev]

I think maybe we wait for this one a bit more. If #12450 is merged, then we will win a bit, but if not, I think we won't be winning much.

[patak-dev]

Let's try this one then now that #12450 has been merged