From 9d789eeb11f7a049ed8e1f7c68ca866a1fc9226a Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Fri, 23 Jun 2023 14:03:58 +0200 Subject: [PATCH 01/12] chore: reproduce bug --- .../default/pages/api/shows/{[id].js => [show-id].js} | 2 +- test/__snapshots__/index.spec.ts.snap | 10 +++++----- test/helpers/functions.spec.ts | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) rename demos/default/pages/api/shows/{[id].js => [show-id].js} (94%) diff --git a/demos/default/pages/api/shows/[id].js b/demos/default/pages/api/shows/[show-id].js similarity index 94% rename from demos/default/pages/api/shows/[id].js rename to demos/default/pages/api/shows/[show-id].js index e0d1d088f1..8726585af1 100644 --- a/demos/default/pages/api/shows/[id].js +++ b/demos/default/pages/api/shows/[show-id].js @@ -4,7 +4,7 @@ export default async (req, res) => { // Get the ID of the show const { query } = req - const { id } = query + const id = query['show-id'] // Get the data const fetchRes = await fetch(`https://tvproxy.netlify.app/shows/${Number(id)}`) diff --git a/test/__snapshots__/index.spec.ts.snap b/test/__snapshots__/index.spec.ts.snap index b1230f0480..1d60432185 100644 --- a/test/__snapshots__/index.spec.ts.snap +++ b/test/__snapshots__/index.spec.ts.snap @@ -151,7 +151,7 @@ Array [ ".next/server/pages/api/og.js", ".next/server/pages/api/revalidate.js", ".next/server/pages/api/shows/[...params].js", - ".next/server/pages/api/shows/[id].js", + ".next/server/pages/api/shows/[show-id].js", ".next/server/pages/deep/import.js", ".next/server/pages/edge/[id].js", ".next/server/pages/getServerSideProps/[id].js", @@ -442,7 +442,7 @@ exports.resolvePages = () => { require.resolve('../../../.next/server/pages/api/og.js') require.resolve('../../../.next/server/pages/api/revalidate.js') require.resolve('../../../.next/server/pages/api/shows/[...params].js') - require.resolve('../../../.next/server/pages/api/shows/[id].js') + require.resolve('../../../.next/server/pages/api/shows/[show-id].js') require.resolve('../../../.next/server/pages/deep/import.js') require.resolve('../../../.next/server/pages/edge/[id].js') require.resolve('../../../.next/server/pages/getServerSideProps/[id].js') @@ -626,7 +626,7 @@ exports.resolvePages = () => { require.resolve('../../../.next/server/pages/api/og.js') require.resolve('../../../.next/server/pages/api/revalidate.js') require.resolve('../../../.next/server/pages/api/shows/[...params].js') - require.resolve('../../../.next/server/pages/api/shows/[id].js') + require.resolve('../../../.next/server/pages/api/shows/[show-id].js') require.resolve('../../../.next/server/pages/deep/import.js') require.resolve('../../../.next/server/pages/edge/[id].js') require.resolve('../../../.next/server/pages/getServerSideProps/[id].js') @@ -810,7 +810,7 @@ exports.resolvePages = () => { require.resolve('../../../web/.next/server/pages/api/og.js') require.resolve('../../../web/.next/server/pages/api/revalidate.js') require.resolve('../../../web/.next/server/pages/api/shows/[...params].js') - require.resolve('../../../web/.next/server/pages/api/shows/[id].js') + require.resolve('../../../web/.next/server/pages/api/shows/[show-id].js') require.resolve('../../../web/.next/server/pages/deep/import.js') require.resolve('../../../web/.next/server/pages/edge/[id].js') require.resolve('../../../web/.next/server/pages/getServerSideProps/[id].js') @@ -994,7 +994,7 @@ exports.resolvePages = () => { require.resolve('../../../web/.next/server/pages/api/og.js') require.resolve('../../../web/.next/server/pages/api/revalidate.js') require.resolve('../../../web/.next/server/pages/api/shows/[...params].js') - require.resolve('../../../web/.next/server/pages/api/shows/[id].js') + require.resolve('../../../web/.next/server/pages/api/shows/[show-id].js') require.resolve('../../../web/.next/server/pages/deep/import.js') require.resolve('../../../web/.next/server/pages/edge/[id].js') require.resolve('../../../web/.next/server/pages/getServerSideProps/[id].js') diff --git a/test/helpers/functions.spec.ts b/test/helpers/functions.spec.ts index 5dc08f66ca..bc37ed1406 100644 --- a/test/helpers/functions.spec.ts +++ b/test/helpers/functions.spec.ts @@ -46,11 +46,11 @@ describeCwdTmpDir('api route file analysis', () => { route: '/api/shows/[...params]', }, { - functionName: '_api_shows_id-PARAM-handler', - functionTitle: 'Next.js API handler /api/shows/[id]', - compiled: 'pages/api/shows/[id].js', + functionName: '_api_shows_show-id-PARAM-handler', + functionTitle: 'Next.js API handler /api/shows/[show-id]', + compiled: 'pages/api/shows/[show-id].js', config: {}, - route: '/api/shows/[id]', + route: '/api/shows/[show-id]', }, { functionName: '_api_hello-background-background', From f7f285621c44e5f2949448441b3df6566c21dd0e Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Fri, 23 Jun 2023 14:20:45 +0200 Subject: [PATCH 02/12] fix: don't interpret files as glob patterns --- packages/runtime/src/helpers/functions.ts | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/packages/runtime/src/helpers/functions.ts b/packages/runtime/src/helpers/functions.ts index 98a770b1c3..6cfef4b09e 100644 --- a/packages/runtime/src/helpers/functions.ts +++ b/packages/runtime/src/helpers/functions.ts @@ -310,19 +310,14 @@ export const getAPIPRouteCommonDependencies = async (publish: string) => { const sum = (arr: number[]) => arr.reduce((v, current) => v + current, 0) // TODO: cache results -const getBundleWeight = async (patterns: string[]) => { +const getBundleWeight = async (files: string[]) => { const sizes = await Promise.all( - patterns.flatMap(async (pattern) => { - const files = await glob(pattern) - return Promise.all( - files.map(async (file) => { - const fStat = await stat(file) - if (fStat.isFile()) { - return fStat.size - } - return 0 - }), - ) + files.map(async (file) => { + const fStat = await stat(file) + if (fStat.isFile()) { + return fStat.size + } + return 0 }), ) From 6b2946a8d1adad1095c0364fc040c28f7ea04cb5 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Fri, 23 Jun 2023 15:09:03 +0200 Subject: [PATCH 03/12] fix: move test directory relative to node_modules --- test/index.spec.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/index.spec.ts b/test/index.spec.ts index 038dfbde64..d5da0b207f 100644 --- a/test/index.spec.ts +++ b/test/index.spec.ts @@ -75,7 +75,9 @@ let cleanup // In each test, we change cwd to a temporary directory. // This allows us not to have to mock filesystem operations. beforeEach(async () => { - const tmpDir = await getTmpDir({ unsafeCleanup: true }) + const tmpdir = join(__dirname, '..', 'tmp') + await ensureDir(tmpdir) + const tmpDir = await getTmpDir({ unsafeCleanup: true, tmpdir }) restoreCwd = changeCwd(tmpDir.path) cleanup = tmpDir.cleanup From a521dfb60ac9826efe8626563c0bafdfcfb0bd86 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Fri, 23 Jun 2023 15:15:20 +0200 Subject: [PATCH 04/12] fix: update snapshot --- test/__snapshots__/index.spec.ts.snap | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/__snapshots__/index.spec.ts.snap b/test/__snapshots__/index.spec.ts.snap index 1d60432185..240e0d780c 100644 --- a/test/__snapshots__/index.spec.ts.snap +++ b/test/__snapshots__/index.spec.ts.snap @@ -2117,12 +2117,12 @@ Array [ "to": "/.netlify/functions/___netlify-api-handler", }, Object { - "from": "/api/shows/:id", + "from": "/api/shows/:params/*", "status": 200, "to": "/.netlify/functions/___netlify-api-handler", }, Object { - "from": "/api/shows/:params/*", + "from": "/api/shows/:show-id", "status": 200, "to": "/.netlify/functions/___netlify-api-handler", }, From 64aa932ec45135fdfbb4e19dcd79bc5798b75c42 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Fri, 23 Jun 2023 15:20:12 +0200 Subject: [PATCH 05/12] fix: displayname test --- test/index.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/index.spec.ts b/test/index.spec.ts index d5da0b207f..1b9986379d 100644 --- a/test/index.spec.ts +++ b/test/index.spec.ts @@ -750,7 +750,7 @@ describe('onBuild()', () => { expect(functionsManifest).toEqual({ config: { - generator: '@netlify/next-runtime@unknown', + generator: expect.stringContaining('@netlify/next-runtime@'), name: 'Next.js API handler', }, version: 1, From 02802905c970af3624d991837056006c4ca535d3 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 26 Jun 2023 13:33:06 +0200 Subject: [PATCH 06/12] fix: subdir test should have node_modules in same relative position --- test/index.spec.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/index.spec.ts b/test/index.spec.ts index 1b9986379d..95b2094a3e 100644 --- a/test/index.spec.ts +++ b/test/index.spec.ts @@ -4,7 +4,7 @@ import process from 'process' import type { NetlifyPluginOptions } from '@netlify/build' import Chance from 'chance' -import { writeJSON, unlink, existsSync, readFileSync, ensureDir, readJson, pathExists, writeFile, move } from 'fs-extra' +import { writeJSON, unlink, existsSync, readFileSync, ensureDir, readJson, pathExists, writeFile, move, symlink } from 'fs-extra' import { join, relative } from 'pathe' import { dir as getTmpDir } from 'tmp-promise' @@ -525,6 +525,9 @@ describe('onBuild()', () => { it('generates a file referencing all when publish dir is a subdirectory', async () => { const dir = 'web/.next' await moveNextDist(dir) + const symlinkPath = join(__dirname, '..', 'tmp', 'node_modules') + + await symlink(join(__dirname, '..', 'node_modules'), symlinkPath) netlifyConfig.build.publish = path.resolve(dir) const config = { @@ -538,6 +541,8 @@ describe('onBuild()', () => { expect(normalizeChunkNames(readFileSync(handlerPagesFile, 'utf8'))).toMatchSnapshot() expect(normalizeChunkNames(readFileSync(odbHandlerPagesFile, 'utf8'))).toMatchSnapshot() + + await unlink(symlinkPath) }) it('generates entrypoints with correct references', async () => { From acc23faaeae177474e173cc18a3a1127d70e6e08 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Wed, 28 Jun 2023 09:37:38 +0200 Subject: [PATCH 07/12] fix: subdir test should have node_modules in same relative position --- test/index.spec.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/test/index.spec.ts b/test/index.spec.ts index 95b2094a3e..3f0b415b49 100644 --- a/test/index.spec.ts +++ b/test/index.spec.ts @@ -4,7 +4,18 @@ import process from 'process' import type { NetlifyPluginOptions } from '@netlify/build' import Chance from 'chance' -import { writeJSON, unlink, existsSync, readFileSync, ensureDir, readJson, pathExists, writeFile, move, symlink } from 'fs-extra' +import { + writeJSON, + unlink, + existsSync, + readFileSync, + ensureDir, + readJson, + pathExists, + writeFile, + move, + symlink, +} from 'fs-extra' import { join, relative } from 'pathe' import { dir as getTmpDir } from 'tmp-promise' From 598756b88394976db321026397b7496ad6ffd5d5 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Thu, 13 Jul 2023 12:36:38 +0200 Subject: [PATCH 08/12] fix: revert test changes --- test/index.spec.ts | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/test/index.spec.ts b/test/index.spec.ts index 3f0b415b49..038dfbde64 100644 --- a/test/index.spec.ts +++ b/test/index.spec.ts @@ -4,18 +4,7 @@ import process from 'process' import type { NetlifyPluginOptions } from '@netlify/build' import Chance from 'chance' -import { - writeJSON, - unlink, - existsSync, - readFileSync, - ensureDir, - readJson, - pathExists, - writeFile, - move, - symlink, -} from 'fs-extra' +import { writeJSON, unlink, existsSync, readFileSync, ensureDir, readJson, pathExists, writeFile, move } from 'fs-extra' import { join, relative } from 'pathe' import { dir as getTmpDir } from 'tmp-promise' @@ -86,9 +75,7 @@ let cleanup // In each test, we change cwd to a temporary directory. // This allows us not to have to mock filesystem operations. beforeEach(async () => { - const tmpdir = join(__dirname, '..', 'tmp') - await ensureDir(tmpdir) - const tmpDir = await getTmpDir({ unsafeCleanup: true, tmpdir }) + const tmpDir = await getTmpDir({ unsafeCleanup: true }) restoreCwd = changeCwd(tmpDir.path) cleanup = tmpDir.cleanup @@ -536,9 +523,6 @@ describe('onBuild()', () => { it('generates a file referencing all when publish dir is a subdirectory', async () => { const dir = 'web/.next' await moveNextDist(dir) - const symlinkPath = join(__dirname, '..', 'tmp', 'node_modules') - - await symlink(join(__dirname, '..', 'node_modules'), symlinkPath) netlifyConfig.build.publish = path.resolve(dir) const config = { @@ -552,8 +536,6 @@ describe('onBuild()', () => { expect(normalizeChunkNames(readFileSync(handlerPagesFile, 'utf8'))).toMatchSnapshot() expect(normalizeChunkNames(readFileSync(odbHandlerPagesFile, 'utf8'))).toMatchSnapshot() - - await unlink(symlinkPath) }) it('generates entrypoints with correct references', async () => { @@ -766,7 +748,7 @@ describe('onBuild()', () => { expect(functionsManifest).toEqual({ config: { - generator: expect.stringContaining('@netlify/next-runtime@'), + generator: '@netlify/next-runtime@unknown', name: 'Next.js API handler', }, version: 1, From 62a24179b1f6fef056a58ee5c0dda00dcf7bada4 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 17 Jul 2023 10:50:18 +0200 Subject: [PATCH 09/12] refactor: move getBundleWeight to utils --- packages/runtime/src/helpers/functions.ts | 21 ++------------------- packages/runtime/src/helpers/utils.ts | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/packages/runtime/src/helpers/functions.ts b/packages/runtime/src/helpers/functions.ts index 5880613908..47dcd274bf 100644 --- a/packages/runtime/src/helpers/functions.ts +++ b/packages/runtime/src/helpers/functions.ts @@ -2,7 +2,7 @@ import type { NetlifyConfig, NetlifyPluginConstants } from '@netlify/build' import bridgeFile from '@vercel/node-bridge' import chalk from 'chalk' import destr from 'destr' -import { copyFile, ensureDir, existsSync, readJSON, writeFile, writeJSON, stat } from 'fs-extra' +import { copyFile, ensureDir, existsSync, readJSON, writeFile, writeJSON } from 'fs-extra' import { PrerenderManifest } from 'next/dist/build' import type { ImageConfigComplete, RemotePattern } from 'next/dist/shared/lib/image-config' import { outdent } from 'outdent' @@ -31,7 +31,7 @@ import { getDependenciesOfFile, getServerFile, getSourceFileForPage } from './fi import { writeFunctionConfiguration } from './functionsMetaData' import { pack } from './pack' import { ApiRouteType } from './types' -import { getFunctionNameForPage } from './utils' +import { getBundleWeight, getFunctionNameForPage } from './utils' export interface RouteConfig { functionName: string @@ -326,23 +326,6 @@ export const getCommonDependencies = async (publish: string) => { return deps.flat(1) } -const sum = (arr: number[]) => arr.reduce((v, current) => v + current, 0) - -// TODO: cache results -const getBundleWeight = async (files: string[]) => { - const sizes = await Promise.all( - files.map(async (file) => { - const fStat = await stat(file) - if (fStat.isFile()) { - return fStat.size - } - return 0 - }), - ) - - return sum(sizes.flat(1)) -} - const changeExtension = (file: string, extension: string) => { const base = basename(file, extname(file)) return join(dirname(file), base + extension) diff --git a/packages/runtime/src/helpers/utils.ts b/packages/runtime/src/helpers/utils.ts index 386c6f91ac..0513b5aee7 100644 --- a/packages/runtime/src/helpers/utils.ts +++ b/packages/runtime/src/helpers/utils.ts @@ -1,5 +1,6 @@ import type { NetlifyConfig } from '@netlify/build' import type { Header } from '@netlify/build/types/config/netlify_config' +import { stat } from 'fs-extra' import globby from 'globby' import type { ExperimentalConfig } from 'next/dist/server/config-shared' import type { ImageConfigComplete, RemotePattern } from 'next/dist/shared/lib/image-config' @@ -327,3 +328,20 @@ export const getRemotePatterns = (experimental: ExperimentalConfigWithLegacy, im const reHasRegExp = /[|\\{}()[\]^$+*?.-]/ const reReplaceRegExp = /[|\\{}()[\]^$+*?.-]/g export const escapeStringRegexp = (str: string) => (reHasRegExp.test(str) ? str.replace(reReplaceRegExp, '\\$&') : str) + +const sum = (arr: number[]) => arr.reduce((v, current) => v + current, 0) + +// TODO: cache results +export const getBundleWeight = async (files: string[]) => { + const sizes = await Promise.all( + files.map(async (file) => { + const fStat = await stat(file) + if (fStat.isFile()) { + return fStat.size + } + return 0 + }), + ) + + return sum(sizes.flat(1)) +} \ No newline at end of file From 864ce5d58fc095ead0bab01d1db2756eaf854b62 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 17 Jul 2023 10:51:25 +0200 Subject: [PATCH 10/12] fix: mock FS access --- packages/runtime/src/helpers/utils.ts | 2 +- test/index.spec.ts | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/runtime/src/helpers/utils.ts b/packages/runtime/src/helpers/utils.ts index 0513b5aee7..0299854e8c 100644 --- a/packages/runtime/src/helpers/utils.ts +++ b/packages/runtime/src/helpers/utils.ts @@ -344,4 +344,4 @@ export const getBundleWeight = async (files: string[]) => { ) return sum(sizes.flat(1)) -} \ No newline at end of file +} diff --git a/test/index.spec.ts b/test/index.spec.ts index 038dfbde64..4b12b66c80 100644 --- a/test/index.spec.ts +++ b/test/index.spec.ts @@ -18,9 +18,12 @@ import { getAllPageDependencies } from '../packages/runtime/src/templates/getPag import { changeCwd, useFixture, moveNextDist } from './test-utils' +const getBundleWeightMock = jest.fn().mockReturnValue(0) + jest.mock('../packages/runtime/src/helpers/utils', () => ({ ...jest.requireActual('../packages/runtime/src/helpers/utils'), isNextAuthInstalled: jest.fn(), + getBundleWeight: getBundleWeightMock, })) jest.mock('../packages/runtime/src/helpers/functionsMetaData', () => { From 9ee560bde0ef6d8f4617eeb3467e3097e880855b Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 17 Jul 2023 11:25:19 +0200 Subject: [PATCH 11/12] fix: move mock in-scope --- test/index.spec.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/index.spec.ts b/test/index.spec.ts index 4b12b66c80..f1b3c82edf 100644 --- a/test/index.spec.ts +++ b/test/index.spec.ts @@ -18,12 +18,10 @@ import { getAllPageDependencies } from '../packages/runtime/src/templates/getPag import { changeCwd, useFixture, moveNextDist } from './test-utils' -const getBundleWeightMock = jest.fn().mockReturnValue(0) - jest.mock('../packages/runtime/src/helpers/utils', () => ({ ...jest.requireActual('../packages/runtime/src/helpers/utils'), isNextAuthInstalled: jest.fn(), - getBundleWeight: getBundleWeightMock, + getBundleWeight: jest.fn().mockReturnValue(0), })) jest.mock('../packages/runtime/src/helpers/functionsMetaData', () => { From 081b7c0a45c3a8afd4703dd61d12a5052928daa3 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 17 Jul 2023 15:03:24 +0200 Subject: [PATCH 12/12] fix: smh, jest.fn() sometimes returns undefined --- test/index.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/index.spec.ts b/test/index.spec.ts index f1b3c82edf..3f66fa459c 100644 --- a/test/index.spec.ts +++ b/test/index.spec.ts @@ -21,7 +21,7 @@ import { changeCwd, useFixture, moveNextDist } from './test-utils' jest.mock('../packages/runtime/src/helpers/utils', () => ({ ...jest.requireActual('../packages/runtime/src/helpers/utils'), isNextAuthInstalled: jest.fn(), - getBundleWeight: jest.fn().mockReturnValue(0), + getBundleWeight: async () => 0, })) jest.mock('../packages/runtime/src/helpers/functionsMetaData', () => {