Skip to content

Commit 1539e04

Browse files
bluwybluwy
authoredJan 17, 2024
Simplify HMR for circular imports and CSS (#9706)
·
create-astro@4.11.1astro@4.2.0
1 parent 6c64b14 commit 1539e04

File tree

12 files changed

+110
-103
lines changed

12 files changed

+110
-103
lines changed
 

‎.changeset/curvy-seas-explain.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@astrojs/mdx": patch
3+
---
4+
5+
Removes redundant HMR handling code

‎.changeset/tidy-planets-whisper.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"astro": patch
3+
---
4+
5+
Simplifies HMR handling, improves circular dependency invalidation, and fixes Astro styles invalidation

‎packages/astro/e2e/fixtures/tailwindcss/astro.config.mjs

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ export default defineConfig({
77
integrations: [
88
tailwind({
99
configFile: fileURLToPath(new URL('./tailwind.config.js', import.meta.url)),
10+
applyBaseStyles: false
1011
}),
1112
],
1213
devToolbar: {

‎packages/astro/e2e/fixtures/tailwindcss/src/components/Layout.astro

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<html lang="en">
2+
<head>
3+
<meta charset="UTF-8" />
4+
<link rel="icon" type="image/x-icon" href="/favicon.ico" />
5+
<title>Astro + TailwindCSS</title>
6+
</head>
7+
<body class="bg-dawn text-midnight">
8+
<slot/>
9+
</body>
10+
</html>
11+
12+
<style is:global>
13+
@tailwind base;
14+
15+
@tailwind utilities;
16+
</style>

‎packages/astro/e2e/fixtures/tailwindcss/src/pages/index.astro

+5-12
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,11 @@
11
---
22
// Component Imports
3+
import Layout from '../components/Layout.astro';
34
import Button from '../components/Button.astro';
45
import Complex from '../components/Complex.astro';
56
---
67

7-
<html lang="en">
8-
<head>
9-
<meta charset="UTF-8" />
10-
<link rel="icon" type="image/x-icon" href="/favicon.ico" />
11-
<title>Astro + TailwindCSS</title>
12-
</head>
13-
14-
<body class="bg-dawn text-midnight">
15-
<Button>I’m a Tailwind Button!</Button>
16-
<Complex />
17-
</body>
18-
</html>
8+
<Layout>
9+
<Button>I’m a Tailwind Button!</Button>
10+
<Complex />
11+
</Layout>

‎packages/astro/src/core/logger/vite.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { isLogLevelEnabled, type Logger as AstroLogger } from './core.js';
66

77
const PKG_PREFIX = fileURLToPath(new URL('../../../', import.meta.url));
88
const E2E_PREFIX = fileURLToPath(new URL('../../../e2e', import.meta.url));
9-
export function isAstroSrcFile(id: string | null) {
9+
function isAstroSrcFile(id: string | null) {
1010
return id?.startsWith(PKG_PREFIX) && !id.startsWith(E2E_PREFIX);
1111
}
1212

‎packages/astro/src/vite-plugin-astro/compile.ts

-5
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,6 @@ export async function cachedFullCompilation({
7676
}
7777
}
7878

79-
// Prefer live reload to HMR in `.astro` files
80-
if (!compileProps.viteConfig.isProduction) {
81-
SUFFIX += `\nif (import.meta.hot) { import.meta.hot.decline() }`;
82-
}
83-
8479
return {
8580
...transformResult,
8681
code: esbuildResult.code + SUFFIX,

‎packages/astro/src/vite-plugin-astro/hmr.ts

+35-60
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,23 @@
1-
import type { HmrContext, ModuleNode } from 'vite';
1+
import path from 'node:path';
2+
import { appendForwardSlash } from '@astrojs/internal-helpers/path';
3+
import type { HmrContext } from 'vite';
24
import type { AstroConfig } from '../@types/astro.js';
35
import type { cachedCompilation } from '../core/compile/index.js';
46
import { invalidateCompilation, isCached, type CompileResult } from '../core/compile/index.js';
57
import type { Logger } from '../core/logger/core.js';
6-
import { isAstroSrcFile } from '../core/logger/vite.js';
7-
import { isAstroScript } from './query.js';
88

99
export interface HandleHotUpdateOptions {
1010
config: AstroConfig;
1111
logger: Logger;
12+
astroFileToCssAstroDeps: Map<string, Set<string>>;
1213

1314
compile: () => ReturnType<typeof cachedCompilation>;
1415
source: string;
1516
}
1617

1718
export async function handleHotUpdate(
1819
ctx: HmrContext,
19-
{ config, logger, compile, source }: HandleHotUpdateOptions
20+
{ config, logger, astroFileToCssAstroDeps, compile, source }: HandleHotUpdateOptions
2021
) {
2122
let isStyleOnlyChange = false;
2223
if (ctx.file.endsWith('.astro') && isCached(config, ctx.file)) {
@@ -34,75 +35,45 @@ export async function handleHotUpdate(
3435
invalidateCompilation(config, ctx.file);
3536
}
3637

37-
// Skip monorepo files to avoid console spam
38-
if (isAstroSrcFile(ctx.file)) {
39-
return;
40-
}
41-
42-
// go through each of these modules importers and invalidate any .astro compilation
43-
// that needs to be rerun.
44-
const filtered = new Set<ModuleNode>(ctx.modules);
45-
const files = new Set<string>();
46-
for (const mod of ctx.modules) {
47-
// Skip monorepo files to avoid console spam
48-
if (isAstroSrcFile(mod.id ?? mod.file)) {
49-
filtered.delete(mod);
50-
continue;
51-
}
52-
53-
if (mod.file && isCached(config, mod.file)) {
54-
filtered.add(mod);
55-
files.add(mod.file);
56-
}
57-
58-
for (const imp of mod.importers) {
59-
if (imp.file && isCached(config, imp.file)) {
60-
filtered.add(imp);
61-
files.add(imp.file);
62-
}
63-
}
64-
}
65-
66-
// Invalidate happens as a separate step because a single .astro file
67-
// produces multiple CSS modules and we want to return all of those.
68-
for (const file of files) {
69-
if (isStyleOnlyChange && file === ctx.file) continue;
70-
invalidateCompilation(config, file);
71-
// If `ctx.file` is depended by an .astro file, e.g. via `this.addWatchFile`,
72-
// Vite doesn't trigger updating that .astro file by default. See:
73-
// https://github.com/vitejs/vite/issues/3216
74-
// For now, we trigger the change manually here.
75-
if (file.endsWith('.astro')) {
76-
ctx.server.moduleGraph.onFileChange(file);
77-
}
78-
}
79-
80-
// Bugfix: sometimes style URLs get normalized and end with `lang.css=`
81-
// These will cause full reloads, so filter them out here
82-
const mods = [...filtered].filter((m) => !m.url.endsWith('='));
83-
84-
// If only styles are changed, remove the component file from the update list
8538
if (isStyleOnlyChange) {
8639
logger.debug('watch', 'style-only change');
8740
// Only return the Astro styles that have changed!
88-
return mods.filter((mod) => mod.id?.includes('astro&type=style'));
41+
return ctx.modules.filter((mod) => mod.id?.includes('astro&type=style'));
8942
}
9043

91-
// Add hoisted scripts so these get invalidated
92-
for (const mod of mods) {
93-
for (const imp of mod.importedModules) {
94-
if (imp.id && isAstroScript(imp.id)) {
95-
mods.push(imp);
44+
// Edge case handling usually caused by Tailwind creating circular dependencies
45+
//
46+
// TODO: we can also workaround this with better CSS dependency management for Astro files,
47+
// so that changes within style tags are scoped to itself. But it'll take a bit of work.
48+
// https://github.com/withastro/astro/issues/9370#issuecomment-1850160421
49+
for (const [astroFile, cssAstroDeps] of astroFileToCssAstroDeps) {
50+
// If the `astroFile` has a CSS dependency on `ctx.file`, there's a good chance this causes a
51+
// circular dependency, which Vite doesn't issue a full page reload. Workaround it by forcing a
52+
// full page reload ourselves. (Vite bug)
53+
// https://github.com/vitejs/vite/pull/15585
54+
if (cssAstroDeps.has(ctx.file)) {
55+
// Mimic the HMR log as if this file is updated
56+
logger.info('watch', getShortName(ctx.file, ctx.server.config.root));
57+
// Invalidate the modules of `astroFile` explicitly as Vite may incorrectly soft-invalidate
58+
// the parent if the parent actually imported `ctx.file`, but `this.addWatchFile` was also called
59+
// on `ctx.file`. Vite should do a hard-invalidation instead. (Vite bug)
60+
const parentModules = ctx.server.moduleGraph.getModulesByFile(astroFile);
61+
if (parentModules) {
62+
for (const mod of parentModules) {
63+
ctx.server.moduleGraph.invalidateModule(mod);
64+
}
9665
}
66+
ctx.server.ws.send({ type: 'full-reload', path: '*' });
9767
}
9868
}
99-
100-
return mods;
10169
}
10270

10371
function isStyleOnlyChanged(oldResult: CompileResult, newResult: CompileResult) {
10472
return (
10573
normalizeCode(oldResult.code) === normalizeCode(newResult.code) &&
74+
// If style tags are added/removed, we need to regenerate the main Astro file
75+
// so that its CSS imports are also added/removed
76+
oldResult.css.length === newResult.css.length &&
10677
!isArrayEqual(oldResult.css, newResult.css)
10778
);
10879
}
@@ -129,3 +100,7 @@ function isArrayEqual(a: any[], b: any[]) {
129100
}
130101
return true;
131102
}
103+
104+
function getShortName(file: string, root: string): string {
105+
return file.startsWith(appendForwardSlash(root)) ? path.posix.relative(root, file) : file;
106+
}

‎packages/astro/src/vite-plugin-astro/index.ts

+40-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
cachedCompilation,
1010
getCachedCompileResult,
1111
type CompileProps,
12+
invalidateCompilation,
1213
} from '../core/compile/index.js';
1314
import { isRelativePath } from '../core/path.js';
1415
import { normalizeFilename } from '../vite-plugin-utils/index.js';
@@ -27,7 +28,13 @@ interface AstroPluginOptions {
2728
export default function astro({ settings, logger }: AstroPluginOptions): vite.Plugin[] {
2829
const { config } = settings;
2930
let resolvedConfig: vite.ResolvedConfig;
30-
let server: vite.ViteDevServer;
31+
let server: vite.ViteDevServer | undefined;
32+
// Tailwind styles could register Astro files as dependencies of other Astro files,
33+
// causing circular imports which trips Vite's HMR. This set is passed to `handleHotUpdate`
34+
// to force a page reload when these dependency files are updated
35+
// NOTE: We need to initialize a map here and in `buildStart` because our unit tests don't
36+
// call `buildStart` (test bug)
37+
let astroFileToCssAstroDeps = new Map<string, Set<string>>();
3138

3239
// Variables for determining if an id starts with /src...
3340
const srcRootWeb = config.srcDir.pathname.slice(config.root.pathname.length - 1);
@@ -42,6 +49,9 @@ export default function astro({ settings, logger }: AstroPluginOptions): vite.Pl
4249
configureServer(_server) {
4350
server = _server;
4451
},
52+
buildStart() {
53+
astroFileToCssAstroDeps = new Map();
54+
},
4555
async load(id, opts) {
4656
const parsedId = parseAstroRequest(id);
4757
const query = parsedId.query;
@@ -157,11 +167,39 @@ export default function astro({ settings, logger }: AstroPluginOptions): vite.Pl
157167
source,
158168
};
159169

170+
// We invalidate and then compile again as we know Vite will only call this `transform`
171+
// when its cache is invalidated.
172+
// TODO: Do the compilation directly and remove our cache so we rely on Vite only.
173+
invalidateCompilation(config, compileProps.filename);
160174
const transformResult = await cachedFullCompilation({ compileProps, logger });
161175

176+
// Register dependencies of this module
177+
const astroDeps = new Set<string>();
162178
for (const dep of transformResult.cssDeps) {
179+
if (dep.endsWith('.astro')) {
180+
astroDeps.add(dep);
181+
}
163182
this.addWatchFile(dep);
164183
}
184+
astroFileToCssAstroDeps.set(id, astroDeps);
185+
186+
// When a dependency from the styles are updated, the dep and Astro module will get invalidated.
187+
// However, the Astro style virtual module is not invalidated because we didn't register that the virtual
188+
// module has that dependency. We currently can't do that either because of a Vite bug.
189+
// https://github.com/vitejs/vite/pull/15608
190+
// Here we manually invalidate the virtual modules ourselves when we're compiling the Astro module.
191+
// When that bug is resolved, we can add the dependencies to the virtual module directly and remove this.
192+
if (server) {
193+
const mods = server.moduleGraph.getModulesByFile(compileProps.filename);
194+
if (mods) {
195+
const seen = new Set(mods);
196+
for (const mod of mods) {
197+
if (mod.url.includes('?astro')) {
198+
server.moduleGraph.invalidateModule(mod, seen);
199+
}
200+
}
201+
}
202+
}
165203

166204
const astroMetadata: AstroPluginMetadata['astro'] = {
167205
clientOnlyComponents: transformResult.clientOnlyComponents,
@@ -200,6 +238,7 @@ export default function astro({ settings, logger }: AstroPluginOptions): vite.Pl
200238
return handleHotUpdate(context, {
201239
config,
202240
logger,
241+
astroFileToCssAstroDeps,
203242
compile,
204243
source,
205244
});

‎packages/astro/src/vite-plugin-astro/query.ts

-5
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,3 @@ export function parseAstroRequest(id: string): ParsedRequestResult {
3535
query,
3636
};
3737
}
38-
39-
export function isAstroScript(id: string): boolean {
40-
const parsed = parseAstroRequest(id);
41-
return parsed.query.type === 'script';
42-
}

‎packages/astro/test/units/vite-plugin-astro/compile.test.js

-5
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,6 @@ const name = 'world
6262
expect(result).to.be.undefined;
6363
});
6464

65-
it('injects hmr code', async () => {
66-
const result = await compile(`<h1>Hello World</h1>`, '/src/components/index.astro');
67-
expect(result.code).to.include('import.meta.hot');
68-
});
69-
7065
it('has file and url exports for markdwon compat', async () => {
7166
const result = await compile(`<h1>Hello World</h1>`, '/src/components/index.astro');
7267
await init;

‎packages/integrations/mdx/src/index.ts

+2-14
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,8 @@ export default function mdx(partialMdxOptions: Partial<MdxOptions> = {}): AstroI
4040
name: '@astrojs/mdx',
4141
hooks: {
4242
'astro:config:setup': async (params) => {
43-
const {
44-
updateConfig,
45-
config,
46-
addPageExtension,
47-
addContentEntryType,
48-
command,
49-
addRenderer,
50-
} = params as SetupHookParams;
43+
const { updateConfig, config, addPageExtension, addContentEntryType, addRenderer } =
44+
params as SetupHookParams;
5145

5246
addRenderer(astroJSXRenderer);
5347
addPageExtension('.mdx');
@@ -209,12 +203,6 @@ export default function mdx(partialMdxOptions: Partial<MdxOptions> = {}): AstroI
209203
code += `\nContent[Symbol.for('astro.needsHeadRendering')] = !Boolean(frontmatter.layout);`;
210204
code += `\nContent.moduleId = ${JSON.stringify(id)};`;
211205

212-
if (command === 'dev') {
213-
// TODO: decline HMR updates until we have a stable approach
214-
code += `\nif (import.meta.hot) {
215-
import.meta.hot.decline();
216-
}`;
217-
}
218206
return { code, map: null };
219207
},
220208
},

0 commit comments

Comments
 (0)
Please sign in to comment.