Skip to content

Commit

Permalink
fix: do redirect to locale and trailingSlash only if the route exist
Browse files Browse the repository at this point in the history
  • Loading branch information
aralroca committed Oct 4, 2023
1 parent 040e6fe commit 392f947
Show file tree
Hide file tree
Showing 3 changed files with 146 additions and 19 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"module": "./out/core/index.js",
"main": "./out/core/index.js",
"types": "./out/core/index.d.ts",
"version": "0.0.3",
"version": "0.0.4",
"description": "lightweight and flexible front-end library based on Bun.js for modern web apps",
"license": "MIT",
"type": "module",
Expand Down
98 changes: 98 additions & 0 deletions src/cli/serve/serve-options.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,54 @@ describe("CLI: serve", () => {
globalThis.mockConstants = undefined;
});

it("should return 404 page without redirect to the locale if the page doesn't exist", async () => {
const response = await testRequest(
new Request("http://localhost:1234/not-found-page"),
);
const html = await response.text();

expect(response.status).toBe(404);
expect(html).toContain('<title id="title">Page not found</title>');
expect(html).not.toContain('<title id="title">CUSTOM LAYOUT</title>');
expect(html).toContain("<h1>Page not found 404</h1>");
});

it("should return 404 page without redirect to the trailingSlash if the page doesn't exist", async () => {
globalThis.mockConstants = {
...globalThis.mockConstants,
CONFIG: {
trailingSlash: true,
},
};
const response = await testRequest(
new Request("http://localhost:1234/es/not-found-page"),
);
const html = await response.text();

expect(response.status).toBe(404);
expect(html).toContain('<title id="title">Page not found</title>');
expect(html).not.toContain('<title id="title">CUSTOM LAYOUT</title>');
expect(html).toContain("<h1>Page not found 404</h1>");
});

it("should return 404 page without redirect to the locale and trailingSlash if the page doesn't exist", async () => {
globalThis.mockConstants = {
...globalThis.mockConstants,
CONFIG: {
trailingSlash: true,
},
};
const response = await testRequest(
new Request("http://localhost:1234/not-found-page"),
);
const html = await response.text();

expect(response.status).toBe(404);
expect(html).toContain('<title id="title">Page not found</title>');
expect(html).not.toContain('<title id="title">CUSTOM LAYOUT</title>');
expect(html).toContain("<h1>Page not found 404</h1>");
});

it("should return 404 page", async () => {
const response = await testRequest(
new Request("http://localhost:1234/es/not-found-page"),
Expand All @@ -49,6 +97,30 @@ describe("CLI: serve", () => {
expect(html).toContain("<h1>Page not found 404</h1>");
});

it("should redirect to the correct locale", async () => {
const response = await testRequest(
new Request(`http://localhost:1234/somepage`),
);
expect(response.status).toBe(301);
expect(response.headers.get("Location")).toBe("/es/somepage");
});

it("should redirect with trailingSlash", async () => {
globalThis.mockConstants = {
...globalThis.mockConstants,
CONFIG: {
trailingSlash: true,
},
};
const response = await testRequest(
new Request(`http://localhost:1234/es/somepage`),
);
expect(response.status).toBe(301);
expect(response.headers.get("Location")).toBe(
"http://localhost:1234/es/somepage/",
);
});

it("should return a page with layout and i18n", async () => {
const response = await testRequest(
new Request(`http://localhost:1234/es/somepage`),
Expand Down Expand Up @@ -87,4 +159,30 @@ describe("CLI: serve", () => {
expect(response.status).toBe(200);
expect(json).toEqual({ name: "Brisa", email: "[email protected]" });
});

it("should return 404 page if the api route does not exist", async () => {
const response = await testRequest(
new Request(`http:///localhost:1234/es/api/not-found`),
);
const html = await response.text();

expect(response.status).toBe(404);
expect(html).toContain('<title id="title">Page not found</title>');
expect(html).not.toContain('<title id="title">CUSTOM LAYOUT</title>');
expect(html).toContain("<h1>Page not found 404</h1>");
});

it("should return 404 page if the api route exist but the method does not", async () => {
const response = await testRequest(
new Request(`http:///localhost:1234/es/api/example`, {
method: "PUT",
}),
);
const html = await response.text();

expect(response.status).toBe(404);
expect(html).toContain('<title id="title">Page not found</title>');
expect(html).not.toContain('<title id="title">CUSTOM LAYOUT</title>');
expect(html).toContain("<h1>Page not found 404</h1>");
});
});
65 changes: 47 additions & 18 deletions src/cli/serve/serve-options.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,21 @@ import fs from "node:fs";
import path from "node:path";

import LoadLayout from "../../utils/load-layout";
import getRouteMatcher from "../../utils/get-route-matcher";
import { renderToReadableStream } from "../../core";
import { LiveReloadScript } from "../dev-live-reload";
import { MatchedRoute, Serve, Server, ServerWebSocket } from "bun";
import importFileIfExists from "../../utils/import-file-if-exists";
import extendRequestContext from "../../utils/extend-request-context";
import getConstants from "../../constants";
import getImportableFilepath from "../../utils/get-importable-filepath";
import getRouteMatcher from "../../utils/get-route-matcher";
import handleI18n from "../../utils/handle-i18n";
import importFileIfExists from "../../utils/import-file-if-exists";
import redirectTrailingSlash from "../../utils/redirect-trailing-slash";
import getImportableFilepath from "../../utils/get-importable-filepath";
import extendRequestContext from "../../utils/extend-request-context";
import { LiveReloadScript } from "../dev-live-reload";
import { MatchedRoute, Serve, ServerWebSocket } from "bun";
import { RequestContext } from "../../types";
import { renderToReadableStream } from "../../core";

declare global {
var ws: ServerWebSocket<unknown> | undefined;
}

const {
IS_PRODUCTION,
Expand All @@ -25,17 +29,14 @@ const {
ASSETS_DIR,
} = getConstants();

let pagesRouter = getRouteMatcher(PAGES_DIR, RESERVED_PAGES);
let rootRouter = getRouteMatcher(ROOT_DIR);

const WEBSOCKET_PATH = getImportableFilepath("websocket", ROOT_DIR);
const wsModule = WEBSOCKET_PATH ? await import(WEBSOCKET_PATH) : null;

declare global {
var ws: ServerWebSocket<unknown> | undefined;
}

const route404 = pagesRouter.reservedRoutes[PAGE_404];
const middlewareModule = await importFileIfExists("middleware", ROOT_DIR);
const customMiddleware = middlewareModule?.default;
let pagesRouter = getRouteMatcher(PAGES_DIR, RESERVED_PAGES);
let rootRouter = getRouteMatcher(ROOT_DIR);

const responseInitWithGzip = {
headers: {
Expand All @@ -57,15 +58,45 @@ export const serveOptions: Serve = {
const isAnAsset = !isHome && fs.existsSync(assetPath);
const i18nRes = isAnAsset ? {} : handleI18n(request);

if (i18nRes.response) return i18nRes.response;
const isResponseLocationValidRoute = (response: Response) => {
const location = response.headers.get("Location") ?? "";

url.pathname = URL.canParse(location)
? new URL(location).pathname
: location;

const req = extendRequestContext({
originalRequest: request,
finalURL: url.toString(),
});

return pagesRouter.match(req).route || rootRouter.match(req).route;
};

// 404 page
const return404Error = () =>
route404
? responseRenderedPage({ req: request, route: route404, status: 404 })
: new Response("Not found", { status: 404 });

if (i18nRes.response) {
// Redirect to the locale
if (isResponseLocationValidRoute(i18nRes.response))
return i18nRes.response;

return return404Error();
}

if (i18nRes.pagesRouter && i18nRes.rootRouter) {
pagesRouter = i18nRes.pagesRouter;
rootRouter = i18nRes.rootRouter;
}

if (!isAnAsset) {
const redirect = redirectTrailingSlash(request);
if (redirect) return redirect;

if (redirect && isResponseLocationValidRoute(redirect)) return redirect;
if (redirect) return return404Error();
}

request.getIP = () => server.requestIP(req);
Expand Down Expand Up @@ -152,8 +183,6 @@ async function handleRequest(req: RequestContext, isAnAsset: boolean) {
}

// 404 page
const route404 = pagesRouter.reservedRoutes[PAGE_404];

return route404
? responseRenderedPage({ req, route: route404, status: 404 })
: new Response("Not found", { status: 404 });
Expand Down

0 comments on commit 392f947

Please sign in to comment.