feat(plugin-react-refresh): add include / exclude options

[IanVS]

Description

This PR unblocks a problem in storybook-builder-vite by allowing us to specify an exclude filter option to the plugin which prevents it from treating story files as HMR boundaries, as discussed in #3778.

I've tested this locally in my own storybook vite project, and it does what we want, allowing the change to storybook story files to propagate into storybook itself.

Additional context

I'd love to hear from anyone who is knowledgable about HMR and in particular getting vite HMR and webpack HMR to play nicely together. 🙏 But this PR is pretty straightforward and I think it could be generally useful in other situations as well, outside of storybook.

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the Commit Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[antfu]

I think we could follow the rollup's conversion to have include / exclude pair to construct the filter, same as the vue plugin, ref:

vite/packages/plugin-vue/src/index.ts

Lines 64 to 67 in a4acc16

	const filter = createFilter(
	rawOptions.include || /\.vue$/,
	rawOptions.exclude
	)

[IanVS]

Thanks for the reviews, @patak-js and @antfu, and sorry for my silly mistakes.

See comment

[fwouts]

I noticed a behaviour change in this PR that might not have been intentional.

I use a plugin similar to https://github.com/rollup/plugins/blob/master/packages/virtual/src/index.ts that provides virtual files for Vite. IDs need to start with \0 (otherwise Vite expects the file to exist on disk and crashes, see source).

It appears createFilter always returns false for a string that starts with \0, so no files provided by this plugin can be processed by the react-refresh plugin.

This could probably be fixed by stripping any \0 prefix in the ID before passing it to filter(). Would there be any objections if I sent a PR doing that?

[patak-dev]

We should probably align ourselves with rollup here. But it is strange to me that they would not even let you add these virtual files patterns to include somehow. You can't even opt in. Maybe you could send an issue to https://github.com/rollup/plugins to better understand why this is the case and fix it upstream?

[fwouts]

Thanks @patak-js, good call :)

Before posting there, here's what I found looking through the history of the Rollup pluginutils package:

• [createFilter] Add an option to include virtual modules? rollup/rollup-pluginutils#54
• Add "resolve" option to createFilter rollup/rollup-pluginutils#61

In particular, this comment seems to indicate that we're expected to remove \0 on our side.

Do you think this requires further clarification?

[patak-dev]

Good digging, let's discuss in the PR then 👍🏼
There are other places in Vite where we are using createFilter if you would like to check them out also. This call by rollup seems to make it hard for the ecosystem to play nicely with virtual modules using \0 though, but maybe it is needed.
In the Vite ecosystem, instead of \0 some plugins are using virtual:{package-name}/{virtual-file} because this plays nicely with URLs in dev. Check out https://github.com/hannoeru/vite-plugin-pages/blob/main/src/constants.ts for example. The first id is for back compat, then they started using virtual:. Vite internally needs to convert \0 to another string also for dev.

[fwouts]

Indeed I was able to get rid of \0 in my plugin with a little effort. In case someone else running into issues in the future is wondering, I had to circumvent injectSourcesContent() which would fail because the path didn't exist by returning { code: ..., map: "{}" } from my plugin's load() function, so map.mappings would be empty.

Given this and the fact that I was probably the only one affected by this, maybe we don't need to fix it after all :)