Skip to content

Commit 676a963

Browse files
authored
Validate redirects instead of rewriting them (#219)
In order to migrate the legacy custom site to Docusaurus, we added logic to the Docusaurus config to convert redirects to use the Docusaurus format. The logic included some steps to rewrite redirect sources and desinations. This change adjusts the logic to validate redirects instead and throw errors on malformed redirects. This way, it is easier to troubleshoot issues with redirects, since the redirects that a documentation author would see are the same ones that Docusaurus processes. As part of this, this change adds a rule to require a leading slash in redirect paths. When validating redirects, Docusaurus ignores destinations that do not begin with a slash, causing unexpected issues for custom logic in the docs engine that expects redirect paths to begin with slashes. For context, see the following file in facebook/docusaurus#9171: packages/docusaurus-plugin-client-redirects/src/collectRedirects.ts
1 parent 86a7527 commit 676a963

2 files changed

Lines changed: 113 additions & 42 deletions

File tree

server/redirects.test.ts

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import { describe, expect, test } from "@jest/globals";
2+
import { CustomRedirect, validateRedirect } from "./redirects";
3+
4+
describe("validateRedirects", () => {
5+
interface testCase {
6+
description: string;
7+
input: CustomRedirect;
8+
shouldThrow: boolean;
9+
errorSubstring: string;
10+
}
11+
const testCases: Array<testCase> = [
12+
{
13+
description: "incorrect index page source",
14+
input: {
15+
source: "/enroll-resources/applications/applications/",
16+
destination: "/enroll-resources/apps/",
17+
permanent: true,
18+
},
19+
shouldThrow: true,
20+
errorSubstring:
21+
"redirect source includes an incorrect category index page path - remove the final path segment: /enroll-resources/applications/applications/",
22+
},
23+
{
24+
description: "incorrect index page destination",
25+
input: {
26+
source: "/enroll-resources/apps/",
27+
destination: "/enroll-resources/applications/applications/",
28+
permanent: true,
29+
},
30+
shouldThrow: true,
31+
errorSubstring:
32+
"redirect destination includes an incorrect category index page path - remove the final path segment: /enroll-resources/applications/applications/",
33+
},
34+
{
35+
description: "no leading slash in a source",
36+
input: {
37+
source: "enroll-resources/apps/",
38+
destination: "/enroll-resources/applications/applications/",
39+
permanent: true,
40+
},
41+
shouldThrow: true,
42+
errorSubstring:
43+
"redirect source must start with a trailing slash: enroll-resources/apps/",
44+
},
45+
{
46+
description: "no leading slash in a destination",
47+
input: {
48+
source: "/enroll-resources/apps/",
49+
destination: "enroll-resources/applications/applications/",
50+
permanent: true,
51+
},
52+
shouldThrow: true,
53+
errorSubstring:
54+
"redirect destination includes an incorrect category index page path - remove the final path segment: enroll-resources/applications/applications/",
55+
},
56+
{
57+
description: "valid case",
58+
input: {
59+
source: "/enroll-resources/apps/",
60+
destination: "/enroll-resources/applications/",
61+
permanent: true,
62+
},
63+
shouldThrow: false,
64+
errorSubstring: "",
65+
},
66+
];
67+
68+
test.each(testCases)("$description", (c) => {
69+
if (c.shouldThrow) {
70+
expect(() => {
71+
validateRedirect(c.input);
72+
}).toThrow(c.errorSubstring);
73+
} else {
74+
expect(() => {
75+
validateRedirect(c.input);
76+
}).not.toThrow();
77+
}
78+
});
79+
});

server/redirects.ts

Lines changed: 34 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -5,56 +5,48 @@ const versions = getVersionNames();
55

66
// Gather all redirects from all versions and convert them in the Docusaurus format.
77

8+
export interface CustomRedirect {
9+
source: string;
10+
destination: string;
11+
permanent: boolean;
12+
}
13+
14+
const isIndexPage = (urlpath: string): boolean => {
15+
const parts = urlpath.split("/").filter((p) => p !== "");
16+
return (
17+
parts.length >= 2 && parts[parts.length - 1] == parts[parts.length - 2]
18+
);
19+
};
20+
21+
export const validateRedirect = (redirect: CustomRedirect) => {
22+
["source", "destination"].forEach((p) => {
23+
if (isIndexPage(redirect[p])) {
24+
throw new Error(
25+
`redirect ${p} includes an incorrect category index page path - remove the final path segment: ${redirect[p]}`,
26+
);
27+
}
28+
if (!redirect[p].startsWith("/")) {
29+
throw new Error(
30+
`redirect ${p} must start with a trailing slash: ${redirect[p]}`,
31+
);
32+
}
33+
});
34+
35+
return;
36+
};
37+
838
export const getRedirects = () => {
939
const result = versions.flatMap((version) => {
1040
const config = loadDocsConfig(version, ".");
1141

1242
return config.redirects || [];
1343
});
1444

15-
return result.map((redirect) => {
16-
// If a page is an index page for a section, it has the same name as the
17-
// containing subdirectory. Handle this logic to accommodate redirects
18-
// from the previous docs site.
19-
const sourceSegs = redirect.source
20-
.replaceAll(new RegExp("/$", "g"), "")
21-
.split("/");
22-
23-
const destSegs = redirect.destination
24-
.replaceAll(new RegExp("/$", "g"), "")
25-
.split("/");
26-
27-
// The destination is an index page, since the slug matches the containing
28-
// path segment.
29-
const destIndex =
30-
destSegs.length >= 2 &&
31-
destSegs[destSegs.length - 1] == destSegs[destSegs.length - 2];
32-
33-
if (!destIndex) {
34-
return {
35-
from: redirect.source,
36-
to: redirect.destination,
37-
};
38-
}
39-
40-
const docusaurusDest =
41-
destSegs.slice(0, -1).join("/") + "/";
42-
43-
// This redirect maps a Docusaurus-style index page to a previous-site index
44-
// page, so reverse the mapping.
45-
if (redirect.source == docusaurusDest) {
46-
const oldSource = redirect.source;
47-
redirect.source = redirect.destination;
48-
redirect.destination = oldSource;
49-
// This redirect has an old-style index page path as a destination, so
50-
// change it to a Docusaurus-style one.
51-
} else {
52-
redirect.destination = docusaurusDest;
53-
}
54-
45+
result.forEach(validateRedirect);
46+
return result.map((r) => {
5547
return {
56-
from: redirect.source,
57-
to: redirect.destination,
48+
from: r.source,
49+
to: r.destination,
5850
};
5951
});
6052
};

0 commit comments

Comments
 (0)