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

WIP: webpack 5 support #397

Closed
wants to merge 2 commits into from
Closed

WIP: webpack 5 support #397

wants to merge 2 commits into from

Conversation

glenjamin
Copy link
Collaborator

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

Webpack 5 made some changes, which have broken error handling functions in hot middleware

Breaking Changes

None, this should remain compatible with v4.

I intend to configure CI to run the tests against v4 as a bit of insurance for this.

Additional Info

Supercedes #393

Will either incorporate the fix from #394 or some equivalent once the error is fully understood

Also upgrade to use webpack@5, still an outstanding bug in applying
updates with errors
@alexander-akait
Copy link
Member

Ready for review?

@glenjamin
Copy link
Collaborator Author

Ready for review?

I'm happy to take notes on this, but i'm not intending to merge until the fix (or an equivalent) to #394 is included - and I want to understand exactly what's changed before applying that fix.

@wardpeet
Copy link

Would be nice to get this reviewed and merged. Even if it only fixes 80% of webpack 5 integration.

Co-authored-by: Ward Peeters <ward@coding-tech.com>
@samhuk
Copy link

samhuk commented Feb 25, 2021

Does the "bugfix" checkbox in the PR description also cover documentation updates? I wouldn't consider out-of-date docs as a "bug", but each to their own...

Also i second that this is important to expedite.

@weilinzung
Copy link

any updates on when we could get the fixes release?

@glenjamin
Copy link
Collaborator Author

glenjamin commented Mar 14, 2021 via email

@weilinzung
Copy link

weilinzung commented Mar 14, 2021

@glenjamin I am not 100% sure the issue, but I think it is related this middleware’s functions are not injecting to webpack 5 entry. E.g.: overlay and reload are both not working. With webpack 4 I can see the reload script is in the html, but not with webpack 5.

@TrejGun
Copy link

TrejGun commented Apr 4, 2021

@sokra the community struggles with this bug, can you please spend some time and advice on what was changed and how to migrate?

@alexander-akait
Copy link
Member

@TrejGun What is bug?

@TrejGun
Copy link

TrejGun commented Apr 5, 2021

@alexander-akait
there were some API changes in webpack 5 related to how it works with plugins (correct me if i'm wrong but I believe context was changed) . as a result hot-middleware is broken. community managed to write monkey patch which pretends that every update has failed and reloads module. Here is the code

https://github.com/webpack-contrib/webpack-hot-middleware/pull/394/files

The author of this lib is not working with this stack any more and does not seem to be interested to fix his code but is responsive and will accept PR if any
So the best thing I can do is to ask for help from webpack side. Maybe you guys can spare couple of hours and to point out the way how to properly migrate

@alexander-akait
Copy link
Member

@TrejGun Can you provide link on bug or example? Should work...

@glenjamin
Copy link
Collaborator Author

If/when I get time to poke at this a little deeper, my plan would be to:

  • using the example project on webpack 4, run through the main scenarios for success/error/unaccepted, note which event handlers fire
  • upgrade to webpack 5, repeat scenarios, note what the difference is
  • understand why there is a difference, referencing changes in webpack's source and other hot reload clients
  • either submit a bug to webpack, or adjust this module to handle the changes

If there are people who need this to work, they should be able to follow these steps themselves to work towards an understanding and then a solution.

@wardpeet
Copy link

wardpeet commented Apr 5, 2021

We have a fork of whm at gatsby that we're currently using in "Production" which works with fast-refresh, ...

Changes we did on top of it are:
gatsbyjs@4ea7d59

I did not check for webpack 4 compatibility.

@alexander-akait
Copy link
Member

@wardpeet should work with webpack v4

@weilinzung
Copy link

@wardpeet Your fork for the overlay is not working.

'webpack-hot-middleware/client?path=/__webpack_hmr&reload=true&overlay=true'

@mcat95
Copy link

mcat95 commented Sep 15, 2021

Is there any update on this? Maybe you could add a warning on the README warning that webpack 5 is not fully supported yet, since I had a hard time finding this issue

@TrejGun
Copy link

TrejGun commented Sep 15, 2021

@mcat95 you can check this issue
webpack/webpack#12408

@amoroso81
Copy link

After some tests, I can confirm that the fix proposed in fork https://github.com/lukeapage/webpack-hot-middleware#2cdfe0d0111dab6432b8683112fd2d17a5e80572 is working on Webpack 5. Good work, lukeapage!

@brunolau
Copy link

Any chance this gets merged? Fixes the overlay issue for our stack and it's always good to have at least partial support then none

@chucknelson
Copy link

Any chance this gets merged? Fixes the overlay issue for our stack and it's always good to have at least partial support then none

So there was supposedly a fix merged for the related issue webpack/webpack#12408, released with webpack 5.72.0, but when I look at that PR...I only see some test cases updated? I might not understand what those files in the PR actually impact...

@vankop
Copy link

vankop commented Apr 14, 2022

@chucknelson fix was in separate PR, in mentioned PR only test cases..

@chucknelson
Copy link

@chucknelson fix was in separate PR, in mentioned PR only test cases..

Thanks @vankop - I can't find the PR or commit for the actual fix. Do you happen to have a link to that PR?

@glenjamin
Copy link
Collaborator Author

I spent a little time understanding what the remaining outstanding issue is - it does appear to be a change in webpack core, so I'm going to try and produce a minimal reproduction case and follow up there.

If anyone knows of existing reports that aren't already linked from here - please do comment with links.

The Problem Scenario

  1. Save a file with a syntax error
  2. The error overlay shows up
  3. Fix the syntax error and save the file
  4. The error overlay closes
  5. The file that had the error will now no-longer apply hot updates

In webpack 5

  • The first update notes the bundle has errors, and does not apply
  • The second update applies the broken bundle
    • which causes the apply() of that module to error with self-accept-errored
  • Future updates claim to update the broken module, but don't actually do anything

In webpack 4

  • The above scenario works correctly, and the module can be updated.
  • The first update notes the bundle has errors, and does not apply
  • The second update applies the now fixed bundle
  • If a module breaks with self-accept-errored (eg. throws inside if (module.hot) { ...), then future updates will error as unaccepted - rather than claiming to work and doing nothing

@ssbyoung
Copy link

ssbyoung commented Aug 8, 2022

Any update on this? Or does anyone have a good replacement for this library?

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