Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(hmr): add full reload reason #14914

Merged
merged 1 commit into from
Nov 10, 2023
Merged

feat(hmr): add full reload reason #14914

merged 1 commit into from
Nov 10, 2023

Conversation

ArnaudBarre
Copy link
Member

@ArnaudBarre ArnaudBarre commented Nov 8, 2023

Following #14867

Screenshot 2023-11-08 at 23 20 19

Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to elevate the circular imports issue. I also wonder if we should skip the debugHmr part and always log the full circular imports path instead. My PR kinda hides it but it might be worth making it more obvious.

Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the right amount of pressure. Listing the paths everytime feels a bit too noisy for me. Perhaps adding a link to the troubleshooting guide only for the first time might be good.

@patak-dev
Copy link
Member

LGTM too. Lets merge and maybe iterate on the message later.

@patak-dev patak-dev merged commit 60a020e into main Nov 10, 2023
11 checks passed
@patak-dev patak-dev deleted the reload-hint branch November 10, 2023 07:59
@ArnaudBarre
Copy link
Member Author

Yeah I think the path can be quite long. And I was not sure of building a truncate version of it.
Maybe linking once to the troubleshooting can be good. Or maybe directly something like "run with --debug hmr to print cycle"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants