feat: implement hook filters#19602
Conversation
sapphi-red
added
the
p2-nice-to-have
|
/ecosystem-ci run |
commit: |
This comment was marked as outdated.
This comment was marked as outdated.
|
📝 Ran ecosystem CI on
✅ laravel, quasar, histoire, analogjs, marko, ladle, vike, rakkas, qwik, vite-environment-examples, vite-plugin-vue, vite-plugin-cloudflare, react-router, storybook, vite-plugin-react, vite-plugin-svelte, vite-plugin-react-swc, vite-setup-catalogue, sveltekit, astro, previewjs, vuepress, unocss, vitest, vite-plugin-pwa, waku, vitepress |
| const cwd = process.cwd() | ||
| return f | ||
| ? (id) => { | ||
| const normalizedId = normalizePath(path.relative(cwd, id)) |
There was a problem hiding this comment.
what happens with virtual ids here? Won't the virtual id end up mingled with the cwd path?
There was a problem hiding this comment.
It is passed as-is, probably because path.isAbsolute(id) returns false.
I've added a test for virtual ids 👍 : c3b3f70
There was a problem hiding this comment.
Ah, that's right. And it works fine for virtual:module too.
One issue we could have is that some virtual module ids start with a slash. For example, our own /@react-refresh from the plugin react. In this case, you'll end up with something like ../../../@react-refresh and the filter will fail.
Maybe it is fine because when someone adds a filter it is expected that the plugin won't be handling virtual modules and only get paths ids? And given that the relative path is below cwd, they all end up reject (by chance, it feels very hacky but it works).
There was a problem hiding this comment.
Yeah, virtual modules starting with a slash would be normalized unintentionally. This was happening with createFilter from pluginutils, so I assume it won't be a problem.
https://github.com/rollup/plugins/blob/master/packages/pluginutils/src/createFilter.ts
| if (idFilter && !idFilter(id)) { | ||
| return | ||
| } |
There was a problem hiding this comment.
nice that you can do this without performance penalty over the current environment injection
|
Added one more change to align with pluginutils: 32c1283 |
<!-- Thank you for contributing! --> ### Description Ported tests from vitejs/vite#19602 to ensure the behavior is consistent.
2 participants
Description
Built on top of #19586This PR implements hook filters that are supported by rolldown.
It does not improve performance unless rolldown is used, but this would improve performance when we migrate to rolldown and would reduce the diff between rolldown powered Vite and rollup powered Vite.