perf: shim next's telemetry module to improve startup performance (#365)

* perf: shim next's telemetry module to improve startup performance

* Update src/build/content/server.ts

Co-authored-by: Matt Kane <[email protected]>

* fix: don't break the build if failed to apply perf patch

* feat: also run on latest canaries

* test: setup next patching for tests

* test: refactor version checking a bit to allow canaries for ongoing always

* test: update var names in tests, make potential assertion failures more readable by checking object with version so failure would log version as well

* tmp: make minVersion lower to check that tests would suggest bumping it

* test: add test checking that patch is being applied and wether upper bound should be bumped

* test: normalize path for win32 (?)

* test: normalize path for win32 (vol2)

* fix: max version

* chore: specify type of patch in error message

---------

Co-authored-by: Matt Kane <[email protected]>

Author: pieh

src/build/content/next-shims/telemetry-storage.cts

@@ -0,0 +1,46 @@
+import type { Telemetry } from 'next/dist/telemetry/storage.js'
+
+type PublicOf<T> = { [K in keyof T]: T[K] }
+
+export class TelemetryShim implements PublicOf<Telemetry> {
+  sessionId = 'shim'
+
+  get anonymousId(): string {
+    return 'shim'
+  }
+
+  get salt(): string {
+    return 'shim'
+  }
+
+  setEnabled(): string | null {
+    return null
+  }
+
+  get isEnabled(): boolean {
+    return false
+  }
+
+  oneWayHash(): string {
+    return 'shim'
+  }
+
+  record(): Promise<{
+    isFulfilled: boolean
+    isRejected: boolean
+    value?: unknown
+    reason?: unknown
+  }> {
+    return Promise.resolve({ isFulfilled: true, isRejected: false })
+  }
+
+  flush(): Promise<
+    { isFulfilled: boolean; isRejected: boolean; value?: unknown; reason?: unknown }[] | null
+  > {
+    return Promise.resolve(null)
+  }
+
+  flushDetached(): void {
+    // no-op
+  }
+}

src/build/content/server.test.ts

@@ -7,7 +7,7 @@ import { expect, test, vi, describe, beforeEach } from 'vitest'
 import { mockFileSystem } from '../../../tests/index.js'
 import { PluginContext, RequiredServerFilesManifest } from '../plugin-context.js'
 
-import { copyNextServerCode, verifyHandlerDirStructure } from './server.js'
+import { copyNextServerCode, getPatchesToApply, verifyHandlerDirStructure } from './server.js'
 
 vi.mock('node:fs', async () => {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any, unicorn/no-await-expression-member
@@ -267,3 +267,111 @@ describe('verifyHandlerDirStructure', () => {
     )
   })
 })
+
+describe(`getPatchesToApply`, () => {
+  beforeEach(() => {
+    delete process.env.NETLIFY_NEXT_FORCE_APPLY_ONGOING_PATCHES
+  })
+  test('ongoing: false', () => {
+    const shouldPatchBeApplied = {
+      '13.4.1': false, // before supported next version
+      '13.5.1': true, // first stable supported version
+      '14.1.4-canary.1': true, // canary version before stable with maxStableVersion - should be applied
+      '14.1.4': true, // latest stable tested version
+      '14.2.0': false, // untested stable version
+      '14.2.0-canary.38': false, // not ongoing patch so should not be applied
+    }
+
+    const nextModule = 'test'
+
+    const patches = [
+      {
+        ongoing: false,
+        minVersion: '13.5.0-canary.0',
+        maxStableVersion: '14.1.4',
+        nextModule,
+        shimModule: 'not-used-in-test',
+      },
+    ]
+
+    for (const [nextVersion, telemetryShimShouldBeApplied] of Object.entries(
+      shouldPatchBeApplied,
+    )) {
+      const patchesToApply = getPatchesToApply(nextVersion, patches)
+      const hasTelemetryShim = patchesToApply.some((patch) => patch.nextModule === nextModule)
+      expect({ nextVersion, apply: hasTelemetryShim }).toEqual({
+        nextVersion,
+        apply: telemetryShimShouldBeApplied,
+      })
+    }
+  })
+
+  test('ongoing: true', () => {
+    const shouldPatchBeApplied = {
+      '13.4.1': false, // before supported next version
+      '13.5.1': true, // first stable supported version
+      '14.1.4-canary.1': true, // canary version before stable with maxStableVersion - should be applied
+      '14.1.4': true, // latest stable tested version
+      '14.2.0': false, // untested stable version
+      '14.2.0-canary.38': true, // ongoing patch so should be applied on prerelease versions
+    }
+
+    const nextModule = 'test'
+
+    const patches = [
+      {
+        ongoing: true,
+        minVersion: '13.5.0-canary.0',
+        maxStableVersion: '14.1.4',
+        nextModule,
+        shimModule: 'not-used-in-test',
+      },
+    ]
+
+    for (const [nextVersion, telemetryShimShouldBeApplied] of Object.entries(
+      shouldPatchBeApplied,
+    )) {
+      const patchesToApply = getPatchesToApply(nextVersion, patches)
+      const hasTelemetryShim = patchesToApply.some((patch) => patch.nextModule === nextModule)
+      expect({ nextVersion, apply: hasTelemetryShim }).toEqual({
+        nextVersion,
+        apply: telemetryShimShouldBeApplied,
+      })
+    }
+  })
+
+  test('ongoing: true + NETLIFY_NEXT_FORCE_APPLY_ONGOING_PATCHES', () => {
+    process.env.NETLIFY_NEXT_FORCE_APPLY_ONGOING_PATCHES = 'true'
+    const shouldPatchBeApplied = {
+      '13.4.1': false, // before supported next version
+      '13.5.1': true, // first stable supported version
+      '14.1.4-canary.1': true, // canary version before stable with maxStableVersion - should be applied
+      '14.1.4': true, // latest stable tested version
+      '14.2.0': true, // untested stable version but NETLIFY_NEXT_FORCE_APPLY_ONGOING_PATCHES is used
+      '14.2.0-canary.38': true, // ongoing patch so should be applied on prerelease versions
+    }
+
+    const nextModule = 'test'
+
+    const patches = [
+      {
+        ongoing: true,
+        minVersion: '13.5.0-canary.0',
+        maxStableVersion: '14.1.4',
+        nextModule,
+        shimModule: 'not-used-in-test',
+      },
+    ]
+
+    for (const [nextVersion, telemetryShimShouldBeApplied] of Object.entries(
+      shouldPatchBeApplied,
+    )) {
+      const patchesToApply = getPatchesToApply(nextVersion, patches)
+      const hasTelemetryShim = patchesToApply.some((patch) => patch.nextModule === nextModule)
+      expect({ nextVersion, apply: hasTelemetryShim }).toEqual({
+        nextVersion,
+        apply: telemetryShimShouldBeApplied,
+      })
+    }
+  })
+})

src/build/content/server.ts

@@ -15,6 +15,7 @@ import { dirname, join, resolve, sep } from 'node:path'
 import { sep as posixSep, join as posixJoin } from 'node:path/posix'
 
 import glob from 'fast-glob'
+import { prerelease, lt as semverLowerThan, lte as semverLowerThanOrEqual } from 'semver'
 
 import { RUN_CONFIG } from '../../run/constants.js'
 import { PluginContext } from '../plugin-context.js'
@@ -149,6 +150,93 @@ async function recreateNodeModuleSymlinks(src: string, dest: string, org?: strin
   )
 }
 
+export type NextInternalModuleReplacement = {
+  /**
+   * Minimum Next.js version that this patch should be applied to
+   */
+  minVersion: string
+  /**
+   * Maximum Next.js version that this patch should be applied to, note that for ongoing patches
+   * we will continue to apply patch for prerelease versions also as canary versions are released
+   * very frequently and trying to target canary versions is not practical. If user is using
+   * canary next versions they should be aware of the risks
+   */
+  maxStableVersion: string
+  /**
+   * If the reason to patch was not addressed in Next.js we mark this as ongoing
+   * to continue to test latest versions to know wether we should bump `maxStableVersion`
+   */
+  ongoing?: boolean
+  /**
+   * Module that should be replaced
+   */
+  nextModule: string
+  /**
+   * Location of replacement module (relative to `<runtime>/dist/build/content`)
+   */
+  shimModule: string
+}
+
+const nextInternalModuleReplacements: NextInternalModuleReplacement[] = [
+  {
+    // standalone is loading expensive Telemetry module that is not actually used
+    // so this replace that module with lightweight no-op shim that doesn't load additional modules
+    // see https://github.com/vercel/next.js/pull/63574 that will remove need for this shim
+    ongoing: true,
+    minVersion: '13.5.0-canary.0',
+    maxStableVersion: '14.1.4',
+    nextModule: 'next/dist/telemetry/storage.js',
+    shimModule: './next-shims/telemetry-storage.cjs',
+  },
+]
+
+export function getPatchesToApply(
+  nextVersion: string,
+  patches: NextInternalModuleReplacement[] = nextInternalModuleReplacements,
+) {
+  return patches.filter(({ minVersion, maxStableVersion, ongoing }) => {
+    // don't apply patches for next versions below minVersion
+    if (semverLowerThan(nextVersion, minVersion)) {
+      return false
+    }
+
+    // apply ongoing patches when used next version is prerelease or NETLIFY_NEXT_FORCE_APPLY_ONGOING_PATCHES env var is used
+    if (
+      ongoing &&
+      (prerelease(nextVersion) || process.env.NETLIFY_NEXT_FORCE_APPLY_ONGOING_PATCHES)
+    ) {
+      return true
+    }
+
+    // apply patches for next versions below maxStableVersion
+    return semverLowerThanOrEqual(nextVersion, maxStableVersion)
+  })
+}
+
+async function patchNextModules(
+  ctx: PluginContext,
+  nextVersion: string,
+  serverHandlerRequireResolve: NodeRequire['resolve'],
+): Promise<void> {
+  // apply only those patches that target used Next version
+  const moduleReplacementsToApply = getPatchesToApply(nextVersion)
+
+  if (moduleReplacementsToApply.length !== 0) {
+    await Promise.all(
+      moduleReplacementsToApply.map(async ({ nextModule, shimModule }) => {
+        try {
+          const nextModulePath = serverHandlerRequireResolve(nextModule)
+          const shimModulePath = posixJoin(ctx.pluginDir, 'dist', 'build', 'content', shimModule)
+
+          await cp(shimModulePath, nextModulePath, { force: true })
+        } catch {
+          // this is perf optimization, so failing it shouldn't break the build
+        }
+      }),
+    )
+  }
+}
+
 export const copyNextDependencies = async (ctx: PluginContext): Promise<void> => {
   const entries = await readdir(ctx.standaloneDir)
   const promises: Promise<void>[] = entries.map(async (entry) => {
@@ -199,6 +287,8 @@ export const copyNextDependencies = async (ctx: PluginContext): Promise<void> =>
 
   if (nextVersion) {
     verifyNextVersion(ctx, nextVersion)
+
+    await patchNextModules(ctx, nextVersion, serverHandlerRequire.resolve)
   }
 
   try {

tests/integration/simple-app.test.ts

@@ -1,12 +1,17 @@
 import { load } from 'cheerio'
 import { getLogger } from 'lambda-local'
+import { cp } from 'node:fs/promises'
+import { createRequire } from 'node:module'
+import { join } from 'node:path'
 import { gunzipSync } from 'node:zlib'
+import { gt, prerelease } from 'semver'
 import { v4 } from 'uuid'
-import { beforeEach, describe, expect, test, vi } from 'vitest'
+import { Mock, afterAll, beforeAll, beforeEach, describe, expect, test, vi } from 'vitest'
 import {
   createFixture,
   invokeFunction,
   runPlugin,
+  getFixtureSourceDirectory,
   type FixtureTestContext,
 } from '../utils/fixture.js'
 import {
@@ -15,6 +20,20 @@ import {
   getBlobEntries,
   startMockBlobStore,
 } from '../utils/helpers.js'
+import { getPatchesToApply } from '../../src/build/content/server.js'
+
+const mockedCp = cp as Mock<
+  Parameters<(typeof import('node:fs/promises'))['cp']>,
+  ReturnType<(typeof import('node:fs/promises'))['cp']>
+>
+
+vi.mock('node:fs/promises', async (importOriginal) => {
+  const fsPromisesModule = (await importOriginal()) as typeof import('node:fs/promises')
+  return {
+    ...fsPromisesModule,
+    cp: vi.fn(fsPromisesModule.cp.bind(fsPromisesModule)),
+  }
+})
 
 // Disable the verbose logging of the lambda-local runtime
 getLogger().level = 'alert'
@@ -183,3 +202,78 @@ test<FixtureTestContext>('Test that a simple next app with PPR is working', asyn
   expect(home.statusCode).toBe(200)
   expect(load(home.body)('h1').text()).toBe('Home')
 })
+
+describe('next patching', async () => {
+  const { cp: originalCp, appendFile } = (await vi.importActual(
+    'node:fs/promises',
+  )) as typeof import('node:fs/promises')
+
+  const { version: nextVersion } = createRequire(
+    `${getFixtureSourceDirectory('simple-next-app')}/:internal:`,
+  )('next/package.json')
+
+  beforeAll(() => {
+    process.env.NETLIFY_NEXT_FORCE_APPLY_ONGOING_PATCHES = 'true'
+  })
+
+  afterAll(() => {
+    delete process.env.NETLIFY_NEXT_FORCE_APPLY_ONGOING_PATCHES
+  })
+
+  beforeEach(() => {
+    mockedCp.mockClear()
+    mockedCp.mockRestore()
+  })
+
+  test<FixtureTestContext>(`expected patches are applied and used (next version: "${nextVersion}")`, async (ctx) => {
+    const patches = getPatchesToApply(nextVersion)
+
+    await createFixture('simple-next-app', ctx)
+
+    const fieldNamePrefix = `TEST_${Date.now()}`
+
+    mockedCp.mockImplementation(async (...args) => {
+      const returnValue = await originalCp(...args)
+      if (typeof args[1] === 'string') {
+        for (const patch of patches) {
+          if (args[1].includes(join(patch.nextModule))) {
+            // we append something to assert that patch file was actually used
+            await appendFile(
+              args[1],
+              `;globalThis['${fieldNamePrefix}_${patch.nextModule}'] = 'patched'`,
+            )
+          }
+        }
+      }
+
+      return returnValue
+    })
+
+    await runPlugin(ctx)
+
+    // patched files was not used before function invocation
+    for (const patch of patches) {
+      expect(globalThis[`${fieldNamePrefix}_${patch.nextModule}`]).not.toBeDefined()
+    }
+
+    const home = await invokeFunction(ctx)
+    // make sure the function does work
+    expect(home.statusCode).toBe(200)
+    expect(load(home.body)('h1').text()).toBe('Home')
+
+    let shouldUpdateUpperBoundMessage = ''
+
+    // file was used during function invocation
+    for (const patch of patches) {
+      expect(globalThis[`${fieldNamePrefix}_${patch.nextModule}`]).toBe('patched')
+
+      if (patch.ongoing && !prerelease(nextVersion) && gt(nextVersion, patch.maxStableVersion)) {
+        shouldUpdateUpperBoundMessage += `Ongoing ${shouldUpdateUpperBoundMessage ? '\n' : ''}"${patch.nextModule}" patch still works on "${nextVersion}" which is higher than currently set maxStableVersion ("${patch.maxStableVersion}"). Update maxStableVersion in "src/build/content/server.ts" for this patch to at least "${nextVersion}".`
+      }
+    }
+
+    if (shouldUpdateUpperBoundMessage) {
+      expect.fail(shouldUpdateUpperBoundMessage)
+    }
+  })
+})