-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Handle compiler breaking change #5803
Changes from 9 commits
c2a7ada
86e9fb0
82e855b
bb4594e
425bb57
3292c13
892345d
72a8db8
3432dd7
5f1c619
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'astro': patch | ||
--- | ||
|
||
Upgrade compiler and handle breaking changes |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,6 +66,9 @@ | |
"react", | ||
"react-dom", | ||
"@types/react" | ||
], | ||
"allowAny": [ | ||
"astro" | ||
Comment on lines
+70
to
+71
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is added to suppress the peer dep warning of |
||
] | ||
}, | ||
"patchedDependencies": { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,12 @@ | ||
import { fileURLToPath } from 'node:url'; | ||
import type { HmrContext, ModuleNode } from 'vite'; | ||
import type { AstroConfig } from '../@types/astro'; | ||
import { cachedCompilation, invalidateCompilation, isCached } from '../core/compile/index.js'; | ||
import { | ||
cachedCompilation, | ||
CompileResult, | ||
invalidateCompilation, | ||
isCached, | ||
} from '../core/compile/index.js'; | ||
import type { LogOptions } from '../core/logger/core.js'; | ||
import { info } from '../core/logger/core.js'; | ||
import * as msg from '../core/messages.js'; | ||
|
@@ -17,32 +22,24 @@ export interface HandleHotUpdateOptions { | |
config: AstroConfig; | ||
logging: LogOptions; | ||
compile: () => ReturnType<typeof cachedCompilation>; | ||
source: string; | ||
} | ||
|
||
export async function handleHotUpdate( | ||
ctx: HmrContext, | ||
{ config, logging, compile }: HandleHotUpdateOptions | ||
{ config, logging, compile, source }: HandleHotUpdateOptions | ||
) { | ||
let isStyleOnlyChange = false; | ||
if (ctx.file.endsWith('.astro') && isCached(config, ctx.file)) { | ||
// Get the compiled result from the cache | ||
const oldResult = await compile(); | ||
// 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 []; | ||
} | ||
Comment on lines
-30
to
-45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since scope hash now derives from I also remove the
It seems that this does handle skipping HMR if no source code change, which I handled on line 37 instead. |
||
} | ||
} else { | ||
invalidateCompilation(config, ctx.file); | ||
|
@@ -134,3 +131,33 @@ export async function handleHotUpdate( | |
|
||
return mods; | ||
} | ||
|
||
function isStyleOnlyChanged(oldResult: CompileResult, newResult: CompileResult) { | ||
return ( | ||
normalizeCode(oldResult.code) === normalizeCode(newResult.code) && | ||
!isArrayEqual(oldResult.css, newResult.css) | ||
); | ||
} | ||
|
||
const astroStyleImportRE = /import\s*"[^"]+astro&type=style[^"]+";/g; | ||
const sourceMappingUrlRE = /\/\/# sourceMappingURL=[^ ]+$/gm; | ||
|
||
/** | ||
* Remove style-related code and sourcemap from the final astro output so they | ||
* can be compared between non-style code | ||
*/ | ||
function normalizeCode(code: string) { | ||
return code.replace(astroStyleImportRE, '').replace(sourceMappingUrlRE, '').trim(); | ||
} | ||
|
||
function isArrayEqual(a: any[], b: any[]) { | ||
if (a.length !== b.length) { | ||
return false; | ||
} | ||
for (let i = 0; i < a.length; i++) { | ||
if (a[i] !== b[i]) { | ||
return false; | ||
} | ||
} | ||
return true; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ | |
"version": "0.0.0", | ||
"private": true, | ||
"dependencies": { | ||
"astro": "^1.0.0", | ||
"astro": "workspace:*", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
"astro-embed": "^0.1.1" | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have the
svelte-preprocess
dependency anymore, so this can be removed.