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

HMR: Installing sass when in error state does not reload page #5145

Closed
1 task done
matthewp opened this issue Oct 21, 2022 · 7 comments · Fixed by #5412
Closed
1 task done

HMR: Installing sass when in error state does not reload page #5145

matthewp opened this issue Oct 21, 2022 · 7 comments · Fixed by #5412
Assignees
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: hmr Related to HMR (scope)

Comments

@matthewp
Copy link
Contributor

What version of astro are you using?

1.5.2

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

npm

What operating system are you using?

Mac

Describe the Bug

  1. Do not install sass.
  2. Add lang="scss" to a style attribute.
  3. You will get an error.
  4. npm install sass
  5. Should reload the browser, does not. Reloading manually fixes it.

Link to Minimal Reproducible Example

n/a

Participation

  • I am willing to submit a pull request for this issue.
@matthewp matthewp added - P4: important Violate documented behavior or significantly impacts performance (priority) feat: hmr Related to HMR (scope) labels Oct 21, 2022
@matthewp matthewp self-assigned this Oct 21, 2022
@matthewp
Copy link
Contributor Author

This is blocked by this Node core bug: nodejs/node#44663

@bluwy
Copy link
Member

bluwy commented Nov 3, 2022

I think it would be cool if this works, but I'm not sure if Vite currently works when installing packages on the fly too. Traditionally whenever installing or updating a package, it would be safer to reboot everything.

@matthewp
Copy link
Contributor Author

matthewp commented Nov 3, 2022

Vite would work in this case if not for the Node core bug. Another alternative is to launch Astro as a subprocess and to kill that subprocess on any package.json changes. That would be heavy, however.

@matthewp
Copy link
Contributor Author

matthewp commented Nov 4, 2022

Another solution being discussed is installing sass when it is attempted to be used, see https://discord.com/channels/804011606160703521/1037805603956998304.

@matthewp
Copy link
Contributor Author

matthewp commented Nov 4, 2022

Assigning to @bluwy who's going to update Vite to first check if the package exists before trying to require it.

@matthewp matthewp assigned bluwy and unassigned matthewp Nov 4, 2022
@bluwy
Copy link
Member

bluwy commented Nov 8, 2022

Ref Vite PR: vitejs/vite#10812. Still needs to be cherry-picked into the Vite v3 branch and released before we can make the change here.

@bluwy
Copy link
Member

bluwy commented Nov 15, 2022

The fix is released in Vite 3.2.4 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: hmr Related to HMR (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants