Skip to content
This repository was archived by the owner on May 10, 2021. It is now read-only.

Commit 23144d4

Browse files
FinnWoelmFinnWoelm
authored
Fix broken redirects (#45) (#47)
* Add test for optional catch-all route at top-level Add a test for #45 * Fix invalid redirects for optional catch-all route at top-level See: #45 * Fix order of redirects generated by next-on-netlify Use the NextJS code for sorting routes (instead of @sls-next's code): - /node_modules/next/dist/next-server/lib/router/utils/sorted-routes - https://github.com/vercel/next.js/blob/canary/packages/next/ next-server/lib/router/utils/sorted-routes.ts The regex for removing file endings used in /lib/helpers/getSortedRoutes was incorrect and led to incorrect route paths being passed into the sorting function. This has been fixed. * Cleanup: Remove unused require-statements Remove two unused require-statements from /lib/pages/getStaticProps/redirects.js. These are leftover from refactoring.
1 parent 562c84c commit 23144d4

File tree

10 files changed

+69
-48
lines changed

10 files changed

+69
-48
lines changed

‎lib/helpers/getNetlifyRoutes.js

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,23 @@ const getNetlifyRoutes = (nextRoute) => {
1616
// Netlify route for the base path (when no parameters are present).
1717
// The file ending must be present!
1818
if (nextRoute.match(OPTIONAL_CATCH_ALL_REGEX)) {
19-
netlifyRoutes.push(nextRoute.replace(OPTIONAL_CATCH_ALL_REGEX, "$2"));
19+
let netlifyRoute = nextRoute.replace(OPTIONAL_CATCH_ALL_REGEX, "$2");
20+
21+
// When optional catch-all route is at top-level, the regex on line 19 will
22+
// create an empty string, but actually needs to be a forward slash
23+
if (netlifyRoute === "") netlifyRoute = "/";
24+
25+
// When optional catch-all route is at top-level, the regex on line 19 will
26+
// create an incorrect route for the data route. For example, it creates
27+
// /_next/data/%BUILDID%.json, but NextJS looks for
28+
// /_next/data/%BUILDID%/index.json
29+
netlifyRoute = netlifyRoute.replace(
30+
/(\/_next\/data\/[^/]+).json/,
31+
"$1/index.json"
32+
);
33+
34+
// Add second route to the front of the array
35+
netlifyRoutes.unshift(netlifyRoute);
2036
}
2137

2238
// Replace catch-all, e.g., [...slug]

‎lib/helpers/getSortedRoutes.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
const {
2-
getSortedRoutes: getSortedRoutesFromSlsNext,
3-
} = require("@sls-next/lambda-at-edge/dist/lib/sortedRoutes");
2+
getSortedRoutes: getSortedRoutesFromNext,
3+
} = require("next/dist/next-server/lib/router/utils/sorted-routes");
44

55
// Remove the file extension form the route
6-
const removeFileExtension = (route) => route.replace(/\.[A-z]+$/, "");
6+
const removeFileExtension = (route) => route.replace(/\.[a-zA-Z]+$/, "");
77

88
// Return an array of routes sorted in order of specificity, i.e., more generic
99
// routes precede more specific ones
@@ -16,7 +16,7 @@ const getSortedRoutes = (routes) => {
1616
);
1717

1818
// Sort the "naked" routes
19-
const sortedRoutes = getSortedRoutesFromSlsNext(routesWithoutExtensions);
19+
const sortedRoutes = getSortedRoutesFromNext(routesWithoutExtensions);
2020

2121
// Return original routes in the sorted order
2222
return routes.sort(

‎lib/pages/getStaticProps/redirects.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
const { relative } = require("path");
2-
const isDynamicRoute = require("../../helpers/isDynamicRoute");
31
const pages = require("./pages");
42

53
// Pages with getStaticProps do not need redirects, unless they are using

‎tests/__snapshots__/defaults.test.js.snap

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
exports[`Routing creates Netlify redirects 1`] = `
44
"# Next-on-Netlify Redirects
55
/ /.netlify/functions/next_index 200
6-
/_next/data/%BUILD_ID%/getServerSideProps/all/* /.netlify/functions/next_getServerSideProps_all_slug 200
76
/_next/data/%BUILD_ID%/getServerSideProps/all.json /.netlify/functions/next_getServerSideProps_all_slug 200
7+
/_next/data/%BUILD_ID%/getServerSideProps/all/* /.netlify/functions/next_getServerSideProps_all_slug 200
88
/_next/data/%BUILD_ID%/getServerSideProps/static.json /.netlify/functions/next_getServerSideProps_static 200
99
/_next/data/%BUILD_ID%/getServerSideProps/:id.json /.netlify/functions/next_getServerSideProps_id 200
1010
/_next/data/%BUILD_ID%/getStaticProps/with-revalidate.json /.netlify/functions/next_getStaticProps_withrevalidate 200
@@ -13,20 +13,20 @@ exports[`Routing creates Netlify redirects 1`] = `
1313
/_next/data/%BUILD_ID%/getStaticProps/withRevalidate/1.json /.netlify/functions/next_getStaticProps_withRevalidate_id 200
1414
/_next/data/%BUILD_ID%/getStaticProps/withRevalidate/2.json /.netlify/functions/next_getStaticProps_withRevalidate_id 200
1515
/_next/data/%BUILD_ID%/getStaticProps/withRevalidate/withFallback/:id.json /.netlify/functions/next_getStaticProps_withRevalidate_withFallback_id 200
16-
/api/shows/:params/* /.netlify/functions/next_api_shows_params 200
1716
/api/shows/:id /.netlify/functions/next_api_shows_id 200
17+
/api/shows/:params/* /.netlify/functions/next_api_shows_params 200
1818
/api/static /.netlify/functions/next_api_static 200
19-
/getServerSideProps/all/* /.netlify/functions/next_getServerSideProps_all_slug 200
2019
/getServerSideProps/all /.netlify/functions/next_getServerSideProps_all_slug 200
20+
/getServerSideProps/all/* /.netlify/functions/next_getServerSideProps_all_slug 200
2121
/getServerSideProps/static /.netlify/functions/next_getServerSideProps_static 200
2222
/getServerSideProps/:id /.netlify/functions/next_getServerSideProps_id 200
2323
/getStaticProps/with-revalidate /.netlify/functions/next_getStaticProps_withrevalidate 200
24-
/getStaticProps/withFallback/:slug/* /.netlify/functions/next_getStaticProps_withFallback_slug 200
2524
/getStaticProps/withFallback/:id /.netlify/functions/next_getStaticProps_withFallback_id 200
25+
/getStaticProps/withFallback/:slug/* /.netlify/functions/next_getStaticProps_withFallback_slug 200
2626
/getStaticProps/withRevalidate/1 /.netlify/functions/next_getStaticProps_withRevalidate_id 200
2727
/getStaticProps/withRevalidate/2 /.netlify/functions/next_getStaticProps_withRevalidate_id 200
2828
/getStaticProps/withRevalidate/withFallback/:id /.netlify/functions/next_getStaticProps_withRevalidate_withFallback_id 200
29-
/shows/:params/* /.netlify/functions/next_shows_params 200
3029
/shows/:id /.netlify/functions/next_shows_id 200
30+
/shows/:params/* /.netlify/functions/next_shows_params 200
3131
/static/:id /static/[id].html 200"
3232
`;

‎tests/__snapshots__/optionalCatchAll.test.js.snap

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@
22

33
exports[`Routing creates Netlify redirects 1`] = `
44
"# Next-on-Netlify Redirects
5-
/_next/data/%BUILD_ID%/catch/* /.netlify/functions/next_catch_all 200
6-
/_next/data/%BUILD_ID%/catch.json /.netlify/functions/next_catch_all 200
7-
/catch/* /.netlify/functions/next_catch_all 200
8-
/catch /.netlify/functions/next_catch_all 200"
5+
/_next/data/%BUILD_ID%/page.json /.netlify/functions/next_page 200
6+
/_next/data/%BUILD_ID%/index.json /.netlify/functions/next_all 200
7+
/_next/data/%BUILD_ID%/* /.netlify/functions/next_all 200
8+
/page /.netlify/functions/next_page 200
9+
/ /.netlify/functions/next_all 200
10+
/* /.netlify/functions/next_all 200"
911
`;

‎tests/fixtures/next.config.js-with-optionalCatchAll

Lines changed: 0 additions & 6 deletions
This file was deleted.

‎tests/fixtures/pages-with-optionalCatchAll/catch/[[...all]].js renamed to ‎tests/fixtures/pages-with-optionalCatchAll/[[...all]].js

File renamed without changes.

‎tests/fixtures/pages-with-optionalCatchAll/index.js

Lines changed: 0 additions & 25 deletions
This file was deleted.

‎tests/fixtures/pages-with-optionalCatchAll/page.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import Link from "next/link";
2+
3+
const Index = () => (
4+
<div>
5+
<ul>
6+
<li>
7+
<Link href="/">
8+
<a>/</a>
9+
</Link>
10+
</li>
11+
<li>
12+
<Link href="/catch/25/catch/all">
13+
<a>/catch/25/catch/all</a>
14+
</Link>
15+
</li>
16+
<li>
17+
<Link href="/catch/75/undefined/path/test">
18+
<a>/catch/75/undefined/path/test</a>
19+
</Link>
20+
</li>
21+
</ul>
22+
</div>
23+
);
24+
25+
export const getServerSideProps = async ({ params }) => {
26+
const res = await fetch("https://api.tvmaze.com/shows/42");
27+
const data = await res.json();
28+
29+
return {
30+
props: {
31+
show: data,
32+
},
33+
};
34+
};
35+
36+
export default Index;

‎tests/optionalCatchAll.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ beforeAll(
2525
buildOutput = await buildNextApp()
2626
.forTest(__filename)
2727
.withPages("pages-with-optionalCatchAll")
28-
.withNextConfig("next.config.js-with-optionalCatchAll")
28+
.withNextConfig("next.config.js")
2929
.withPackageJson("package.json")
3030
.build();
3131
},

0 commit comments

Comments
 (0)