Description

Edit: after internal discussions, the PR is only doing the workaround for /@react-refresh. We'll remove this check once we have a proper solution to move the vite-plugin-react to use the \0 convention or decide to properly formalize a new convention. @bluwy was proposing a new meta flag returned from resolve id for example. I would prefer that conventions keep being reflected on the path itself. Maybe we should expose a function isVirtualModule(id) so we can at one point have an easier time in case a change in conventions is needed.

In the profile for windows shared by @sapphi-red (recorded against #15279), the internal normalizeUrl in the import analysis plugin is now taking more time than resolveId. Most of the time is spent in fs.existsSync.

This is 2% of the total work now.

The cached fs tree isn't kicking here because we are checking if an out-of-root absolute path exists in the fs. But the real problem is that we are calling fs.existsSync('/@react-refresh') over and over in this benchmark.

vite-plugin-react-swc is using /@react-refresh both for authoring, and as the resolved id. See https://github.com/vitejs/vite-plugin-react-swc/blob/37f002cc9ecdf7e616004ad61ad9eeb3e3b6e873/src/index.ts#L73C8-L73C8. It isn't prefixing the id with the virtual file convention of \0.

We could start prefixing the resolved virtual module with \0. @ArnaudBarre, I think we should try to add that in the next major. I'm a bit afraid though in this case because there may be integrations that expect the resolved id to be /@react-refresh. It seems the ecosystem has this id harcoded everywhere. I think there are some plugins that could break here: https://github.com/search?q=%2F%40react-refresh&type=code

The other problem is that /@plugin-name/xxx is a convention that the ecosystem has widely used for resolved ids. This was started by us, with /@vite/client, /@fs/..., /@id/..., and /@react-refresh. And I saw /@vite-plugin-pwa, /@vite-plugin-pages/..., among others.

Given that we already don't support URLs that are absolute paths starting with /@vite, /@fs, /@id at the root of the filesystem. This PR generalizes that to URLs starting with /@. It only touches the check that is causing issues, which is done after resolving so plugins can't avoid it (there is even a comment in the react plugin about using order pre to avoid fs calls).

This is only for the root of the filesystem, and not the root of the project. We don't support /@vite, /@fs, /@id either at the root of the project though. I think we could in the future (next minor or major), formalize that /@... is a valid virtual module convention and also avoid checks at the root of the project. For now, I only added this check so we stop doing these expensive fs exists calls.

A detail, in my M1, it seems it these checks aren't that expensive. We could also avoid the check only for Windows, where it makes even less sense to be checking for these paths, but I think it is better to do it for all platforms.

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other