Skip to content
This repository has been archived by the owner on Jan 28, 2025. It is now read-only.

Commit

Permalink
fix(lambda-at-edge): locale subpath bug fixes
Browse files Browse the repository at this point in the history 
* SSG index page routing
* static pages / json files only in locale directories
* separate unit tests for locale testing with/without basepath
  • Loading branch information
@dphang
dphang committed Jan 26, 2021
1 parent 8e39294 commit 95e000f
Show file tree
Hide file tree
Showing 29 changed files with 3,092 additions and 836 deletions.
96 changes: 35 additions & 61 deletions packages/libs/lambda-at-edge/src/build.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ class Builder {
htmlPagesNonDynamicLoop: for (const key in htmlPages.nonDynamic) {
// Locale-prefixed pages don't need to be duplicated
for (const locale of routesManifest.i18n.locales) {
if (key.startsWith(`/${locale}/`)) {
if (key.startsWith(`/${locale}/`) || key === `/${locale}`) {
break htmlPagesNonDynamicLoop;
}
}
Expand Down Expand Up @@ -715,11 +715,18 @@ class Builder {
ssgPages.nonDynamic[key]
);

// Replace with localized value
newSsgRoute.dataRoute = newSsgRoute.dataRoute.replace(
`/_next/data/${buildId}/`,
`/_next/data/${buildId}/${locale}/`
);
// Replace with localized value. For non-dynamic index page, this is in format "en.json"
if (key === "/") {
newSsgRoute.dataRoute = newSsgRoute.dataRoute.replace(
`/_next/data/${buildId}/index.json`,
`/_next/data/${buildId}/${locale}.json`
);
} else {
newSsgRoute.dataRoute = newSsgRoute.dataRoute.replace(
`/_next/data/${buildId}/`,
`/_next/data/${buildId}/${locale}/`
);
}

newSsgRoute.srcRoute = newSsgRoute.srcRoute
? `/${locale}/${newSsgRoute.srcRoute}`
Expand Down Expand Up @@ -911,9 +918,9 @@ class Builder {
let prerenderManifestHTMLPageAssets: Promise<void>[] = [];
let fallbackHTMLPageAssets: Promise<void>[] = [];

// Copy locale-specific prerendered files if defined, otherwise use empty which works for no locale
// Copy locale-specific prerendered files if defined, otherwise use empty locale
// which would copy to root only
const locales = routesManifest.i18n?.locales ?? [""];
const defaultLocale = routesManifest.i18n?.defaultLocale;

for (const locale of locales) {
prerenderManifestJSONPropFileAssets.concat(
Expand All @@ -922,7 +929,15 @@ class Builder {
? key + "index.json"
: key + ".json";

const localePrefixedJSONFileName = locale + JSONFileName;
let localePrefixedJSONFileName;

// If there are locales and index is SSG page
// Filename is <locale>.json e.g en.json, not index.json or en/index.json
if (locale && key === "/") {
localePrefixedJSONFileName = `${locale}.json`;
} else {
localePrefixedJSONFileName = locale + JSONFileName;
}

const source = path.join(
dotNextDirectory,
Expand All @@ -933,23 +948,7 @@ class Builder {
withBasePath(`_next/data/${buildId}/${localePrefixedJSONFileName}`)
);

if (defaultLocale && defaultLocale === locale) {
// If this is default locale, we need to copy to two destinations
// the locale-prefixed path and non-locale-prefixed path
const defaultDestination = path.join(
assetOutputDirectory,
withBasePath(`_next/data/${buildId}/${JSONFileName}`)
);

return new Promise(async () => {
await Promise.all([
copyIfExists(source, destination),
copyIfExists(source, defaultDestination)
]);
});
} else {
return copyIfExists(source, destination);
}
return copyIfExists(source, destination);
})
);

Expand All @@ -959,7 +958,14 @@ class Builder {
? path.join(key, "index.html")
: key + ".html";

const localePrefixedPageFilePath = locale + pageFilePath;
// If there are locales and index is SSG page,
// Filename is <locale>.html e.g en.html, not index.html or en/index.html
let localePrefixedPageFilePath;
if (locale && key === "/") {
localePrefixedPageFilePath = `${locale}.html`;
} else {
localePrefixedPageFilePath = locale + pageFilePath;
}

const source = path.join(
dotNextDirectory,
Expand All @@ -972,23 +978,7 @@ class Builder {
)
);

if (defaultLocale && defaultLocale === locale) {
// If this is default locale, we need to copy to two destinations
// the locale-prefixed path and non-locale-prefixed path
const defaultDestination = path.join(
assetOutputDirectory,
withBasePath(path.join("static-pages", buildId, pageFilePath))
);

return new Promise(async () => {
await Promise.all([
copyIfExists(source, destination),
copyIfExists(source, defaultDestination)
]);
});
} else {
return copyIfExists(source, destination);
}
return copyIfExists(source, destination);
})
);

Expand All @@ -1013,23 +1003,7 @@ class Builder {
)
);

if (defaultLocale && defaultLocale === locale) {
// If this is default locale, we need to copy to two destinations
// the locale-prefixed path and non-locale-prefixed path
const defaultDestination = path.join(
assetOutputDirectory,
withBasePath(path.join("static-pages", buildId, fallback))
);

return new Promise(async () => {
await Promise.all([
copyIfExists(source, destination),
copyIfExists(source, defaultDestination)
]);
});
} else {
return copyIfExists(source, destination);
}
return copyIfExists(source, destination);
})
);
}
Expand Down
111 changes: 72 additions & 39 deletions packages/libs/lambda-at-edge/src/default-handler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ import { addHeadersToResponse } from "./headers/addHeaders";
import { isValidPreviewRequest } from "./lib/isValidPreviewRequest";
import { getUnauthenticatedResponse } from "./auth/authenticator";
import { buildS3RetryStrategy } from "./s3/s3RetryStrategy";
import { isLocaleIndexUri } from "./routing/locale-utils";
import {
isLocalePrefixedUri,
removeLocalePrefixFromUri
} from "./routing/locale-utils";

const basePath = RoutesManifestJson.basePath;

Expand Down Expand Up @@ -67,13 +70,17 @@ const addS3HostHeader = (

const isDataRequest = (uri: string): boolean => uri.startsWith("/_next/data");

const normaliseUri = (uri: string): string => {
const normaliseUri = (uri: string, routesManifest: RoutesManifest): string => {
if (basePath) {
if (uri.startsWith(basePath)) {
uri = uri.slice(basePath.length);
} else {
// basePath set but URI does not start with basePath, return 404
return "/404";
if (routesManifest.i18n?.defaultLocale) {
return `/${routesManifest.i18n?.defaultLocale}/404`;
} else {
return "/404";
}
}
}

Expand Down Expand Up @@ -217,7 +224,8 @@ export const handler = async (
response = await handleOriginResponse({
event,
manifest,
prerenderManifest
prerenderManifest,
routesManifest
});
} else {
response = await handleOriginRequest({
Expand Down Expand Up @@ -277,7 +285,7 @@ const handleOriginRequest = async ({
}

const basePath = routesManifest.basePath;
let uri = normaliseUri(request.uri);
let uri = normaliseUri(request.uri, routesManifest);
const decodedUri = decodeURI(uri);
const { pages, publicFiles } = manifest;

Expand All @@ -294,7 +302,11 @@ const handleOriginRequest = async ({
if (newUri.endsWith("/")) {
newUri = newUri.slice(0, -1);
}
} else if (request.uri !== "/" && request.uri !== "" && uri !== "/404") {
} else if (
request.uri !== "/" &&
request.uri !== "" &&
!uri.endsWith("/404")
) {
// HTML/SSR pages get redirected based on trailingSlash in next.config.js
// We do not redirect:
// 1. Unnormalised URI is "/" or "" as this could cause a redirect loop due to browsers appending trailing slash
Expand Down Expand Up @@ -324,6 +336,34 @@ const handleOriginRequest = async ({
);
}

// Handle root language redirect
const languageHeader = request.headers["accept-language"];
const languageRedirectUri = getLanguageRedirect(
languageHeader ? languageHeader[0].value : undefined,
uri,
routesManifest,
manifest
);

if (languageRedirectUri) {
return createRedirectResponse(
languageRedirectUri,
request.querystring,
307
);
}

// Always add default locale prefix to URIs without it that are not public files or data requests
const defaultLocale = routesManifest.i18n?.defaultLocale;
if (
defaultLocale &&
!isLocalePrefixedUri(uri, routesManifest) &&
!isPublicFile &&
!isDataReq
) {
uri = uri === "/" ? `/${defaultLocale}` : `/${defaultLocale}${uri}`;
}

// Check for non-dynamic pages before rewriting
const isNonDynamicRoute =
pages.html.nonDynamic[uri] || pages.ssr.nonDynamic[uri] || isPublicFile;
Expand Down Expand Up @@ -369,25 +409,17 @@ const handleOriginRequest = async ({
return await responsePromise;
}

uri = normaliseUri(request.uri);
}
}
uri = normaliseUri(request.uri, routesManifest);

// Handle root language rewrite
const languageHeader = request.headers["accept-language"];
const languageRedirectUri = getLanguageRedirect(
languageHeader ? languageHeader[0].value : undefined,
uri,
routesManifest,
manifest
);

if (languageRedirectUri) {
return createRedirectResponse(
languageRedirectUri,
request.querystring,
307
);
if (
defaultLocale &&
!isLocalePrefixedUri(uri, routesManifest) &&
!isPublicFile &&
!isDataReq
) {
uri = uri === "/" ? `/${defaultLocale}` : `/${defaultLocale}${uri}`;
}
}
}

const isStaticPage = pages.html.nonDynamic[uri]; // plain page without any props
Expand All @@ -396,7 +428,7 @@ const handleOriginRequest = async ({
const s3Origin = origin.s3 as CloudFrontS3Origin;
const isHTMLPage = isStaticPage || isPrerenderedPage;
const normalisedS3DomainName = normaliseS3OriginDomain(s3Origin);
const hasFallback = hasFallbackForUri(uri, prerenderManifest, manifest);
const hasFallback = hasFallbackForUri(uri, manifest, routesManifest);
const { now, log } = perfLogger(manifest.logLambdaExecutionTimes);
const isPreviewRequest = isValidPreviewRequest(
request.headers.cookie,
Expand All @@ -420,12 +452,7 @@ const handleOriginRequest = async ({
}
} else if (isHTMLPage || hasFallback) {
s3Origin.path = `${basePath}/static-pages/${manifest.buildId}`;
let pageName;
if (isLocaleIndexUri(uri, routesManifest)) {
pageName = `${uri}/index`;
} else {
pageName = uri === "/" ? "/index" : uri;
}
const pageName = uri === "/" ? "/index" : uri;
request.uri = `${pageName}.html`;
} else if (isDataReq) {
// We need to check whether data request is unmatched i.e routed to 404.html or _error.js
Expand All @@ -441,8 +468,8 @@ const handleOriginRequest = async ({
(!pages.ssg.nonDynamic[normalisedDataRequestUri] &&
!hasFallbackForUri(
normalisedDataRequestUri,
prerenderManifest,
manifest
manifest,
routesManifest
))
) {
// Break to continue to SSR render in two cases:
Expand Down Expand Up @@ -526,11 +553,13 @@ const handleOriginRequest = async ({
const handleOriginResponse = async ({
event,
manifest,
prerenderManifest
prerenderManifest,
routesManifest
}: {
event: OriginResponseEvent;
manifest: OriginRequestDefaultHandlerManifest;
prerenderManifest: PrerenderManifestType;
routesManifest: RoutesManifest;
}) => {
const response = event.Records[0].cf.response;
const request = event.Records[0].cf.request;
Expand All @@ -549,7 +578,7 @@ const handleOriginResponse = async ({
return response;
}

const uri = normaliseUri(request.uri);
const uri = normaliseUri(request.uri, routesManifest);
const { domainName, region } = request.origin!.s3!;
const bucketName = domainName.replace(`.s3.${region}.amazonaws.com`, "");

Expand Down Expand Up @@ -615,7 +644,7 @@ const handleOriginResponse = async ({
res.end(JSON.stringify(renderOpts.pageData));
return await responsePromise;
} else {
const hasFallback = hasFallbackForUri(uri, prerenderManifest, manifest);
const hasFallback = hasFallbackForUri(uri, manifest, routesManifest);
if (!hasFallback) return response;

// If route has fallback, return that page from S3, otherwise return 404 page
Expand Down Expand Up @@ -675,8 +704,8 @@ const isOriginResponse = (

const hasFallbackForUri = (
uri: string,
prerenderManifest: PrerenderManifestType,
manifest: OriginRequestDefaultHandlerManifest
manifest: OriginRequestDefaultHandlerManifest,
routesManifest: RoutesManifest
) => {
const {
pages: { ssr, html, ssg }
Expand Down Expand Up @@ -712,7 +741,11 @@ const hasFallbackForUri = (
const matchesFallbackRoute = Object.keys(ssg.dynamic).find(
(dynamicSsgRoute) => {
const fileMatchesPrerenderRoute =
dynamicRoute.file === `pages${dynamicSsgRoute}.js`;
dynamicRoute.file ===
`pages${removeLocalePrefixFromUri(
dynamicSsgRoute,
routesManifest
)}.js`;

if (fileMatchesPrerenderRoute) {
foundFallback = ssg.dynamic[dynamicSsgRoute];
Expand Down
Loading

0 comments on commit 95e000f

Please sign in to comment.