Modules with no react exports are treated as fast-refresh boundaries

[IanVS]

Describe the bug

In Dan's original comment describing fast-refresh (facebook/react#16604), he says only files that only export components should be treated as boundaries. But, it seems that the isRefreshBoundary function in @vitejs/plugin-react is much more lax. It seems to only check that export names are capitalized. But, they don't need to be functions, they can be exported objects. See the reproduction for an example of how this causes problems.

One approach we could take which might be more robust, would be to perform a runtime check using isLikelyComponentType from the react-refresh runtime library. That has more robust checks than I think we can accomplish statically in the AST. https://github.com/PepsRyuu/rollup-plugin-react-refresh/blob/master/index.js is an example of a project that takes roughly this approach.

Reproduction

https://stackblitz.com/edit/vitejs-vite-b4yvhl?file=src/not-react-components.jsx

System Info

N/A

Used Package Manager

npm

Logs

No response

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to vuejs/core instead.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[IanVS]

I'm willing to attempt a PR to use isLikelyComponentType if it's agreed that's a viable option by the core team.

[IanVS]

I've hit a snag, unfortunately. I was able to add a conditional import.meta.hot.accept() which checks if all the exports are react components, using isLikelyComponentType, and that part worked great. But I found that changes were still not bubbling up to the parent, and tracked down the reason:

vite/packages/vite/src/node/plugins/importAnalysis.ts

Lines 399 to 419 in 71eaacf

	if (rawUrl === 'import.meta') {
	const prop = source.slice(end, end + 4)
	if (prop === '.hot') {
	hasHMR = true
	if (source.slice(end + 4, end + 11) === '.accept') {
	// further analyze accepted modules
	if (source.slice(end + 4, end + 18) === '.acceptExports') {
	lexAcceptedHmrExports(
	source,
	source.indexOf('(', end + 18) + 1,
	acceptedExports
	)
	isPartiallySelfAccepting = true
	} else if (
	lexAcceptedHmrDeps(
	source,
	source.indexOf('(', end + 11) + 1,
	acceptedUrls
	)
	) {
	isSelfAccepting = true

It appears that the import analyzer only looks for the string import.meta.hot.accept() in the source code, and does not consider that it might be conditional. So, even when my if block would prevent the self-accept from happening, vite still does not allow the change to bubble to a parent and be dealt with there.

I tried to calculate a list of "acceptable" exports and use import.meta.hot.acceptExports (which would be really cool to do!), but again hit a problem because it's looking for a static list of export names, not a dynamic variable calculated at runtime.

I'd love to hear any thoughts from @patak-dev or @aleclarson (or anyone else!) about what could potentially be done here.