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: Windows client-side script reloads on dev server #4645

Merged
merged 7 commits into from
Sep 9, 2022

Conversation

bholmesdev
Copy link
Contributor

Changes

Testing

TODO: e2e test

Docs

N/A

@changeset-bot
Copy link

changeset-bot bot commented Sep 6, 2022

🦋 Changeset detected

Latest commit: 5329097

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

This PR includes changesets to release 16 packages
Name Type
astro Patch
@e2e/astro-component Patch
@e2e/error-cyclic Patch
@e2e/error-react-spectrum Patch
@e2e/error-sass Patch
@e2e/errors Patch
@e2e/hydration-race Patch
@e2e/lit-component Patch
@e2e/preact-component Patch
@e2e/react-component Patch
@e2e/solid-component Patch
@e2e/solid-recurse Patch
@e2e/svelte-component Patch
@e2e/e2e-tailwindcss Patch
@e2e/ts-resolution Patch
@e2e/third-party-astro 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

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Sep 6, 2022
@bholmesdev
Copy link
Contributor Author

Well, it looks like the hotfix @natemoo-re and I discovered doesn't work outside our local test setup. I was already confused why our fix would work anyways, so I'm tabling this effort for now until @bluwy (or another Vite maintainer) is free. Pretty stumped on this one!

@bholmesdev
Copy link
Contributor Author

Alright, tossing over to @matthewp to see what happens! Here's everything I tried (that didn't work):

  • appending /@fs/, /@id/, /@fs//, /@id// or a simple / to our generated imports and/or our scripts.ts elements
  • removing our generated imports from vite-plugin-astro entirely. These should be redundant since we inject a script tag into the page already. Funny enough, HMR continues to work after deleting this section on Unix
  • using a root-relative path (i.e. /src/pages/index.astro...) in our generated imports and/or scripts.ts elements
  • **adding a ?t=[random-hash] to our generated imports and/or scripts.ts elements

I also have some miscellaneous "woah really?" moments:

  • hoisted scripts will work in Stackblitz on Windows. They may be using linux subsystem under-the-hood, or something unrelated is going on 🤷
  • visiting a hoisted script's path in the dev server with a different path (ex. visit http://localhost:3000/src/pages/index.astro?astro&type=script&lang.ts when the homepage loads /Users/.../src/pages/index.astro...) will have the most up-to-date script contents on first visit, despite the contents being out-of-date on the actual page

Copy link
Contributor Author

@bholmesdev bholmesdev left a comment

Choose a reason for hiding this comment

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

I'll pretend to understand why this solves the problem with... a comment approval ✅

@matthewp
Copy link
Contributor

matthewp commented Sep 9, 2022

@bholmesdev It's because Vite determines which modules to invalidate based on which files they belong to. It determines this because it expects resolveId to return the file path.

In this case we have to make sure that it begins with C: (or whatever drive it is) on Windows and this does that.

@matthewp matthewp merged commit f27ca6a into main Sep 9, 2022
@matthewp matthewp deleted the fix/windows-client-scripts branch September 9, 2022 14:53
This was referenced Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 BUG: Vite's HMR not working with client-side scripts in new Beta
2 participants