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 style crawling logic for CSS HMR #7565

Merged
merged 1 commit into from Jul 5, 2023
Merged

Fix style crawling logic for CSS HMR #7565

merged 1 commit into from Jul 5, 2023

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Jul 4, 2023

Changes

Fix #7529

Used the not-so-performant technique initially used in the first commit of #7381 to fix the issue for now.

The issue seems to be that UnoCSS is invalidating the Astro files at https://github.com/unocss/unocss/blob/757b7f18b060f32387689545c878aea1e63203b4/packages/vite/src/modes/global/dev.ts#L59. This breaks our expectations where they are already loaded and exist as ssrTransformResult. The invalidation clears out ssrTransformResult, causing the issue.

A fix I intially thought is to force loader.import() on all modules while we're traversing the graph to counteract UnoCSS, but that feels more non-performant than the initial PR idea, so I went with the initial PR idea instead.

Testing

Existing tests should pass. Also tested the repro locally. Unfortunately time is a bit tight for me now to add a real test with unocss.

Docs

n/a.

@changeset-bot
Copy link

changeset-bot bot commented Jul 4, 2023

🦋 Changeset detected

Latest commit: 9e90cc8

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

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 Jul 4, 2023
@bluwy bluwy merged commit 5ffdec7 into main Jul 5, 2023
13 checks passed
@bluwy bluwy deleted the fix-css-hmr-issue branch July 5, 2023 06:13
@astrobot-houston astrobot-houston mentioned this pull request Jul 5, 2023
@duxphp
Copy link

duxphp commented Jul 5, 2023

[unocss-hmr] failed to get unocss hash, hmr might not work

The error is still reported when using unocss

@bluwy
Copy link
Member Author

bluwy commented Jul 5, 2023

Can you provide a repro where that's happening?

@marvin-j97
Copy link
Contributor

Can you provide a repro where that's happening?

https://stackblitz.com/edit/withastro-astro-yrtpg7?file=package.json

@bluwy
Copy link
Member Author

bluwy commented Jul 5, 2023

Thanks for the repro. Seems like it's a browser warning log, not an error. And it's also happening in 2.7.0, which is out of scope of this PR as it's fixing a regression from 2.7.1.

IIUC this needs to be fix upstream at https://github.com/unocss/unocss/blob/757b7f18b060f32387689545c878aea1e63203b4/packages/vite/src/modes/global/dev.ts#L187C6-L198, skipping the warning if __vite_css__ is an empty string.

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.

CSS HMR is broken on 2.7.2
4 participants