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

fix: prevent overlay code from running if disabled #374

Merged
merged 1 commit into from
May 30, 2021
Merged

Conversation

keraf
Copy link
Contributor

@keraf keraf commented Apr 26, 2021

Setting the overlay property to false in the plugin options blows up when loading the page. The following error message is displayed in the latest beta release (0.5.0-beta.4, same issue is present in 0.4.3):
Screenshot from 2021-04-26 15-25-24

Checking if __react_refresh_socket__ is truth-y does the trick. __react_refresh_socket__ is set to false when the overlay is disabled (see lib/index.js#L101).

@pmmmwh
Copy link
Owner

pmmmwh commented Apr 26, 2021

Hmm, this seems a bit weird to me.

By default the overlay entry would not be injected if overlay is set to false.

Are you using it directly?

(I would happily accept this fix, just wanted to know the root cause of this)

@keraf
Copy link
Contributor Author

keraf commented Apr 26, 2021

I just tried to reproduce it on a different machine and using the webpack-plugin-serve example project. The ErrorOverlayEntry client is indeed not injected when overlay is set to false. But for some reason, in my project it still gets injected, leading to this error (and the app breaking).

No clue why this could be happening. I use a more recent version of webpack (5.28.0) and webpack-plugin-server (1.4.1). Or it could just be a misconfiguration in my config. I will investigate it tomorrow and keep you updated on my findings.

@keraf
Copy link
Contributor Author

keraf commented Apr 30, 2021

Sorry for the late reply. I've updated the dependencies in the example projects just to make sure and it works as expected without this "fix". I guess I must have fudged something in my configuration. I will keep you updated if I find out why.

@pmmmwh
Copy link
Owner

pmmmwh commented May 30, 2021

I'll move forward to merge this in the spirit of keeping things in the plugin more "re-usable" when something custom is needed.

@pmmmwh pmmmwh self-requested a review May 30, 2021 22:51
@pmmmwh pmmmwh merged commit 2c268ab into pmmmwh:main May 30, 2021
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.

None yet

2 participants