fix(remix-dev/vite): invalidate route manifest on route export change #8157
Conversation
🦋 Changeset detectedLatest commit: dff9f79 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
hi-ogawa
force-pushed
the
fix-vite-invalidate-vmod-on-hot-exports-change
branch
2 times, most recently
from
82cef66 to
500b839
Compare
hi-ogawa
changed the title
hi-ogawa
changed the title
| if (browserName === "webkit") { | ||
| // force new page instance for webkit. | ||
| // otherwise browser doesn't seem to fetch new manifest probably due to caching. | ||
| page = await context.newPage(); | ||
| } |
There was a problem hiding this comment.
I found a very odd quirk on webkit after the huge amount of debugging (on CI).
It seems webkit is using caching for some assets between multiple page.goto, reload, etc... calls.
In this test, I'm almost certain that /@id/__x00__virtual:browser-manifest (aka. browserManifestId) was cached and webkit doesn't fetch new manifest when page.goto after server invalidated the module.
I hope this behavior is only playwright integration specific quirk and it doesn't happen on local usage of mac/safari.
I would appreciate if someone can verify this locally.
There was a problem hiding this comment.
Thanks for the PR! Sorry for the delay on this. I'll follow up on your other PR tomorrow, but for now I'm keen to get this fix in as-is.
I've added clientLoader and clientAction to the list of checked route exports since they've recently been introduced, and I've made a few minor refactors, but otherwise this looks great!
|
Thanks for follow up! No worries a all. Please feel free to ping me if you want me to fix merge conflict or deal with something in the PR. |
|
🤖 Hello there, We just published version Thanks! |
|
🤖 Hello there, We just published version Thanks! |
Closes: #8114
I went with a very direct approach just to fix this exact issue and the change itself is relatively small, but it might make sense to move this part of code into own filesystem watcher, which I'm experimenting in #8164.
Testing Strategy
I added
integration/vite-manifest-invalidation-test.tsto test the similar scenario from the reproduction.