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: match static files in dev based on the pathname to fix HMR #69

Merged

Conversation

philipp-spiess
Copy link
Contributor

This fixes an issue that I observed while trying out the HMR prerelease build of Remix.

When Remix makes the HMR request, it adds a ?t=123 parameter to the URL for cache busting. The fastify adapter however did not properly resolve these static assets because it was comparing url (e.g. build/routes/foo.js?t=123) with the filename using string equality.

This fixes the issue by parsing the URL and comparing the pathname instead.

To make the example run locally, I also had to upgrade to Remix 1.13.0 (1.12.0 had some issues loading the flat routes properly, somehow it was looking for them in /outes instead /routes 😅 ). I included this as a separate commit in this PR but I’m happy to remove it.

It doesn't seem like there's a setup for testing the full fastify plugin end-to-end so I verified this based on the example app and curl instead:

Before

Screenshot 2023-02-24 at 01 43 50

After

Screenshot 2023-02-24 at 01 51 23

@changeset-bot
Copy link

changeset-bot bot commented Feb 24, 2023

🦋 Changeset detected

Latest commit: 1c24d95

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@mcansh/remix-fastify Minor
remix-app-template Patch

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

@philipp-spiess philipp-spiess changed the title Match static files in dev based on the pathname fix: match static files in dev based on the pathname Feb 24, 2023
@philipp-spiess philipp-spiess changed the title fix: match static files in dev based on the pathname fix: match static files in dev based on the pathname to fix HMR Feb 24, 2023
Copy link
Owner

@mcansh mcansh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:chefkiss: LGTM via mobile, i’ll review again in the morning and cut a release

@mcansh mcansh merged commit 394c89c into mcansh:main Feb 24, 2023
@philipp-spiess philipp-spiess deleted the ps/fix-dev-static-path-matching-for-hmr branch February 24, 2023 15:09
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.

2 participants