Skip to content

Commit

Permalink
Prevent overcaching of astro components for HMR (#5293)
Browse files Browse the repository at this point in the history
* Prevent overcaching of astro components for HMR

* Get tests working in windows

* Use the right drive

* Adding a changeset
  • Loading branch information
matthewp authored Nov 3, 2022
1 parent e7dc8b9 commit 4af4d8f
Show file tree
Hide file tree
Showing 6 changed files with 148 additions and 19 deletions.
5 changes: 5 additions & 0 deletions .changeset/heavy-bananas-deny.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Prevent overcaching .astro HMR changes
2 changes: 1 addition & 1 deletion packages/astro/src/core/create-vite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export async function createVite(
},
plugins: [
configAliasVitePlugin({ settings }),
astroLoadFallbackPlugin({ fs, settings }),
astroVitePlugin({ settings, logging }),
astroScriptsPlugin({ settings }),
// The server plugin is for dev only and having it run during the build causes
Expand All @@ -110,7 +111,6 @@ export async function createVite(
astroPostprocessVitePlugin({ settings }),
astroIntegrationsContainerPlugin({ settings, logging }),
astroScriptsPageSSRPlugin({ settings }),
astroLoadFallbackPlugin({ fs }),
],
publicDir: fileURLToPath(settings.config.publicDir),
root: fileURLToPath(settings.config.root),
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/vite-plugin-astro/hmr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export async function handleHotUpdate(
{ config, logging, compile }: HandleHotUpdateOptions
) {
let isStyleOnlyChange = false;
if (ctx.file.endsWith('.astro')) {
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
Expand Down
55 changes: 43 additions & 12 deletions packages/astro/src/vite-plugin-load-fallback/index.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,65 @@
import nodeFs from 'fs';
import type { AstroSettings } from '../@types/astro';
import type * as vite from 'vite';
import nodeFs from 'fs';
import npath from 'path';


type NodeFileSystemModule = typeof nodeFs;

export interface LoadFallbackPluginParams {
fs?: NodeFileSystemModule;
settings: AstroSettings;
}

export default function loadFallbackPlugin({ fs }: LoadFallbackPluginParams): vite.Plugin | false {
export default function loadFallbackPlugin({ fs, settings }: LoadFallbackPluginParams): vite.Plugin[] | false {
// Only add this plugin if a custom fs implementation is provided.
if (!fs || fs === nodeFs) {
return false;
}

return {
name: 'astro:load-fallback',
enforce: 'post',
async load(id) {
const tryLoadModule = async (id: string) => {
try {
// await is necessary for the catch
return await fs.promises.readFile(cleanUrl(id), 'utf-8');
} catch (e) {
try {
// await is necessary for the catch
return await fs.promises.readFile(cleanUrl(id), 'utf-8');
} catch (e) {
return await fs.promises.readFile(id, 'utf-8');
} catch (e2) {
try {
return await fs.promises.readFile(id, 'utf-8');
} catch (e2) {
const fullpath = new URL('.' + id, settings.config.root);
return await fs.promises.readFile(fullpath, 'utf-8');
} catch (e3) {
// Let fall through to the next
}
}
},
}
};

return [{
name: 'astro:load-fallback',
enforce: 'post',
resolveId(id, parent) {
if(id.startsWith('.') && parent && fs.existsSync(parent)) {
return npath.posix.join(npath.posix.dirname(parent), id);
}
},
async load(id) {
const source = await tryLoadModule(id);
return source;
}
}, {
name: 'astro:load-fallback-hmr',
enforce: 'pre',
handleHotUpdate(context) {
// Wrap context.read so it checks our filesystem first.
const read = context.read;
context.read = async () => {
const source = await tryLoadModule(context.file);
if(source) return source;
return read.call(context);
};
}
}];
}

const queryRE = /\?.*$/s;
Expand Down
65 changes: 64 additions & 1 deletion packages/astro/test/units/dev/dev.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { expect } from 'chai';
import * as cheerio from 'cheerio';

import { runInContainer } from '../../../dist/core/dev/index.js';
import { createFs, createRequestAndResponse } from '../test-utils.js';
import { createFs, createRequestAndResponse, triggerFSEvent } from '../test-utils.js';

const root = new URL('../../fixtures/alias/', import.meta.url);

Expand Down Expand Up @@ -37,4 +37,67 @@ describe('dev container', () => {
expect($('h1')).to.have.a.lengthOf(1);
});
});

it('HMR only short circuits on previously cached modules', async () => {
const fs = createFs(
{
'/src/components/Header.astro': `
<h1>{Astro.props.title}</h1>
`,
'/src/pages/index.astro': `
---
import Header from '../components/Header.astro';
const name = 'Testing';
---
<html>
<head><title>{name}</title></head>
<body class="one">
<Header title={name} />
</body>
</html>
`,
},
root
);

await runInContainer({ fs, root }, async (container) => {
let r = createRequestAndResponse({
method: 'GET',
url: '/',
});
container.handle(r.req, r.res);
let html = await r.text();
let $ = cheerio.load(html);
expect($('body.one')).to.have.a.lengthOf(1);

fs.writeFileFromRootSync('/src/components/Header.astro', `
<h1>{Astro.props.title}</h1>
`);
triggerFSEvent(container, fs, '/src/components/Header.astro', 'change');

fs.writeFileFromRootSync('/src/pages/index.astro', `
---
import Header from '../components/Header.astro';
const name = 'Testing';
---
<html>
<head><title>{name}</title></head>
<body class="two">
<Header title={name} />
</body>
</html>
`);
triggerFSEvent(container, fs, '/src/pages/index.astro', 'change');

r = createRequestAndResponse({
method: 'GET',
url: '/',
});
container.handle(r.req, r.res);
html = await r.text();
$ = cheerio.load(html);
expect($('body.one')).to.have.a.lengthOf(0);
expect($('body.two')).to.have.a.lengthOf(1);
});
});
});
38 changes: 34 additions & 4 deletions packages/astro/test/units/test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,23 @@ class MyVolume extends Volume {
this.#root = root;
}

#forcePath(p) {
if (p instanceof URL) {
p = unixify(fileURLToPath(p));
}
return p;
}

getFullyResolvedPath(pth) {
return npath.posix.join(this.#root, pth);
}

existsSync(p) {
if (p instanceof URL) {
p = fileURLToPath(p);
}
return super.existsSync(p);
return super.existsSync(this.#forcePath(p));
}

readFile(p, ...args) {
return super.readFile(this.#forcePath(p), ...args);
}

writeFileFromRootSync(pth, ...rest) {
Expand All @@ -44,6 +52,28 @@ export function createFs(json, root) {
return fs;
}

/**
*
* @param {import('../../src/core/dev/container').Container} container
* @param {typeof import('fs')} fs
* @param {string} shortPath
* @param {'change'} eventType
*/
export function triggerFSEvent(container, fs, shortPath, eventType) {
container.viteServer.watcher.emit(
eventType,
fs.getFullyResolvedPath(shortPath)
);

if(!fileURLToPath(container.settings.config.root).startsWith('/')) {
const drive = fileURLToPath(container.settings.config.root).slice(0, 2);
container.viteServer.watcher.emit(
eventType,
drive + fs.getFullyResolvedPath(shortPath)
);
}
}

export function createRequestAndResponse(reqOptions = {}) {
const req = httpMocks.createRequest(reqOptions);

Expand Down

0 comments on commit 4af4d8f

Please sign in to comment.