perf(resolve): avoid tryFsResolve for /@fs/ paths

[patak-dev]

Description

/@fs/ should already be resolved. I think we shouldn't need a full tryFsResolve (that checks for extensions and package entries). And we don't even need to check if the file exists.

Doing a PR to test CI.

---

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	❌ failure
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

[patak-dev]

It seems that ecosystem CI is happy too. @antfu double-checking that as a plugin author, you don't expect to be able to use /@fs/... with an unresolved path.

[patak-dev]

@Liknox if you spam the repo again, you'll be banned from the org (context: three other PRs got approvals or a generic comment). You're welcome to do reviews that are focused on the reason to move or not forward with a PR

[bluwy]

Astro used to rely on a hack with /@fs, where when it imports /@fs/.../Component.jsx, it wants to resolve to /@fs/.../Component.tsx, but Vite doesn't resolve that way, so Astro imported /@fs/.../Component instead which worked. This is resolved in Astro side now though, but putting out this usecase for reference.

I think vite-node / vitest relies on manually crafting /@fs too so would be good to check with @antfu and @sheremet-va. But since mucking around /@fs is internal API, this should be good even so.

[sheremet-va]

I think vite-node / vitest relies on manually crafting /@fs too so would be good to check with @antfu and @sheremet-va. But since mucking around /@fs is internal API, this should be good even so.

Vitest doesn't rely on manually crafting /@fs, we always use pluginContainer.resolve. Vite-Node actually expects /@fs/ to always point to the actual file on the file system, and doesn't check its existence.

[antfu]

I am not very aware of the context here. But I think if it passes for Vitest it's probably also ok with other usages.

[patak-dev]

Thanks for the PR to Astro and the context @bluwy. cc @danielroe @benmccann @brillout @Akryum, just in case you also relied on hacks using unresolved /@fs/ ids.

But that's a good point @antfu. Maybe we could include this one on Vite 4.3. I'll add it to the team board so we can check with Evan too because looking at the history, resolving after unwrapping /@fs/ paths was intentional at first. I think there may have been cases where the equivalent to the import analysis plugin didn't fully resolve the files and just prefixed certain import paths

[brillout]

👍 Sounds good to me.

[patak-dev]

We discussed this PR in today's team meeting and decided to move forward with it. Merging so we can test during the beta.