Skip to content

Commit

Permalink
fix: ignore invalid i18n functions with valid alternatives (#466)
Browse files Browse the repository at this point in the history
  • Loading branch information
james-elicx authored Sep 24, 2023
1 parent 23562ad commit 628f6bb
Show file tree
Hide file tree
Showing 9 changed files with 235 additions and 7 deletions.
5 changes: 5 additions & 0 deletions .changeset/rich-squids-visit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@cloudflare/next-on-pages': patch
---

Ignore invalid nodejs i18n functions with valid alternatives.
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ async function prepareAndBuildWorker(
workerJsDir,
nopDistDir: join(workerJsDir, '__next-on-pages-dist__'),
disableChunksDedup,
vercelConfig,
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export type CollectedFunctions = {
edgeFunctions: Map<string, FunctionInfo>;
prerenderedFunctions: Map<string, FunctionInfo>;
invalidFunctions: Map<string, FunctionInfo>;
ignoredFunctions: Map<string, FunctionInfo>;
ignoredFunctions: Map<string, FunctionInfo & { reason?: string }>;
};

export type FunctionInfo = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ export async function processEdgeFunctions(
break;
}
case 'ignore': {
ignoredFunctions.set(path, fnInfo);
ignoredFunctions.set(path, {
reason: entrypointStatus.reason,
...fnInfo,
});
edgeFunctions.delete(path);
break;
}
Expand Down Expand Up @@ -89,7 +92,9 @@ async function checkEntrypoint(
relativePath: string,
entrypoint: string,
): Promise<
{ value: 'ignore' | 'invalid' } | { value: 'valid'; finalEntrypoint: string }
| { value: 'invalid' }
| { value: 'ignore'; reason: string }
| { value: 'valid'; finalEntrypoint: string }
> {
let finalEntrypoint = entrypoint;

Expand All @@ -109,7 +114,7 @@ async function checkEntrypoint(
cliWarn(
`Detected an invalid middleware function for ${relativePath}. Skipping...`,
);
return { value: 'ignore' };
return { value: 'ignore', reason: 'invalid middleware function' };
}

return { value: 'invalid' };
Expand Down Expand Up @@ -196,7 +201,10 @@ function replaceRscWithNonRsc(
tempFunctionsMap.delete(formattedPath);
edgeFunctions.delete(path);
invalidFunctions.delete(path);
ignoredFunctions.set(path, rscFnInfo);
ignoredFunctions.set(path, {
reason: 'unnecessary rsc function',
...rscFnInfo,
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export async function processVercelFunctions(

await processEdgeFunctions(collectedFunctions);

await checkInvalidFunctions(collectedFunctions);
await checkInvalidFunctions(collectedFunctions, opts);

const identifiers = await dedupeEdgeFunctions(collectedFunctions, opts);

Expand All @@ -36,6 +36,7 @@ export type ProcessVercelFunctionsOpts = {
workerJsDir: string;
nopDistDir: string;
disableChunksDedup?: boolean;
vercelConfig: VercelConfig;
};

export type ProcessedVercelFunctions = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { gtr as versionGreaterThan, coerce } from 'semver';
import { cliError, cliWarn } from '../../cli';
import { getPackageVersion } from '../packageManagerUtils';
import { stripFuncExtension } from '../../utils';
import { formatRoutePath, stripFuncExtension } from '../../utils';
import type { CollectedFunctions, FunctionInfo } from './configs';
import { join, resolve } from 'path';
import type { ProcessVercelFunctionsOpts } from '.';

/**
* Checks if there are any invalid functions from the Vercel build output.
Expand All @@ -14,12 +15,16 @@ import { join, resolve } from 'path';
* If however the build output can't be used, an error message will be printed and the process will exit.
*
* @param collectedFunctions Collected functions from the Vercel build output.
* @param opts Options for processing Vercel functions.
*/
export async function checkInvalidFunctions(
collectedFunctions: CollectedFunctions,
opts: Pick<ProcessVercelFunctionsOpts, 'functionsDir' | 'vercelConfig'>,
): Promise<void> {
await tryToFixNotFoundRoute(collectedFunctions);

await tryToFixI18nFunctions(collectedFunctions, opts);

if (collectedFunctions.invalidFunctions.size > 0) {
await printInvalidFunctionsErrorMessage(
collectedFunctions.invalidFunctions,
Expand Down Expand Up @@ -71,6 +76,71 @@ async function tryToFixNotFoundRoute(
}
}

/**
* Tries to fix potential unnecessary and invalid i18n functions from the Vercel build output.
*
* This is a workaround for Vercel creating invalid Node.js i18n functions in the build output, and
* is achieved by combing through the Vercel build output config to find i18n keys that match the
* invalid functions.
*
* @param collectedFunctions Collected functions from the Vercel build output.
* @param opts Options for processing Vercel functions.
*/
async function tryToFixI18nFunctions(
{ edgeFunctions, invalidFunctions, ignoredFunctions }: CollectedFunctions,
{
vercelConfig,
functionsDir,
}: Pick<ProcessVercelFunctionsOpts, 'functionsDir' | 'vercelConfig'>,
): Promise<void> {
if (!invalidFunctions.size || !vercelConfig.routes?.length) {
return;
}

const foundI18nKeys = vercelConfig.routes.reduce((acc, route) => {
if ('handle' in route) return acc;

// Matches the format used in certain source route entries in the build output config.
// e.g. "src": "/(?<nextLocale>default|en|ja)(/.*|$)"
/\(\?<nextLocale>([^)]+)\)/
.exec(route.src)?.[1]
?.split('|')
?.forEach(locale => acc.add(locale));

return acc;
}, new Set<string>());

if (!foundI18nKeys.size) {
// no i18n keys found in the build output config, so we can't fix anything
return;
}

for (const [fullPath, fnInfo] of invalidFunctions.entries()) {
for (const i18nKey of foundI18nKeys) {
const firstRouteSegment = stripFuncExtension(fnInfo.relativePath)
.replace(/^\//, '')
.split('/')[0];

if (firstRouteSegment === i18nKey) {
const pathWithoutI18nKey = fnInfo.relativePath
.replace(new RegExp(`^/${i18nKey}.func`), '/index.func')
.replace(new RegExp(`^/${i18nKey}/`), '/');
const fullPathWithoutI18nKey = join(functionsDir, pathWithoutI18nKey);

const edgeFn = edgeFunctions.get(fullPathWithoutI18nKey);
if (edgeFn) {
invalidFunctions.delete(fullPath);
ignoredFunctions.set(fullPath, {
reason: 'unnecessary invalid i18n function',
...fnInfo,
});
edgeFn.route?.overrides?.push(formatRoutePath(fnInfo.relativePath));
}
}
}
}
}

/**
* Prints an error message for the invalid functions from the Vercel build output.
*
Expand Down
1 change: 1 addition & 0 deletions packages/next-on-pages/tests/_helpers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ export async function createRouterTestData(
workerJsDir: workerJsDir,
nopDistDir: join(workerJsDir, '__next-on-pages-dist__'),
disableChunksDedup: true,
vercelConfig: { version: 3 },
});

const staticAssets = await getVercelStaticAssets();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
import { describe, test, expect, afterEach, vi } from 'vitest';
import mockFs from 'mock-fs';
import {
collectFunctionsFrom,
mockConsole,
edgeFuncDir,
nodejsFuncDir,
getRouteInfo,
} from '../../../_helpers';
import { resolve } from 'path';
import { processEdgeFunctions } from '../../../../src/buildApplication/processVercelFunctions/edgeFunctions';
import { checkInvalidFunctions } from '../../../../src/buildApplication/processVercelFunctions/invalidFunctions';

const functionsDir = resolve('.vercel/output/functions');

describe('checkInvalidFunctions', () => {
afterEach(() => mockFs.restore());

test('should ignore i18n index with valid alternative', async () => {
const { collectedFunctions, restoreFsMock } = await collectFunctionsFrom({
functions: {
'index.func': edgeFuncDir,
'en.func': nodejsFuncDir,
},
});

await processEdgeFunctions(collectedFunctions);
await checkInvalidFunctions(collectedFunctions, {
functionsDir,
vercelConfig: {
version: 3,
routes: [{ src: '/(?<nextLocale>fr|en|nl)(/.*|$)' }],
},
});
restoreFsMock();

const { edgeFunctions, invalidFunctions, ignoredFunctions } =
collectedFunctions;

expect(edgeFunctions.size).toEqual(1);
expect(getRouteInfo(edgeFunctions, 'index.func')).toEqual({
path: '/index',
overrides: ['/', '/en'],
});

expect(invalidFunctions.size).toEqual(0);

expect(ignoredFunctions.size).toEqual(1);
expect(ignoredFunctions.has(resolve(functionsDir, 'en.func'))).toEqual(
true,
);
});

test('should ignore i18n nested route with valid alternative', async () => {
const { collectedFunctions, restoreFsMock } = await collectFunctionsFrom({
functions: {
test: { 'route.func': edgeFuncDir },
en: { test: { 'route.func': nodejsFuncDir } },
},
});

await processEdgeFunctions(collectedFunctions);
await checkInvalidFunctions(collectedFunctions, {
functionsDir,
vercelConfig: {
version: 3,
routes: [{ src: '/(?<nextLocale>fr|en|nl)(/.*|$)' }],
},
});
restoreFsMock();

const { edgeFunctions, invalidFunctions, ignoredFunctions } =
collectedFunctions;

expect(edgeFunctions.size).toEqual(1);
expect(getRouteInfo(edgeFunctions, 'test/route.func')).toEqual({
path: '/test/route',
overrides: ['/en/test/route'],
});

expect(invalidFunctions.size).toEqual(0);

expect(ignoredFunctions.size).toEqual(1);
expect(
ignoredFunctions.has(resolve(functionsDir, 'en/test/route.func')),
).toEqual(true);
});

test('should not ignore i18n with no valid alternative', async () => {
const processExitMock = vi
.spyOn(process, 'exit')
.mockImplementation(async () => undefined as never);
const mockedConsole = mockConsole('error');

const { collectedFunctions, restoreFsMock } = await collectFunctionsFrom({
functions: {
'en.func': nodejsFuncDir,
},
});

await processEdgeFunctions(collectedFunctions);
await checkInvalidFunctions(collectedFunctions, {
functionsDir,
vercelConfig: {
version: 3,
routes: [{ src: '/(?<nextLocale>fr|en|nl)(/.*|$)' }],
},
});
restoreFsMock();

const { edgeFunctions, invalidFunctions, ignoredFunctions } =
collectedFunctions;

expect(edgeFunctions.size).toEqual(0);

expect(invalidFunctions.size).toEqual(1);
expect(invalidFunctions.has(resolve(functionsDir, 'en.func'))).toEqual(
true,
);

expect(ignoredFunctions.size).toEqual(0);

expect(processExitMock).toHaveBeenCalledWith(1);
mockedConsole.expectCalls([
/The following routes were not configured to run with the Edge Runtime(?:.|\n)+- \/en/,
]);

processExitMock.mockRestore();
mockedConsole.restore();
});
});
Loading

0 comments on commit 628f6bb

Please sign in to comment.