refactor: make HMR agnostic to environment#15179
Conversation
|
|
|
/ecosystem-ci run |
|
This looks great to me, thanks for dividing the progress on mergeable chunks! About naming, given that we already have |
Yeah, I was off-put by uppercase letters there, but if we already do it like that, I think it's better to adopt the same style here for consistency. |
Ya, the |
|
📝 Ran ecosystem CI on
|
sheremet-va
changed the title
sheremet-va
dismissed stale reviews from sapphi-red and patak-cat
via
601df1f
…backs if needed This is needed when closing a server for example (need to wait!)
packages/vite/src/shared/hmr.ts
Outdated
| public notifyListeners(event: string, data: any): unknown { | ||
| const cbs = this.customListenersMap.get(event) | ||
| if (cbs) { | ||
| return cbs.map((cb) => cb(data)) |
There was a problem hiding this comment.
There was an open issue in vite-node when the user closed the server in this callback, but reload happened too fast and new server was not able to reuse the existing port (because it was still in use). I don't know if this is relevant for HMR in the browser so I didn't touch the behavior there, but I wanted to await here when reusing this method in vite-node
There was a problem hiding this comment.
Oh, I didn't know the value in disposeMap and pruneMap can return a promise. Doing return Promise.allSettled() makes sense to me.
There was a problem hiding this comment.
void allows returning a Promise :P
function test(cb: () => void) {
cb()
}
test(() => {
return new Promise((r => r('1')))
})Issue in vitest: vitest-dev/vitest#2334
There was a problem hiding this comment.
I am returning Promise<void> from this method in the latest commit now
bluwy
left a comment
bluwy
left a comment
There was a problem hiding this comment.
Looks great! I like that the two concepts are now nicely grouped together.
4 participants
Description
This PR makes core HMR implementation agnostic to the environment so it can be reused (and enhanced if needed) in other runtimes (Node/Deno/Bun) like in #12165. No logic is changed.
Unfortunately, I had to move it into another file which breaks git history for client file a bit :(
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123).