From 017417cd4f2c943d448e842ad4fe934ad41ad8d7 Mon Sep 17 00:00:00 2001 From: Finn Woelm Date: Fri, 2 Oct 2020 20:40:22 +0200 Subject: [PATCH 1/4] Add test for optional catch-all route at top-level Add a test for https://github.com/netlify/next-on-netlify/issues/45 --- .../optionalCatchAll.test.js.snap | 10 +++--- .../next.config.js-with-optionalCatchAll | 6 ---- .../{catch => }/[[...all]].js | 0 .../pages-with-optionalCatchAll/index.js | 25 ------------- .../pages-with-optionalCatchAll/page.js | 36 +++++++++++++++++++ tests/optionalCatchAll.test.js | 2 +- 6 files changed, 43 insertions(+), 36 deletions(-) delete mode 100644 tests/fixtures/next.config.js-with-optionalCatchAll rename tests/fixtures/pages-with-optionalCatchAll/{catch => }/[[...all]].js (100%) delete mode 100644 tests/fixtures/pages-with-optionalCatchAll/index.js create mode 100644 tests/fixtures/pages-with-optionalCatchAll/page.js diff --git a/tests/__snapshots__/optionalCatchAll.test.js.snap b/tests/__snapshots__/optionalCatchAll.test.js.snap index 8b854c3..b9d3889 100644 --- a/tests/__snapshots__/optionalCatchAll.test.js.snap +++ b/tests/__snapshots__/optionalCatchAll.test.js.snap @@ -2,8 +2,10 @@ exports[`Routing creates Netlify redirects 1`] = ` "# Next-on-Netlify Redirects -/_next/data/%BUILD_ID%/catch/* /.netlify/functions/next_catch_all 200 -/_next/data/%BUILD_ID%/catch.json /.netlify/functions/next_catch_all 200 -/catch/* /.netlify/functions/next_catch_all 200 -/catch /.netlify/functions/next_catch_all 200" +/* /.netlify/functions/next_all 200 + /.netlify/functions/next_all 200 +/_next/data/%BUILD_ID%/page.json /.netlify/functions/next_page 200 +/_next/data/%BUILD_ID%/* /.netlify/functions/next_all 200 +/_next/data/%BUILD_ID%/.netlify/functions/next_all 200 +/page /.netlify/functions/next_page 200" `; diff --git a/tests/fixtures/next.config.js-with-optionalCatchAll b/tests/fixtures/next.config.js-with-optionalCatchAll deleted file mode 100644 index ce6ef80..0000000 --- a/tests/fixtures/next.config.js-with-optionalCatchAll +++ /dev/null @@ -1,6 +0,0 @@ -module.exports = { - target: 'serverless', - experimental: { - optionalCatchAll: true - } -}; diff --git a/tests/fixtures/pages-with-optionalCatchAll/catch/[[...all]].js b/tests/fixtures/pages-with-optionalCatchAll/[[...all]].js similarity index 100% rename from tests/fixtures/pages-with-optionalCatchAll/catch/[[...all]].js rename to tests/fixtures/pages-with-optionalCatchAll/[[...all]].js diff --git a/tests/fixtures/pages-with-optionalCatchAll/index.js b/tests/fixtures/pages-with-optionalCatchAll/index.js deleted file mode 100644 index 4030a5a..0000000 --- a/tests/fixtures/pages-with-optionalCatchAll/index.js +++ /dev/null @@ -1,25 +0,0 @@ -import Link from "next/link"; - -const Index = () => ( -
- -
-); - -export default Index; diff --git a/tests/fixtures/pages-with-optionalCatchAll/page.js b/tests/fixtures/pages-with-optionalCatchAll/page.js new file mode 100644 index 0000000..d32f6cd --- /dev/null +++ b/tests/fixtures/pages-with-optionalCatchAll/page.js @@ -0,0 +1,36 @@ +import Link from "next/link"; + +const Index = () => ( +
+ +
+); + +export const getServerSideProps = async ({ params }) => { + const res = await fetch("https://api.tvmaze.com/shows/42"); + const data = await res.json(); + + return { + props: { + show: data, + }, + }; +}; + +export default Index; diff --git a/tests/optionalCatchAll.test.js b/tests/optionalCatchAll.test.js index da89b8b..fcd44b7 100644 --- a/tests/optionalCatchAll.test.js +++ b/tests/optionalCatchAll.test.js @@ -25,7 +25,7 @@ beforeAll( buildOutput = await buildNextApp() .forTest(__filename) .withPages("pages-with-optionalCatchAll") - .withNextConfig("next.config.js-with-optionalCatchAll") + .withNextConfig("next.config.js") .withPackageJson("package.json") .build(); }, From fd5d8efd0f1115c5873e59d321c0d0ec773b7304 Mon Sep 17 00:00:00 2001 From: Finn Woelm Date: Fri, 2 Oct 2020 21:37:25 +0200 Subject: [PATCH 2/4] Fix invalid redirects for optional catch-all route at top-level See: https://github.com/netlify/next-on-netlify/issues/45 --- lib/helpers/getNetlifyRoutes.js | 17 ++++++++++++++++- .../__snapshots__/optionalCatchAll.test.js.snap | 4 ++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/helpers/getNetlifyRoutes.js b/lib/helpers/getNetlifyRoutes.js index 034551c..9f7b981 100644 --- a/lib/helpers/getNetlifyRoutes.js +++ b/lib/helpers/getNetlifyRoutes.js @@ -16,7 +16,22 @@ const getNetlifyRoutes = (nextRoute) => { // Netlify route for the base path (when no parameters are present). // The file ending must be present! if (nextRoute.match(OPTIONAL_CATCH_ALL_REGEX)) { - netlifyRoutes.push(nextRoute.replace(OPTIONAL_CATCH_ALL_REGEX, "$2")); + let netlifyRoute = nextRoute.replace(OPTIONAL_CATCH_ALL_REGEX, "$2"); + + // When optional catch-all route is at top-level, the regex on line 19 will + // create an empty string, but actually needs to be a forward slash + if (netlifyRoute === "") netlifyRoute = "/"; + + // When optional catch-all route is at top-level, the regex on line 19 will + // create an incorrect route for the data route. For example, it creates + // /_next/data/%BUILDID%.json, but NextJS looks for + // /_next/data/%BUILDID%/index.json + netlifyRoute = netlifyRoute.replace( + /(\/_next\/data\/[^/]+).json/, + "$1/index.json" + ); + + netlifyRoutes.push(netlifyRoute); } // Replace catch-all, e.g., [...slug] diff --git a/tests/__snapshots__/optionalCatchAll.test.js.snap b/tests/__snapshots__/optionalCatchAll.test.js.snap index b9d3889..a128a48 100644 --- a/tests/__snapshots__/optionalCatchAll.test.js.snap +++ b/tests/__snapshots__/optionalCatchAll.test.js.snap @@ -3,9 +3,9 @@ exports[`Routing creates Netlify redirects 1`] = ` "# Next-on-Netlify Redirects /* /.netlify/functions/next_all 200 - /.netlify/functions/next_all 200 +/ /.netlify/functions/next_all 200 /_next/data/%BUILD_ID%/page.json /.netlify/functions/next_page 200 /_next/data/%BUILD_ID%/* /.netlify/functions/next_all 200 -/_next/data/%BUILD_ID%/.netlify/functions/next_all 200 +/_next/data/%BUILD_ID%/index.json /.netlify/functions/next_all 200 /page /.netlify/functions/next_page 200" `; From 16f2502a3143b9848dbf0bf6639d64ef648a42cb Mon Sep 17 00:00:00 2001 From: Finn Woelm Date: Fri, 2 Oct 2020 23:06:22 +0200 Subject: [PATCH 3/4] 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. --- lib/helpers/getNetlifyRoutes.js | 3 ++- lib/helpers/getSortedRoutes.js | 8 ++++---- tests/__snapshots__/defaults.test.js.snap | 10 +++++----- tests/__snapshots__/optionalCatchAll.test.js.snap | 8 ++++---- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/lib/helpers/getNetlifyRoutes.js b/lib/helpers/getNetlifyRoutes.js index 9f7b981..16ee912 100644 --- a/lib/helpers/getNetlifyRoutes.js +++ b/lib/helpers/getNetlifyRoutes.js @@ -31,7 +31,8 @@ const getNetlifyRoutes = (nextRoute) => { "$1/index.json" ); - netlifyRoutes.push(netlifyRoute); + // Add second route to the front of the array + netlifyRoutes.unshift(netlifyRoute); } // Replace catch-all, e.g., [...slug] diff --git a/lib/helpers/getSortedRoutes.js b/lib/helpers/getSortedRoutes.js index b56cc92..50b09f8 100644 --- a/lib/helpers/getSortedRoutes.js +++ b/lib/helpers/getSortedRoutes.js @@ -1,9 +1,9 @@ const { - getSortedRoutes: getSortedRoutesFromSlsNext, -} = require("@sls-next/lambda-at-edge/dist/lib/sortedRoutes"); + getSortedRoutes: getSortedRoutesFromNext, +} = require("next/dist/next-server/lib/router/utils/sorted-routes"); // Remove the file extension form the route -const removeFileExtension = (route) => route.replace(/\.[A-z]+$/, ""); +const removeFileExtension = (route) => route.replace(/\.[a-zA-Z]+$/, ""); // Return an array of routes sorted in order of specificity, i.e., more generic // routes precede more specific ones @@ -16,7 +16,7 @@ const getSortedRoutes = (routes) => { ); // Sort the "naked" routes - const sortedRoutes = getSortedRoutesFromSlsNext(routesWithoutExtensions); + const sortedRoutes = getSortedRoutesFromNext(routesWithoutExtensions); // Return original routes in the sorted order return routes.sort( diff --git a/tests/__snapshots__/defaults.test.js.snap b/tests/__snapshots__/defaults.test.js.snap index 293dae7..2c3d9d1 100644 --- a/tests/__snapshots__/defaults.test.js.snap +++ b/tests/__snapshots__/defaults.test.js.snap @@ -3,8 +3,8 @@ exports[`Routing creates Netlify redirects 1`] = ` "# Next-on-Netlify Redirects / /.netlify/functions/next_index 200 -/_next/data/%BUILD_ID%/getServerSideProps/all/* /.netlify/functions/next_getServerSideProps_all_slug 200 /_next/data/%BUILD_ID%/getServerSideProps/all.json /.netlify/functions/next_getServerSideProps_all_slug 200 +/_next/data/%BUILD_ID%/getServerSideProps/all/* /.netlify/functions/next_getServerSideProps_all_slug 200 /_next/data/%BUILD_ID%/getServerSideProps/static.json /.netlify/functions/next_getServerSideProps_static 200 /_next/data/%BUILD_ID%/getServerSideProps/:id.json /.netlify/functions/next_getServerSideProps_id 200 /_next/data/%BUILD_ID%/getStaticProps/with-revalidate.json /.netlify/functions/next_getStaticProps_withrevalidate 200 @@ -13,20 +13,20 @@ exports[`Routing creates Netlify redirects 1`] = ` /_next/data/%BUILD_ID%/getStaticProps/withRevalidate/1.json /.netlify/functions/next_getStaticProps_withRevalidate_id 200 /_next/data/%BUILD_ID%/getStaticProps/withRevalidate/2.json /.netlify/functions/next_getStaticProps_withRevalidate_id 200 /_next/data/%BUILD_ID%/getStaticProps/withRevalidate/withFallback/:id.json /.netlify/functions/next_getStaticProps_withRevalidate_withFallback_id 200 -/api/shows/:params/* /.netlify/functions/next_api_shows_params 200 /api/shows/:id /.netlify/functions/next_api_shows_id 200 +/api/shows/:params/* /.netlify/functions/next_api_shows_params 200 /api/static /.netlify/functions/next_api_static 200 -/getServerSideProps/all/* /.netlify/functions/next_getServerSideProps_all_slug 200 /getServerSideProps/all /.netlify/functions/next_getServerSideProps_all_slug 200 +/getServerSideProps/all/* /.netlify/functions/next_getServerSideProps_all_slug 200 /getServerSideProps/static /.netlify/functions/next_getServerSideProps_static 200 /getServerSideProps/:id /.netlify/functions/next_getServerSideProps_id 200 /getStaticProps/with-revalidate /.netlify/functions/next_getStaticProps_withrevalidate 200 -/getStaticProps/withFallback/:slug/* /.netlify/functions/next_getStaticProps_withFallback_slug 200 /getStaticProps/withFallback/:id /.netlify/functions/next_getStaticProps_withFallback_id 200 +/getStaticProps/withFallback/:slug/* /.netlify/functions/next_getStaticProps_withFallback_slug 200 /getStaticProps/withRevalidate/1 /.netlify/functions/next_getStaticProps_withRevalidate_id 200 /getStaticProps/withRevalidate/2 /.netlify/functions/next_getStaticProps_withRevalidate_id 200 /getStaticProps/withRevalidate/withFallback/:id /.netlify/functions/next_getStaticProps_withRevalidate_withFallback_id 200 -/shows/:params/* /.netlify/functions/next_shows_params 200 /shows/:id /.netlify/functions/next_shows_id 200 +/shows/:params/* /.netlify/functions/next_shows_params 200 /static/:id /static/[id].html 200" `; diff --git a/tests/__snapshots__/optionalCatchAll.test.js.snap b/tests/__snapshots__/optionalCatchAll.test.js.snap index a128a48..c3ab72b 100644 --- a/tests/__snapshots__/optionalCatchAll.test.js.snap +++ b/tests/__snapshots__/optionalCatchAll.test.js.snap @@ -2,10 +2,10 @@ exports[`Routing creates Netlify redirects 1`] = ` "# Next-on-Netlify Redirects -/* /.netlify/functions/next_all 200 -/ /.netlify/functions/next_all 200 /_next/data/%BUILD_ID%/page.json /.netlify/functions/next_page 200 -/_next/data/%BUILD_ID%/* /.netlify/functions/next_all 200 /_next/data/%BUILD_ID%/index.json /.netlify/functions/next_all 200 -/page /.netlify/functions/next_page 200" +/_next/data/%BUILD_ID%/* /.netlify/functions/next_all 200 +/page /.netlify/functions/next_page 200 +/ /.netlify/functions/next_all 200 +/* /.netlify/functions/next_all 200" `; From cf70204a2d06816978a6c3d5c7c20f810f71cd32 Mon Sep 17 00:00:00 2001 From: Finn Woelm Date: Fri, 2 Oct 2020 23:10:58 +0200 Subject: [PATCH 4/4] Cleanup: Remove unused require-statements Remove two unused require-statements from /lib/pages/getStaticProps/redirects.js. These are leftover from refactoring. --- lib/pages/getStaticProps/redirects.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/pages/getStaticProps/redirects.js b/lib/pages/getStaticProps/redirects.js index dfc8f11..7d5d95a 100644 --- a/lib/pages/getStaticProps/redirects.js +++ b/lib/pages/getStaticProps/redirects.js @@ -1,5 +1,3 @@ -const { relative } = require("path"); -const isDynamicRoute = require("../../helpers/isDynamicRoute"); const pages = require("./pages"); // Pages with getStaticProps do not need redirects, unless they are using