Skip to content
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

Prerendering corner cases #8070

Merged
merged 15 commits into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 31 additions & 9 deletions packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,10 +286,16 @@ export class App {
const errorRouteData = matchRoute('/' + status, this.#manifestData);
const url = new URL(request.url);
if (errorRouteData) {
if (errorRouteData.prerender && !errorRouteData.route.endsWith(`/${status}`)) {
const statusURL = new URL(`${this.#baseWithoutTrailingSlash}/${status}`, url);
if (errorRouteData.prerender){
const maybeDotHtml = errorRouteData.route.endsWith(`/${status}`) ? '.html' : ''
const statusURL = new URL(`${this.#baseWithoutTrailingSlash}/${status}${maybeDotHtml}`, url);
const response = await fetch(statusURL.toString());
return this.#mergeResponses(response, originalResponse);

// response for /404.html and 500.html is 200, which is not meaningful
// so we create an override
const override = { status }

return this.#mergeResponses(response, originalResponse, override);
}
const mod = await this.#getModuleForRoute(errorRouteData);
try {
Expand All @@ -316,14 +322,30 @@ export class App {
return response;
}

#mergeResponses(newResponse: Response, oldResponse?: Response) {
if (!oldResponse) return newResponse;
const { status, statusText, headers } = oldResponse;
#mergeResponses(newResponse: Response, oldResponse?: Response, override?: { status : 404 | 500 }) {
if (!oldResponse) {
if (override !== undefined) {
return new Response(newResponse.body, {
status: override.status,
statusText: newResponse.statusText,
headers: newResponse.headers
})
}
return newResponse;
}

const { statusText, headers } = oldResponse;

// If the the new response did not have a meaningful status, an override may have been provided
// If the original status was 200 (default), override it with the new status (probably 404 or 500)
// Otherwise, the user set a specific status while rendering and we should respect that one
const status =
override?.status ? override.status :
oldResponse.status === 200 ? newResponse.status :
oldResponse.status

return new Response(newResponse.body, {
// If the original status was 200 (default), override it with the new status (probably 404 or 500)
// Otherwise, the user set a specific status while rendering and we should respect that one
status: status === 200 ? newResponse.status : status,
status,
statusText: status === 200 ? newResponse.statusText : statusText,
headers: new Headers(Array.from(headers)),
});
Expand Down
11 changes: 8 additions & 3 deletions packages/astro/src/core/build/static-build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,12 @@ async function runPostBuildHooks(
async function cleanStaticOutput(opts: StaticBuildOptions, internals: BuildInternals) {
const allStaticFiles = new Set();
for (const pageData of eachPageData(internals)) {
if (pageData.route.prerender)
allStaticFiles.add(internals.pageToBundleMap.get(pageData.moduleSpecifier));
if (pageData.route.prerender) {
const { moduleSpecifier } = pageData;
const pageBundleId = internals.pageToBundleMap.get(moduleSpecifier);
const entryBundleId = internals.entrySpecifierToBundleMap.get(moduleSpecifier);
allStaticFiles.add(pageBundleId ?? entryBundleId);
}
}
const ssr = isServerLikeOutput(opts.settings.config);
const out = ssr
Expand Down Expand Up @@ -337,7 +341,8 @@ async function cleanStaticOutput(opts: StaticBuildOptions, internals: BuildInter
// Replace exports (only prerendered pages) with a noop
let value = 'const noop = () => {};';
for (const e of exports) {
value += `\nexport const ${e.n} = noop;`;
if (e.n === 'default') value += `\n export default noop;`
else value += `\nexport const ${e.n} = noop;`;
}
await fs.promises.writeFile(url, value, { encoding: 'utf8' });
})
Expand Down
3 changes: 1 addition & 2 deletions packages/astro/test/ssr-prerender.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ describe('SSR: prerender', () => {
const app = await fixture.loadTestAdapterApp();
/** @type {Set<string>} */
const assets = app.manifest.assets;
expect(assets.size).to.equal(1);
expect(Array.from(assets)[0].endsWith('static/index.html')).to.be.true;
expect(assets).to.contain('/static/index.html');
});
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
{
"name": "@test/nodejs-prerender-404",
"name": "@test/nodejs-prerender-404-500",
"version": "0.0.0",
"private": true,
"type": "module",
"dependencies": {
"astro": "workspace:*",
"@astrojs/node": "workspace:*"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
body {
background-color: ivory;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// This module is only used by the prerendered 404.astro.
// It exhibits different behavior if it's called more than once,
// which is detected by a test and interpreted as a failure.

let usedOnce = false
let dynamicMessage = "Page was not prerendered"

export default function () {
if (usedOnce === false) {
usedOnce = true
return "Page does not exist"
}

dynamicMessage += "+"

return dynamicMessage
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// This module is only used by the prerendered 500.astro.
// It exhibits different behavior if it's called more than once,
// which is detected by a test and interpreted as a failure.

let usedOnce = false
let dynamicMessage = "Page was not prerendered"

export default function () {
if (usedOnce === false) {
usedOnce = true
return "Something went wrong"
}

dynamicMessage += "+"

return dynamicMessage
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
import message from "../nondeterminism-404"
export const prerender = true;
---
{message()}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
import "../external-stylesheet.css"
import message from "../nondeterminism-500"
export const prerender = true
---
<h1>{message()}</h1>
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
return new Response(null, { status: 500 })
---
<p>This html will not be served</p>

This file was deleted.

3 changes: 0 additions & 3 deletions packages/integrations/node/test/image.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ describe('Image endpoint', () => {
'/_image?href=/_astro/some_penguin.97ef5f92.png&w=50&f=webp'
);

console.log(resImage);
const content = resImage.text();
console.log(content);
expect(resImage.status).to.equal(200);
});
});
Loading
Loading