vite

vite copied to clipboard

Context

Vite currently bundles the config file during development using ESBuild and all the relative imports are marked as "no external" because they are not matched in this regex, meaning that the imported code is read and bundled in the config.

This is great because it allows reloading the imported local plugins without restarting the process by calling viteDevServer.restart(). Otherwise, Node would cache the external imports (import cache) and wouldn't reload the code even if it changes.

Problem

We have a few plugins in a monorepo together with a project template and we'd like to reload the plugins in the template dev server when they change. The problem is that the template is importing the local plugins using NPM workspaces (e.g. published NPM name @shopify/hydrogen/vite) instead of relative paths like ../../packages/hydrogen/dist/plugin.js.

Therefore, our plugins are treated as external imports and Node's import cache makes it impossible to reload the plugin without restarting the whole dev server process.

Changes

Vite already follow symlinks in monorepos for in-app dependencies so I think it would be more consistent if it did the same for plugins in the config file. This PR makes it so imports that resolve to local files are not marked as external anymore.

I'm not sure if this is the correct fix so please let me know what you think.

Run & review this pull request in StackBlitz Codeflow.

It is breaking one of our test because we use a link:../package. We probably can break this test case in Vite 6, we need to solve this. I don't know if the fix is correct though. If the plugin package is optimized for example, we should continue treating it as external, no?

If the plugin package is optimized for example, we should continue treating it as external, no?

If it's optimized, wouldn't the path come from node_modules/.vite/xyz? Or is there any other way to check if it's optimized?

The cache dir is at node_modules/.vite by default but it can be changed using config.cacheDir. An optimized dep would start with {cacheDir}/deps (without the / because we also have {cacheDir}/deps_ssr).

Isn't this about config bundling, so the optimizer shouldn't be in play here? The test fail is not too terrible I think and I'm ok with fixing the test.

This will fix https://github.com/vitejs/vite/issues/5370, which (seems like) I also tried this solution in the past but had issues with __filename and __dirname after bundling the deps: https://github.com/vitejs/vite/issues/5370#issuecomment-1166421844. Maybe it's not an issue now.

Isn't this about config bundling, so the optimizer shouldn't be in play here? The test fail is not too terrible I think and I'm ok with fixing the test.

Oh, you're right. Ok, ya, we can update the test then. Do you think it is safe to include this in 5.3?

/ecosystem-ci run

📝 Ran ecosystem CI on 65c037a: Open

suite	result	latest scheduled
histoire	:white_check_mark: success	:x: failure
nuxt	:x: failure	:x: failure
qwik	:x: failure	:white_check_mark: success
remix	:x: failure	:white_check_mark: success
sveltekit	:x: failure	:x: failure
vike	:x: failure	:white_check_mark: success
vite-plugin-react	:x: failure	:white_check_mark: success
vite-plugin-react-pages	:x: failure	:x: failure
vite-plugin-vue	:x: failure	:white_check_mark: success
vitest	:x: failure	:x: failure

:white_check_mark: analogjs, astro, ladle, laravel, marko, previewjs, quasar, rakkas, unocss, vite-plugin-pwa, vite-plugin-react-swc, vite-plugin-svelte, vite-setup-catalogue, vitepress

@patak-dev @bluwy Please also check #17286 as this is a similar idea to fix the same issue (#5370), but specifically uses tsconfig paths.

Might be a more robust solution as it requires the tsconfig to be setup correctly for it to work. Might also be a sledgehammer to crack a nut :) Either way would be good to get eyes on it.

@garydex I've replied in the other PR

---

About the ecosystem-ci results, seems like it does break a few downstream frameworks, and perhaps we should move this to the next major.

Hm yeah, the dirname/filename issue is indeed unfortunate. It might be a bit of an overkill but here's another idea:

1. Get the import statements from the config file (e.g. with es-module-lexer or simple regexps for static imports).
2. Filter out local paths (./, ../).
3. Try to resolve each remaining import with import.meta.resolve or require.resolve.
4. Filter out paths from /node_modules/, and take only the ones that resolve to local files
5. Bundle the config file normally like we do right now
6. Replace imports that resolve to local files (from step 4) in the resulting bundled config with the resolved imports but adding random query strings ?t=123 to bust the import cache.
7. Write to disk and import it dynamically like it's done now.

Would that work? We would end up with ./something, @remix-run/dev unmodified but replace @shopify/hydrogen/vite with ../../packages/hydrogen/dist/vite/plugin.js?t=123

--

Edit: problems I see with this:

• It might not work with package.json#exports in some situations?
• It wouldn't work if what you're importing is not already valid JS

@bluwy @patak-dev Will this be targeted to v6? Would be great to have the untranspiled monorepos config working.

Though the log is not available now, when I checked it last time, vite-plugin-react ecosystem ci failure is due to "non static" dynamic import (similar to https://github.com/vitejs/vite/issues/13730). Playground's vite.config.ts is bundled with @vitejs/plugin-react as it's a workspace package, but statically-non-analyzable import("react-refresh/babel") used by @vitejs/plugin-react is not rewritten, so they become not found in playground.