Skip to content

Commit

Permalink
fix: serve internal JS from path containing build ID (#2753)
Browse files Browse the repository at this point in the history
Previously imported chunks were not cached at all. They are now cached
properly with immutable cache headers.
  • Loading branch information
lucacasonato authored Nov 8, 2024
1 parent 96e520a commit 6b685dd
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 27 deletions.
6 changes: 3 additions & 3 deletions src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
setRenderState,
} from "./runtime/server/preact_hooks.tsx";
import { DEV_ERROR_OVERLAY_URL } from "./constants.ts";
import { asset } from "./runtime/shared.ts";
import { BUILD_ID } from "./runtime/build_id.ts";

export interface Island {
file: string | URL;
Expand Down Expand Up @@ -240,13 +240,13 @@ function preactRender<State, Data>(
} finally {
// Add preload headers
const basePath = ctx.config.basePath;
const runtimeUrl = asset(`${basePath}/fresh-runtime.js`);
const runtimeUrl = `${basePath}/_fresh/js/${BUILD_ID}/fresh-runtime.js`;
let link = `<${encodeURI(runtimeUrl)}>; rel="modulepreload"; as="script"`;
state.islands.forEach((island) => {
const chunk = buildCache.getIslandChunkName(island.name);
if (chunk !== null) {
link += `, <${
encodeURI(asset(`${basePath}${chunk}`))
encodeURI(`${basePath}${chunk}`)
}>; rel="modulepreload"; as="script"`;
}
});
Expand Down
6 changes: 4 additions & 2 deletions src/dev/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,11 @@ export class Builder implements FreshBuilder {
denoJsonPath: denoJson.filePath,
});

const prefix = `/_fresh/js/${BUILD_ID}/`;

for (let i = 0; i < output.files.length; i++) {
const file = output.files[i];
const pathname = `/${file.path}`;
const pathname = `${prefix}${file.path}`;
await buildCache.addProcessedFile(pathname, file.contents, file.hash);
}

Expand All @@ -196,7 +198,7 @@ export class Builder implements FreshBuilder {
`Missing chunk for ${island.file}#${island.exportName}`,
);
}
buildCache.islands.set(island.name, `/${chunk}`);
buildCache.islands.set(island.name, `${prefix}${chunk}`);
}

await buildCache.flush();
Expand Down
11 changes: 10 additions & 1 deletion src/dev/builder_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as path from "@std/path";
import { Builder } from "./builder.ts";
import { App } from "../app.ts";
import { RemoteIsland } from "@marvinh-test/fresh-island";
import { BUILD_ID } from "../runtime/build_id.ts";

Deno.test({
name: "Builder - chain onTransformStaticFile",
Expand Down Expand Up @@ -114,7 +115,15 @@ Deno.test({
await builder.build(app);

const code = await Deno.readTextFile(
path.join(tmp, "dist", "static", "RemoteIsland.js"),
path.join(
tmp,
"dist",
"static",
"_fresh",
"js",
BUILD_ID,
"RemoteIsland.js",
),
);
expect(code).toContain('"remote-island"');
},
Expand Down
36 changes: 20 additions & 16 deletions src/middlewares/static_files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,26 +61,30 @@ export function staticFiles<T>(): MiddlewareFn<T> {
vary: "If-None-Match",
});

if (cacheKey === null || ctx.config.mode === "development") {
const ifNoneMatch = req.headers.get("If-None-Match");
if (
ifNoneMatch !== null &&
(ifNoneMatch === etag || ifNoneMatch === `W/"${etag}"`)
) {
file.close();
return new Response(null, { status: 304, headers });
} else if (etag !== null) {
headers.set("Etag", `W/"${etag}"`);
}

if (
ctx.config.mode !== "development" &&
(BUILD_ID === cacheKey ||
url.pathname.startsWith(
`${ctx.config.basePath}/_fresh/js/${BUILD_ID}/`,
))
) {
headers.append("Cache-Control", "public, max-age=31536000, immutable");
} else {
headers.append(
"Cache-Control",
"no-cache, no-store, max-age=0, must-revalidate",
);
} else {
const ifNoneMatch = req.headers.get("If-None-Match");
if (
ifNoneMatch !== null &&
(ifNoneMatch === etag || ifNoneMatch === `W/"${etag}"`)
) {
file.close();
return new Response(null, { status: 304, headers });
} else if (etag !== null) {
headers.set("Etag", `W/"${etag}"`);
}
}

if (BUILD_ID === cacheKey) {
headers.append("Cache-Control", "public, max-age=31536000, immutable");
}

headers.set("Content-Length", String(file.size));
Expand Down
8 changes: 4 additions & 4 deletions src/runtime/server/preact_hooks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
import type { Signal } from "@preact/signals";
import type { Stringifiers } from "../../jsonify/stringify.ts";
import type { PageProps } from "../../context.ts";
import { asset, Partial, type PartialProps } from "../shared.ts";
import { Partial, type PartialProps } from "../shared.ts";
import { stringify } from "../../jsonify/stringify.ts";
import type { ServerIslandRegistry } from "../../context.ts";
import type { Island } from "../../context.ts";
Expand Down Expand Up @@ -438,7 +438,7 @@ function FreshRuntimeScript() {
}
return {
exportName: island.exportName,
chunk: asset(chunk),
chunk,
name: island.name,
};
});
Expand Down Expand Up @@ -467,7 +467,7 @@ function FreshRuntimeScript() {
const named = island.exportName === "default"
? island.name
: `{ ${island.exportName} }`;
return `import ${named} from "${asset(`${basePath}${chunk}`)}";`;
return `import ${named} from "${`${basePath}${chunk}`}";`;
}).join("");

const islandObj = "{" + islandArr.map((island) => island.name)
Expand All @@ -478,7 +478,7 @@ function FreshRuntimeScript() {
stringify(islandProps, stringifiers),
);

const runtimeUrl = asset(`${basePath}/fresh-runtime.js`);
const runtimeUrl = `${basePath}/_fresh/js/${BUILD_ID}/fresh-runtime.js`;
const scriptContent =
`import { boot } from "${runtimeUrl}";${islandImports}boot(${islandObj},${serializedProps});`;

Expand Down
2 changes: 1 addition & 1 deletion tests/islands_test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ Deno.test({

const link = res.headers.get("Link");
expect(link).toMatch(
/<\/fresh-runtime\.js\?__frsh_c=[^>]+>; rel="modulepreload"; as="script", <\/SelfCounter\.js\?__frsh_c=[^>]+>; rel="modulepreload"; as="script"/,
/<\/_fresh\/js\/[a-zA-Z0-9]+\/fresh-runtime\.js>; rel="modulepreload"; as="script", <\/_fresh\/js\/[a-zA-Z0-9]+\/SelfCounter\.js>; rel="modulepreload"; as="script"/,
);
},
sanitizeResources: false,
Expand Down

0 comments on commit 6b685dd

Please sign in to comment.