reduce eagerness of RefreshUtils to reduce circular dependency errors

[phryneas]

In our project, we have a lot (around 400) of - generally unproblematic - circular dependencies:

src/hooks/firstHook.js
src/hooks/secondHook.js (uses first hook, imported from src/hooks/index.js)
src/hooks/index.js (re-exports both hooks)

Technically, this is a circular depenceny if a file somewhere were to import secondHook from index.js, a la someComponent.js -> src/hooks/index.js -> src/hooks/secondHook.js -> src/hooks/index.js.

In practice, these kinds of re-exports are not a problem and webpack has always known to deal with that.

Now I added fast-refresh to replace our old react-hot-loader setup (which was running nicely) and it keeps chasing me from one circular dependency to the next - and most of them, like the above we really do not want to resolve. They are a conscious decision.

I have skimmed the source of react-refresh-webpack-plugin a bit and from my reading, the problem comes from the fact that react-refresh-webpack-plugin eagerly tries to register all components. Since we don't have components circularly importing from each other, that should be fine - but it also accesses the getter for everything that is not a component to check if it is a component.

This PR attempts to fix that by looking at the casing of the exports first - and only handling uppercased exports as potential components that have to be checked by isLikelyComponentType. That leads to a lot less eager evaluation and fixes the circular dependencies in our project.

I am not an expert in this and I might have missed some edge cases, but I would really like to hear what you think about it!

[pmmmwh]

This change is not safe, it will likely break HMR when you change something other than React components (e.g. failing to force refresh). If you look at this code, the checks does quite a bit more than just checking casing of the name. Casing of the name is one of the criteria we check, but not a SUFFICIENT criteria.

Is it possible for you to create a reproduction regarding the problem and then we can figure out what's the best way forward? However, I also stand on the side that circular imports are bad. What makes it so hard to just import from ./firstHook instead of from ./index?

[phryneas]

Wouldn't everything that is checked for there at least have to have a capitalized name (or be a default export so it can be imported with a capitalized name) so it is able to be used in JSX?

I know that the check is not enough on it's own, but I am pretty certain it would be safe to say that everything that is not capitalized could never be used as a React component, so doing the check after that should be pretty safe - or am I missing some specific case here?

I can try to build a repo in a few days, probably by end of the week.

As for "what is so hard": in this case, it's a eslint rule that disallowed all imports from the hooks subfolder except through the main export - so we just have 400 of those loops created over the last 4 years - and refactoring that out while everything works from a "module perspective" (modules themselves are actually pretty lazy) seems like a major refactor to pretty much just get back to the status quo of react-hot-loader just working fine right now.

Of course we could do it - but it doesn't really seem like a good use of time if we can fix it upstream in some way and maybe also have others benefit from that :)