Skip to content

Commit

Permalink
feat(spas): streamline 404 (#12248)
Browse files Browse the repository at this point in the history
* feat(spas): move 404 out of _spas and into a index.html

This has been a special case, let's streamline it a bit.

* update spa url

* fix 404 path
  • Loading branch information
fiji-flo authored Dec 5, 2024
1 parent aeb1c1d commit 6646831
Show file tree
Hide file tree
Showing 10 changed files with 13 additions and 13 deletions.
4 changes: 2 additions & 2 deletions build/spas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,9 @@ export async function buildSPAs(options: {

// The URL isn't very important as long as it triggers the right route in the <App/>
const locale = DEFAULT_LOCALE;
const url = `/${locale}/404.html`;
const url = `/${locale}/404/index.html`;
const context: HydrationData = { url, pageNotFound: true };
const outPath = path.join(BUILD_OUT_ROOT, locale.toLowerCase(), "_spas");
const outPath = path.join(BUILD_OUT_ROOT, locale.toLowerCase(), "404");
fs.mkdirSync(outPath, { recursive: true });
const jsonFilePath = path.join(
outPath,
Expand Down
2 changes: 1 addition & 1 deletion client/src/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ export function App(appProps: HydrationData) {
to simulate it.
*/}
<Route
path="/_404/*"
path="/404/*"
element={
<StandardLayout>
<PageNotFound />
Expand Down
4 changes: 2 additions & 2 deletions client/src/page-not-found/fallback-link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ export default function FallbackLink({ url }: { url: string }) {
// What if we attempt to see if it would be something there in English?
// We'll use the `index.json` version of the URL
let enUSURL = url.replace(`/${locale}/`, "/en-US/");
// But of the benefit of local development, devs can use `/_404/`
// But of the benefit of local development, devs can use `/404/`
// instead of `/docs/` to simulate getting to the Page not found page.
// So remove that when constructing the English index.json URL.
enUSURL = enUSURL.replace("/_404/", "/docs/");
enUSURL = enUSURL.replace("/en-US/404/", "/en-US/docs/");

// The fallback check URL should not force append index.json so it can
// follow any redirects
Expand Down
2 changes: 1 addition & 1 deletion client/src/page-not-found/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const FallbackLink = React.lazy(() => import("./fallback-link"));

// NOTE! To hack on this component, you have to use a trick to even get to this
// unless you use the Express server on localhost:5042.
// To get here, use http://localhost:3000/en-US/_404/Whatever/you/like
// To get here, use http://localhost:3000/en-US/404/Whatever/you/like
// Now hot-reloading works and you can iterate faster.
// Otherwise, you can use http://localhost:5042/en-US/docs/Whatever/you/like
// (note the :5042 port) and that'll test it a bit more realistically.
Expand Down
2 changes: 1 addition & 1 deletion cloud-function/src/handlers/proxy-content.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { Source, sourceUri } from "../env.js";
import { PROXY_TIMEOUT } from "../constants.js";
import { isLiveSampleURL } from "../utils.js";

const NOT_FOUND_PATH = "en-us/_spas/404.html";
const NOT_FOUND_PATH = "en-us/404/index.html";

let notFoundBuffer: ArrayBuffer;

Expand Down
2 changes: 1 addition & 1 deletion docs/spas.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ To debug the 404 page, in local development you have two choices:

- <http://localhost:5042/en-US/docs/Does/not/exist>

- <http://localhost:3000/en-US/_404/Does/not/exist>
- <http://localhost:3000/en-US/404/Does/not/exist>

The latter is used so you get hot-reloading as you're working on it. This will
only work when you do local development on Yari.
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
"prettier-check": "prettier --check .",
"prettier-format": "prettier --write .",
"render:html": "cross-env NODE_ENV=production NODE_OPTIONS=\"--no-warnings=ExperimentalWarning --loader ts-node/esm\" node build/ssr-cli.ts",
"start": "(test -f client/build/asset-manifest.json || yarn build:client) && (test -f ssr/dist/main.js || yarn build:ssr) && (test -f popularities.json || yarn tool popularities) && (test -d client/build/en-us/_spas || yarn tool spas) && (test -d client/build/en-us/_spas/404.html || yarn render:html -s) && nf -j Procfile.start start",
"start": "(test -f client/build/asset-manifest.json || yarn build:client) && (test -f ssr/dist/main.js || yarn build:ssr) && (test -f popularities.json || yarn tool popularities) && (test -d client/build/en-us/404 || yarn tool spas) && (test -d client/build/en-us/404/index.html || yarn render:html -s) && nf -j Procfile.start start",
"start:client": "cd client && cross-env NODE_ENV=development BABEL_ENV=development PORT=3000 node scripts/start.js",
"start:rari": "(test -f client/build/asset-manifest.json || yarn build:client) && (test -f ssr/dist/main.js || yarn build:ssr) && cross-env RARI=true nf -j Procfile.rari start",
"start:rari-external": "(test -f client/build/asset-manifest.json || yarn build:client) && (test -f ssr/dist/main.js || yarn build:ssr) && cross-env RARI=true nf -j Procfile.start start",
Expand Down
2 changes: 1 addition & 1 deletion server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ async function send404(res: express.Response) {
if (!RARI) {
return res
.status(404)
.sendFile(path.join(STATIC_ROOT, "en-us", "_spas", "404.html"));
.sendFile(path.join(STATIC_ROOT, "en-us", "404", "index.html"));
} else {
try {
const index = await fetch_from_rari("/en-US/404");
Expand Down
2 changes: 1 addition & 1 deletion server/static.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ app.get("/*", async (req, res) => {
console.log(`Don't know how to mock: ${req.path}`, req.query);
res
.status(404)
.sendFile(path.join(BUILD_OUT_ROOT, "en-us", "_spas", "404.html"));
.sendFile(path.join(BUILD_OUT_ROOT, "en-us", "404", "index.html"));
});

const HOST = process.env.SERVER_HOST || undefined;
Expand Down
4 changes: 2 additions & 2 deletions testing/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1173,9 +1173,9 @@ test("images that are in the folder but not in <img> tags", () => {
});

test("404 page", () => {
const builtFolder = path.join(buildRoot, "en-us", "_spas");
const builtFolder = path.join(buildRoot, "en-us");
expect(fs.existsSync(builtFolder)).toBeTruthy();
const htmlFile = path.join(builtFolder, "404.html");
const htmlFile = path.join(builtFolder, "404", "index.html");
const html = fs.readFileSync(htmlFile, "utf-8");
const $ = cheerio.load(html);
expect($("title").text()).toContain("Page not found");
Expand Down

0 comments on commit 6646831

Please sign in to comment.