Handle compiler breaking change #5803
Conversation
🦋 Changeset detectedLatest commit: 5f1c619 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
bot
added
the
pkg: astro
bluwy
changed the title
| # On Windows, `svelte-preprocess` can't import `svelte/compiler`. Might be a pnpm bug. | ||
| public-hoist-pattern[]=svelte |
There was a problem hiding this comment.
We don't have the svelte-preprocess dependency anymore, so this can be removed.
| "allowAny": [ | ||
| "astro" |
There was a problem hiding this comment.
This is added to suppress the peer dep warning of astro-embed, since it relied on Astro 1.0, but we want to test in 2.0.
| // But we also need a fresh, uncached result to compare it to | ||
| // Skip HMR if source isn't changed | ||
| if (oldResult.source === source) return []; | ||
| // Invalidate to get fresh, uncached result to compare it to | ||
| invalidateCompilation(config, ctx.file); | ||
| const newResult = await compile(); | ||
| // If the hashes are identical, we assume only styles have changed | ||
| if (oldResult.scope === newResult.scope) { | ||
| if (isStyleOnlyChanged(oldResult, newResult)) { | ||
| isStyleOnlyChange = true; | ||
| // All styles are the same, we can skip an HMR update | ||
| const styles = new Set(newResult.css); | ||
| for (const style of oldResult.css) { | ||
| if (styles.has(style)) { | ||
| styles.delete(style); | ||
| } | ||
| } | ||
| if (styles.size === 0) { | ||
| return []; | ||
| } |
There was a problem hiding this comment.
Since scope hash now derives from normalizedFilename only, I created a isStyleOnlyChanged function to handle it. It's a bit tedious than before, but I think leaving the HMR logic out of the compiler could be nice for now.
I also remove the styles.size === 0 handling. The logic isn't quite right given a scenario where:
oldResulthas two<style>s- User removes one
<style> newResultnow has one<style>- This logic makes
styles.size === 0and skips HMR. - BUG: The removed style's side effect isn't removed as no HMR or reload is done.
It seems that this does handle skipping HMR if no source code change, which I handled on line 37 instead.
| @@ -3,7 +3,7 @@ | |||
| "version": "0.0.0", | |||
| "private": true, | |||
| "dependencies": { | |||
| "astro": "^1.0.0", | |||
| "astro": "workspace:*", | |||
There was a problem hiding this comment.
The 1.0.0 was used to bypass the peer dep warning, but I found this test to use the old 1.0 code instead, breaking the compiler upgrade as both 1.0 and 2.0 code exist at the same time.
Changes
filenameandastroGlobalArgsoptionTesting
Existing tests should pass. But I also tested the HMR issue manually to make sure it still works.
Docs
n/a. internal API