Module becomes undefined after refresh due to cyclic dependencies

[sophiebits]

I'm experiencing a bug where upon changing a file, one of my modules becomes undefined (causing part of the page to disappear).

I've mostly reduced my repro case. It seems related to cyclic dependencies. Repro steps:

1. Clone https://github.com/sophiebits/refresh-cycle-repro

2. Run yarn and yarn start, then visit http://localhost:7002/. You'll see the text "Hello: 100".

3. If you edit the "Hello" (in App.js), it updates properly.

4. If you edit the "100" (in SurveyOverview.js), the 100 disappears until next page reload.

   This is because the DashboardPage import in routes.js becomes undefined (as you can see from the console logs). If you check the same import a tick later, it is correctly populated.

Not sure whether the bug is in react-refresh-webpack-plugin, react-refresh, or webpack.

[Timer]

@sophiebits thanks for this excellent reproduction!

I've added a test case for this to the Fast Refresh conformance suite (destined for the facebook/react repo). Can you please confirm it tests all the cases you've found buggy?

vercel/next.js#12161

[sophiebits]

@Timer I think you're the better judge of that! But it does appear that you successfully copied my code into a test case. 😅 Does the fact that it passes CI mean that this works correctly in @next/react-refresh-utils already?

[pmmmwh]

@sophiebits Thanks for the reproduction! It seems that the latest version of the plugin also don't have this issue anymore, can you try it out and see if it fixes the issue for you too?

I'll further investigate this tonight.

[sophiebits]

Sorry, I had tried on the latest stable but not the latest beta.

For my mini repro case, I did a bisect and it seems that c66bda1 is the commit that fixed this, though I don't understand why.

But for my larger app, it still seems broken on 0.3.0-beta.5. 🙈 Will try to re-reduce when I have time but lmk if you have other ideas.

[pmmmwh]

For my mini repro case, I did a bisect and it seems that c66bda1 is the commit that fixed this, though I don't understand why.

It has something to do with whether SurveyOverview is being detected as a React component, because that affects whether the module is included in the react-refresh life cycle.

But for my larger app, it still seems broken on 0.3.0-beta.5. 🙈 Will try to re-reduce when I have time but lmk if you have other ideas.

I can confirm that if you rename SurveyOverview to surveyOverview, the issue persists (even in @next/react-refresh-utils). I'll have to look deeper and see which part of the integration is broken because it is most likely a race condition ☹️

[pmmmwh]

I have further investigated the issue and it seems that even without the plugin the issue still persists (given that we don't accept SurveyOverview, and accepts Milestones, NudgeOverview and App - as aligned with their corresponding Refresh Boundary status).

Tbh I'm not sure if we're hitting the limit of Webpack here, it seems to me that the issue only exists when there' both cyclic imports and reexported dependencies. I'll file an issue in Webpack's repository and see what we can do about it.

See webpack/webpack#11016

[pmmmwh]

Closing as it is marked as won't fix in upstream.

[romulof]

Closing as it is marked as won't fix in upstream.

It would be nice to have a disclaimer in README.md about cyclic dependencies.