Skip to content

fix: addWatchFile doesn't work if base is specified (fixes #19792)#19794

Merged
sapphi-red merged 3 commits intovitejs:mainfrom
anatawa12:add-watch-file-base-url
Apr 11, 2025
Merged

fix: addWatchFile doesn't work if base is specified (fixes #19792)#19794
sapphi-red merged 3 commits intovitejs:mainfrom
anatawa12:add-watch-file-base-url

Conversation

@anatawa12
Copy link
Copy Markdown
Contributor

Description

Fixes #19792

URLs used in updateModuleInfo are relative to base url but normalizeUrl will prepend base url unconditionally.
This PR changes to not prepend base url if it's for pluginImports

Copy link
Copy Markdown
Contributor

@hi-ogawa hi-ogawa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix! Not sure which is better, but looking at other code, applying stripBase afterward might be more consistent.

Can you also add a test case in playground/transform-plugin? I think you can add playground/transform-plugin/vite.config-base.js and playground/transform-plugin/__tests__/base/transform-plugin.spec.ts. This is the convention to reuse the same test but with slightly different config (you can check other playground like playground/css/vite.config-xxx).

@anatawa12
Copy link
Copy Markdown
Contributor Author

I updated the implementation and added tests.

Similar to css family I moved test implementation to tests.ts and call it in transform-plugin.spec.ts and base/transform-plugin.spec.ts

@sapphi-red sapphi-red added feat: hmr p3-minor-bug An edge case that only affects very specific usage (priority) labels Apr 11, 2025
@sapphi-red
Copy link
Copy Markdown
Member

/ecosystem-ci run

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 11, 2025

Open in StackBlitz

npm i https://pkg.pr.new/vite@19794

commit: a920989

@vite-ecosystem-ci
Copy link
Copy Markdown

📝 Ran ecosystem CI on a920989: Open

suite result latest scheduled
storybook failure failure
vike success failure
vite-environment-examples success failure
laravel failure success
vite-plugin-react success failure
waku success failure

analogjs, astro, histoire, ladle, nuxt, rakkas, quasar, marko, sveltekit, previewjs, qwik, react-router, vite-plugin-svelte, vite-plugin-vue, unocss, vite-plugin-pwa, vite-setup-catalogue, vitest, vite-plugin-cloudflare, vite-plugin-react-swc, vuepress, vitepress

Copy link
Copy Markdown
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@sapphi-red sapphi-red merged commit 8bed1de into vitejs:main Apr 11, 2025
19 checks passed
@anatawa12 anatawa12 deleted the add-watch-file-base-url branch April 11, 2025 14:16
moonlitusun pushed a commit to moonlitusun/vite that referenced this pull request May 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat: hmr p3-minor-bug An edge case that only affects very specific usage (priority) trigger: preview

Projects

None yet

Development

Successfully merging this pull request may close these issues.

this.addWatchFile does not work as expected if base url is specified

3 participants