From 628f6bbbfa8251b703de273f2563567c467f01b8 Mon Sep 17 00:00:00 2001 From: James Date: Sun, 24 Sep 2023 12:45:36 +0100 Subject: [PATCH] fix: ignore invalid i18n functions with valid alternatives (#466) --- .changeset/rich-squids-visit.md | 5 + .../src/buildApplication/buildApplication.ts | 1 + .../processVercelFunctions/configs.ts | 2 +- .../processVercelFunctions/edgeFunctions.ts | 16 ++- .../processVercelFunctions/index.ts | 3 +- .../invalidFunctions.ts | 72 +++++++++- .../next-on-pages/tests/_helpers/index.ts | 1 + .../invalidFunctions.test.ts | 131 ++++++++++++++++++ .../prerenderFunctions.test.ts | 11 ++ 9 files changed, 235 insertions(+), 7 deletions(-) create mode 100644 .changeset/rich-squids-visit.md create mode 100644 packages/next-on-pages/tests/src/buildApplication/processVercelFunctions/invalidFunctions.test.ts diff --git a/.changeset/rich-squids-visit.md b/.changeset/rich-squids-visit.md new file mode 100644 index 000000000..eb492b6d6 --- /dev/null +++ b/.changeset/rich-squids-visit.md @@ -0,0 +1,5 @@ +--- +'@cloudflare/next-on-pages': patch +--- + +Ignore invalid nodejs i18n functions with valid alternatives. diff --git a/packages/next-on-pages/src/buildApplication/buildApplication.ts b/packages/next-on-pages/src/buildApplication/buildApplication.ts index 734a7c918..1ed24027e 100644 --- a/packages/next-on-pages/src/buildApplication/buildApplication.ts +++ b/packages/next-on-pages/src/buildApplication/buildApplication.ts @@ -107,6 +107,7 @@ async function prepareAndBuildWorker( workerJsDir, nopDistDir: join(workerJsDir, '__next-on-pages-dist__'), disableChunksDedup, + vercelConfig, }); } diff --git a/packages/next-on-pages/src/buildApplication/processVercelFunctions/configs.ts b/packages/next-on-pages/src/buildApplication/processVercelFunctions/configs.ts index 016ef4941..a3a23943e 100644 --- a/packages/next-on-pages/src/buildApplication/processVercelFunctions/configs.ts +++ b/packages/next-on-pages/src/buildApplication/processVercelFunctions/configs.ts @@ -63,7 +63,7 @@ export type CollectedFunctions = { edgeFunctions: Map; prerenderedFunctions: Map; invalidFunctions: Map; - ignoredFunctions: Map; + ignoredFunctions: Map; }; export type FunctionInfo = { diff --git a/packages/next-on-pages/src/buildApplication/processVercelFunctions/edgeFunctions.ts b/packages/next-on-pages/src/buildApplication/processVercelFunctions/edgeFunctions.ts index c17db2c6f..332dab24f 100644 --- a/packages/next-on-pages/src/buildApplication/processVercelFunctions/edgeFunctions.ts +++ b/packages/next-on-pages/src/buildApplication/processVercelFunctions/edgeFunctions.ts @@ -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; } @@ -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; @@ -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' }; @@ -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, + }); } } } diff --git a/packages/next-on-pages/src/buildApplication/processVercelFunctions/index.ts b/packages/next-on-pages/src/buildApplication/processVercelFunctions/index.ts index 11ed8be32..d980a75e2 100644 --- a/packages/next-on-pages/src/buildApplication/processVercelFunctions/index.ts +++ b/packages/next-on-pages/src/buildApplication/processVercelFunctions/index.ts @@ -23,7 +23,7 @@ export async function processVercelFunctions( await processEdgeFunctions(collectedFunctions); - await checkInvalidFunctions(collectedFunctions); + await checkInvalidFunctions(collectedFunctions, opts); const identifiers = await dedupeEdgeFunctions(collectedFunctions, opts); @@ -36,6 +36,7 @@ export type ProcessVercelFunctionsOpts = { workerJsDir: string; nopDistDir: string; disableChunksDedup?: boolean; + vercelConfig: VercelConfig; }; export type ProcessedVercelFunctions = { diff --git a/packages/next-on-pages/src/buildApplication/processVercelFunctions/invalidFunctions.ts b/packages/next-on-pages/src/buildApplication/processVercelFunctions/invalidFunctions.ts index 0d68e67aa..b3f7adf39 100644 --- a/packages/next-on-pages/src/buildApplication/processVercelFunctions/invalidFunctions.ts +++ b/packages/next-on-pages/src/buildApplication/processVercelFunctions/invalidFunctions.ts @@ -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. @@ -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, ): Promise { await tryToFixNotFoundRoute(collectedFunctions); + await tryToFixI18nFunctions(collectedFunctions, opts); + if (collectedFunctions.invalidFunctions.size > 0) { await printInvalidFunctionsErrorMessage( collectedFunctions.invalidFunctions, @@ -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, +): Promise { + 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": "/(?default|en|ja)(/.*|$)" + /\(\?([^)]+)\)/ + .exec(route.src)?.[1] + ?.split('|') + ?.forEach(locale => acc.add(locale)); + + return acc; + }, new Set()); + + 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. * diff --git a/packages/next-on-pages/tests/_helpers/index.ts b/packages/next-on-pages/tests/_helpers/index.ts index 41d2ad05d..cb392aff9 100644 --- a/packages/next-on-pages/tests/_helpers/index.ts +++ b/packages/next-on-pages/tests/_helpers/index.ts @@ -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(); diff --git a/packages/next-on-pages/tests/src/buildApplication/processVercelFunctions/invalidFunctions.test.ts b/packages/next-on-pages/tests/src/buildApplication/processVercelFunctions/invalidFunctions.test.ts new file mode 100644 index 000000000..b4e1588e0 --- /dev/null +++ b/packages/next-on-pages/tests/src/buildApplication/processVercelFunctions/invalidFunctions.test.ts @@ -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: '/(?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: '/(?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: '/(?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(); + }); +}); diff --git a/packages/next-on-pages/tests/src/buildApplication/processVercelFunctions/prerenderFunctions.test.ts b/packages/next-on-pages/tests/src/buildApplication/processVercelFunctions/prerenderFunctions.test.ts index bb5690f34..d9108fe3a 100644 --- a/packages/next-on-pages/tests/src/buildApplication/processVercelFunctions/prerenderFunctions.test.ts +++ b/packages/next-on-pages/tests/src/buildApplication/processVercelFunctions/prerenderFunctions.test.ts @@ -39,6 +39,7 @@ describe('processPrerenderFunctions', () => { outputDir, workerJsDir, nopDistDir, + vercelConfig: { version: 3 }, }); restoreFsMock(); @@ -92,6 +93,7 @@ describe('processPrerenderFunctions', () => { outputDir, workerJsDir, nopDistDir, + vercelConfig: { version: 3 }, }); restoreFsMock(); @@ -144,6 +146,7 @@ describe('processPrerenderFunctions', () => { outputDir, workerJsDir, nopDistDir, + vercelConfig: { version: 3 }, }); restoreFsMock(); @@ -201,6 +204,7 @@ describe('processPrerenderFunctions', () => { outputDir, workerJsDir, nopDistDir, + vercelConfig: { version: 3 }, }); restoreFsMock(); @@ -261,6 +265,7 @@ describe('processPrerenderFunctions', () => { outputDir, workerJsDir, nopDistDir, + vercelConfig: { version: 3 }, }); restoreFsMock(); @@ -315,6 +320,7 @@ describe('processPrerenderFunctions', () => { outputDir, workerJsDir, nopDistDir, + vercelConfig: { version: 3 }, }); restoreFsMock(); @@ -366,6 +372,7 @@ describe('processPrerenderFunctions', () => { outputDir, workerJsDir, nopDistDir, + vercelConfig: { version: 3 }, }); restoreFsMock(); @@ -418,6 +425,7 @@ describe('processPrerenderFunctions', () => { outputDir, workerJsDir, nopDistDir, + vercelConfig: { version: 3 }, }); restoreFsMock(); @@ -465,6 +473,7 @@ describe('processPrerenderFunctions', () => { outputDir, workerJsDir, nopDistDir, + vercelConfig: { version: 3 }, }); restoreFsMock(); @@ -510,6 +519,7 @@ describe('processPrerenderFunctions', () => { outputDir: customOutputDir, workerJsDir: customWorkerJsDir, nopDistDir: join(customWorkerJsDir, '__next-on-pages-dist__'), + vercelConfig: { version: 3 }, }); const { edgeFunctions, prerenderedFunctions, invalidFunctions } = @@ -576,6 +586,7 @@ describe('processPrerenderFunctions', () => { outputDir: resolve('custom'), workerJsDir: customWorkerJsDir, nopDistDir: join(customWorkerJsDir, '__next-on-pages-dist__'), + vercelConfig: { version: 3 }, }); const { edgeFunctions, prerenderedFunctions, invalidFunctions } =