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

[fix] page chunk for root level catch-all is served incorrectly to client #52

Merged
merged 1 commit into from
Oct 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions lib/constants/regex.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
const CATCH_ALL_REGEX = /\/\[\.{3}(.*)\](.json)?$/;
const OPTIONAL_CATCH_ALL_REGEX = /\/\[{2}\.{3}(.*)\]{2}(.json)?$/;
const DYNAMIC_PARAMETER_REGEX = /\/\[(.*?)\]/g;

module.exports = {
CATCH_ALL_REGEX,
OPTIONAL_CATCH_ALL_REGEX,
DYNAMIC_PARAMETER_REGEX,
};
8 changes: 5 additions & 3 deletions lib/helpers/getNetlifyRoutes.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
// Adapted from @sls-next/lambda-at-edge (v1.2.0-alpha.3)
// See: https://github.com/danielcondemarin/serverless-next.js/blob/57142970b08e6bc3faf0fc70749b3b0501ad7869/packages/lambda-at-edge/src/lib/expressifyDynamicRoute.ts#L4

const CATCH_ALL_REGEX = /\/\[\.{3}(.*)\](.json)?$/;
const OPTIONAL_CATCH_ALL_REGEX = /\/\[{2}\.{3}(.*)\]{2}(.json)?$/;
const DYNAMIC_PARAMETER_REGEX = /\/\[(.*?)\]/g;
const {
CATCH_ALL_REGEX,
OPTIONAL_CATCH_ALL_REGEX,
DYNAMIC_PARAMETER_REGEX,
} = require("../constants/regex");

// Convert dynamic NextJS routes into their Netlify-equivalent
// Note that file endings (.json) must be removed for catch-all and optional
Expand Down
5 changes: 5 additions & 0 deletions lib/helpers/isRootCatchAllRedirect.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// Return true if the redirect is a root level catch-all
// (e.g., /[[...slug]])
const isRootCatchAllRedirect = (redirect) => redirect.startsWith("/*");

module.exports = isRootCatchAllRedirect;
13 changes: 12 additions & 1 deletion lib/steps/setupRedirects.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const { logTitle, logItem } = require("../helpers/logger");
const { NETLIFY_PUBLISH_PATH, CUSTOM_REDIRECTS_PATH } = require("../config");
const getSortedRoutes = require("../helpers/getSortedRoutes");
const getNetlifyRoutes = require("../helpers/getNetlifyRoutes");
const isRootCatchAllRedirect = require("../helpers/isRootCatchAllRedirect");

// Setup _redirects file that routes all requests to the appropriate location,
// such as one of the Netlify functions or one of the static files.
Expand All @@ -14,7 +15,7 @@ const setupRedirects = () => {
const redirects = [];
if (existsSync(CUSTOM_REDIRECTS_PATH)) {
logItem("# Prepending custom redirects");
redirects.push(readFileSync(CUSTOM_REDIRECTS_PATH));
redirects.push(readFileSync(CUSTOM_REDIRECTS_PATH, "utf8"));
}

// Collect redirects for NextJS pages
Expand Down Expand Up @@ -50,6 +51,16 @@ const setupRedirects = () => {
});
});

// This takes care of this issue: https://github.com/netlify/next-on-netlify/issues/43
// where the page chunk for a root level catch-all is served incorrectly to the client.
// NOTE: Netlify is also investigating this issue internally.
const hasRootCatchAll = redirects.some(isRootCatchAllRedirect);
if (hasRootCatchAll) {
const rootCatchAllIndex = redirects.findIndex(isRootCatchAllRedirect);
// Add general "no-op" redirect before the root catch-all redirect
redirects.splice(rootCatchAllIndex, 0, "/_next/* /_next/:splat 200");
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful :)

// Write redirects to _redirects file
writeFileSync(join(NETLIFY_PUBLISH_PATH, "_redirects"), redirects.join("\n"));
};
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
},
"husky": {
"hooks": {
"pre-commit": "npm run format"
"pre-commit": "prettier --check ."
}
}
}
1 change: 1 addition & 0 deletions tests/__snapshots__/optionalCatchAll.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ exports[`Routing creates Netlify redirects 1`] = `
/_next/data/%BUILD_ID%/* /.netlify/functions/next_all 200
/page /.netlify/functions/next_page 200
/ /.netlify/functions/next_all 200
/_next/* /_next/:splat 200
/* /.netlify/functions/next_all 200"
`;