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

our nextjs app: hot module reloading mysteriously stopped working a few months ago #7067

Closed
williamstein opened this issue Nov 26, 2023 · 4 comments
Assignees

Comments

@williamstein
Copy link
Contributor

No clue why, but HMR stopped working.

HINT: If you directly run webpack in dev mode

~/cocalc/src/packages/next$ pnpm exec next dev
  ▲ Next.js 13.5.6
  - Local:        http://localhost:3000

then visit http://localhost:3000, then HMR does work.

So the problem is somehow interaction between the hub express server and nextjs.

@williamstein
Copy link
Contributor Author

NOTE - using pnpm exec next dev is not a workaround, since it doesn't work.

@williamstein williamstein reopened this Nov 26, 2023
@williamstein williamstein self-assigned this Nov 26, 2023
williamstein added a commit that referenced this issue Nov 26, 2023
- see the comment; this fixes it for me, but it's so scary and wrong.
- fortunately the hackery should only impact dev mode
@williamstein
Copy link
Contributor Author

Note:

We can't robustly do HMR with nextjs and also with the app, and also all the other
websocket stuff we do, all at once, in the same nodejs process.
The problem is that there are four separate websocket "upgrade" handlers when we are doing
development, and nodejs just doesn't hav a good solution to multiple websocket handlers,
as explained here: nodejs/node#6339
The four upgrade handlers are:

  • this proxy here
  • the main hub primus one
  • the HMR reloader for that static webpack server for the app
  • the HMR reloader for nextjs
    These all just sort of randomly fight for any incoming "upgrade" event,
    and if they don't like it, tend to try to kill the socket. It's totally insane.
    What's worse is that getEventListeners only seems to ever return two
    listeners. By extensive trial and error, it seems to return first the primus
    listener, then the nextjs one. I have no idea why the order is that way; I would
    expect the reverse. And I don't know why this handler here isn't in the list.
    In any case, once we get a failed request and we see there are at least two
    other handlers (it's exactly two), we completely steal handling of the upgrade
    event here. We then call the appropriate other handler when needed.
    I have no idea how the HMR reloader for that static webpack plays into this,
    but it appears to just work for some reason.
    But this is just not robust, so I deleted the code.

@avarayr
Copy link

avarayr commented Dec 2, 2023

@williamstein I believe vercel/next.js#58704 should resolve your issues and enable you to handle websocket connections the way you want to.

@williamstein
Copy link
Contributor Author

Thanks! The comment there "To enable this, the base http server has the on('upgrade') handler removed. In this author's opinion, that handler is an anti-pattern as it makes it much more difficult to handle middleware and other request lifecycle behavior." is something I strongly agree with!

Things are working for us know, but it's ugly and there are disturbing messages in the log, so I'm looking forward to trying vercel/next.js#58704 . One major blocker though is that we can't upgrade to nextjs 14 at all, due to #7066, i.e., it seems that something breaks regarding using commonjs instead of ESM modules. Maybe that will get fixed eventually though, as 14.x is pretty new.

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

No branches or pull requests

2 participants