From c7a95c3d700f276e938137728b5ac5a6af2bd7bb Mon Sep 17 00:00:00 2001 From: Mathusan Selvarajah <mathusan@google.com> Date: Tue, 1 Apr 2025 16:21:48 +0000 Subject: [PATCH 01/10] progress --- .../adapter-nextjs/e2e/run-local.ts | 9 +++- .../adapter-nextjs/src/overrides.spec.ts | 51 +++++++++++-------- .../adapter-nextjs/src/overrides.ts | 38 ++++++++++++-- 3 files changed, 73 insertions(+), 25 deletions(-) diff --git a/packages/@apphosting/adapter-nextjs/e2e/run-local.ts b/packages/@apphosting/adapter-nextjs/e2e/run-local.ts index 23104afa..4c09dd66 100644 --- a/packages/@apphosting/adapter-nextjs/e2e/run-local.ts +++ b/packages/@apphosting/adapter-nextjs/e2e/run-local.ts @@ -53,10 +53,9 @@ const scenarios: Scenario[] = [ tests: ["middleware.spec.ts"], // Only run middleware-specific tests }, ...configOverrideTestScenarios.map( - (scenario: { name: string; config: string; file: string }) => ({ + (scenario: { name: string; config?: string; file?: string }) => ({ name: scenario.name, setup: async (cwd: string) => { - const configContent = scenario.config; const files = await fsExtra.readdir(cwd); const configFiles = files .filter((file) => file.startsWith("next.config.")) @@ -67,6 +66,12 @@ const scenarios: Scenario[] = [ console.log(`Removed existing config file: ${file}`); } + // skip creating the test config if data is not provided + if (!scenario.config || !scenario.file) { + return + } + + const configContent = scenario.config; await fsExtra.writeFile(join(cwd, scenario.file), configContent); console.log(`Created ${scenario.file} file with ${scenario.name} config`); }, diff --git a/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts b/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts index 3d478b18..2af93b1d 100644 --- a/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts +++ b/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts @@ -172,13 +172,13 @@ describe("next config overrides", () => { ...config, images: { ...(config.images || {}), - ...(config.images?.unoptimized === undefined && config.images?.loader === undefined - ? { unoptimized: true } + ...(config.images?.unoptimized === undefined && config.images?.loader === undefined + ? { unoptimized: true } : {}), }, }); - const config = typeof originalConfig === 'function' + const config = typeof originalConfig === 'function' ? async (...args) => { const resolvedConfig = await originalConfig(...args); return fahOptimizedConfig(resolvedConfig); @@ -186,6 +186,18 @@ describe("next config overrides", () => { : fahOptimizedConfig(originalConfig); `; + const defaultNextConfig = ` + // @ts-nocheck + + /** @type {import('next').NextConfig} */ + const nextConfig = { + images: { + unoptimized: true, + } + } + + module.exports = nextConfig + ` beforeEach(() => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "test-overrides")); }); @@ -194,12 +206,12 @@ describe("next config overrides", () => { const { overrideNextConfig } = await importOverrides; const originalConfig = ` // @ts-check - + /** @type {import('next').NextConfig} */ const nextConfig = { /* config options here */ } - + module.exports = nextConfig `; @@ -213,7 +225,7 @@ describe("next config overrides", () => { normalizeWhitespace(` // @ts-nocheck const originalConfig = require('./next.config.original.js'); - + ${nextConfigOverrideBody} module.exports = config; @@ -225,14 +237,14 @@ describe("next config overrides", () => { const { overrideNextConfig } = await importOverrides; const originalConfig = ` // @ts-check - + /** * @type {import('next').NextConfig} */ const nextConfig = { /* config options here */ } - + export default nextConfig `; @@ -257,7 +269,7 @@ describe("next config overrides", () => { const { overrideNextConfig } = await importOverrides; const originalConfig = ` // @ts-check - + export default (phase, { defaultConfig }) => { /** * @type {import('next').NextConfig} @@ -280,7 +292,7 @@ describe("next config overrides", () => { import originalConfig from './next.config.original.mjs'; ${nextConfigOverrideBody} - + export default config; `), ); @@ -290,11 +302,11 @@ describe("next config overrides", () => { const { overrideNextConfig } = await importOverrides; const originalConfig = ` import type { NextConfig } from 'next' - + const nextConfig: NextConfig = { /* config options here */ } - + export default nextConfig `; @@ -307,22 +319,21 @@ describe("next config overrides", () => { normalizeWhitespace(` // @ts-nocheck import originalConfig from './next.config.original'; - + ${nextConfigOverrideBody} - + module.exports = config; `), ); }); - it("should not do anything if no next.config.* file exists", async () => { + it("should create a default next.config.js file if one does not exist yet", async () => { const { overrideNextConfig } = await importOverrides; await overrideNextConfig(tmpDir, "next.config.js"); - - // Assert that no next.config* files were created - const files = fs.readdirSync(tmpDir); - const nextConfigFiles = files.filter((file) => file.startsWith("next.config")); - assert.strictEqual(nextConfigFiles.length, 0, "No next.config files should exist"); + const updatedConfig = fs.readFileSync(path.join(tmpDir, "next.config.js"), "utf-8"); + assert.equal( + normalizeWhitespace(updatedConfig), normalizeWhitespace(defaultNextConfig), + ); }); }); diff --git a/packages/@apphosting/adapter-nextjs/src/overrides.ts b/packages/@apphosting/adapter-nextjs/src/overrides.ts index d680e319..9fdce496 100644 --- a/packages/@apphosting/adapter-nextjs/src/overrides.ts +++ b/packages/@apphosting/adapter-nextjs/src/overrides.ts @@ -10,6 +10,8 @@ import { import { join, extname } from "path"; import { rename as renamePromise } from "fs/promises"; +const DEFAULT_NEXT_CONFIG_FILE = 'next.config.js' + /** * Overrides the user's Next Config file (next.config.[ts|js|mjs]) to add configs * optimized for Firebase App Hosting. @@ -19,11 +21,21 @@ export async function overrideNextConfig(projectRoot: string, nextConfigFileName // Check if the file exists in the current working directory const configPath = join(projectRoot, nextConfigFileName); - if (!(await exists(configPath))) { - console.log(`No Next.js config file found at ${configPath}`); - return; + if (!(await exists(configPath))){ + console.log(`No Next config file found at ${configPath}, creating one with Firebase App Hosting overrides`); + try { + await writeFile(join(projectRoot, DEFAULT_NEXT_CONFIG_FILE), defaultNextConfigForFAH()); + } catch (error) { + console.error(`Error creating ${DEFAULT_NEXT_CONFIG_FILE}: ${error}`); + throw error; + } + + console.log(`Successfully created ${DEFAULT_NEXT_CONFIG_FILE} with Firebase App Hosting overrides`); + return } + // A Next Config already exists in the user's project, so it needs to be overriden + // Determine the file extension const fileExtension = extname(nextConfigFileName); const originalConfigName = `next.config.original${fileExtension}`; @@ -104,6 +116,26 @@ function getCustomNextConfig(importStatement: string, fileExtension: string) { `; } +/** + * Returns the default Next Config file that is created in the user's project + * if one does not exist already. This config ensures the Next.Js app is optimized + * for Firebase App Hosting. + */ +function defaultNextConfigForFAH() { + return ` + // @ts-nocheck + + /** @type {import('next').NextConfig} */ + const nextConfig = { + images: { + unoptimized: true, + } + } + + module.exports = nextConfig + ` +} + /** * This function is used to validate the state of an app after running the * overrideNextConfig. It validates that: From b1da3b225192f8b3893b0da5087c04bb7702f4aa Mon Sep 17 00:00:00 2001 From: Mathusan Selvarajah <mathusan@google.com> Date: Tue, 1 Apr 2025 20:27:59 +0000 Subject: [PATCH 02/10] fix implementation and tests --- .../e2e/config-override-test-cases.yaml | 1 + .../e2e/config-override.spec.ts | 4 +-- .../adapter-nextjs/src/bin/build.ts | 7 ++-- .../adapter-nextjs/src/overrides.spec.ts | 23 +++--------- .../adapter-nextjs/src/overrides.ts | 36 ++++++++----------- 5 files changed, 24 insertions(+), 47 deletions(-) diff --git a/packages/@apphosting/adapter-nextjs/e2e/config-override-test-cases.yaml b/packages/@apphosting/adapter-nextjs/e2e/config-override-test-cases.yaml index ee40c3b3..8802f395 100644 --- a/packages/@apphosting/adapter-nextjs/e2e/config-override-test-cases.yaml +++ b/packages/@apphosting/adapter-nextjs/e2e/config-override-test-cases.yaml @@ -200,3 +200,4 @@ tests: module.exports = nextConfig; file: next.config.js + - name: without-a-next-config diff --git a/packages/@apphosting/adapter-nextjs/e2e/config-override.spec.ts b/packages/@apphosting/adapter-nextjs/e2e/config-override.spec.ts index a2ab5beb..d98c4e62 100644 --- a/packages/@apphosting/adapter-nextjs/e2e/config-override.spec.ts +++ b/packages/@apphosting/adapter-nextjs/e2e/config-override.spec.ts @@ -30,7 +30,7 @@ const compiledFilesPath = posix.join( const requiredServerFilePath = posix.join(compiledFilesPath, "required-server-files.json"); describe("next.config override", () => { - it("should have images optimization disabled", async function () { + it("should have image optimization disabled", async function () { if ( scenario.includes("with-empty-config") || scenario.includes("with-images-unoptimized-false") || @@ -53,7 +53,7 @@ describe("next.config override", () => { }); it("should preserve other user set next configs", async function () { - if (scenario.includes("with-empty-config")) { + if (scenario.includes("with-empty-config") || scenario.includes("without-a-next-config")) { // eslint-disable-next-line @typescript-eslint/no-invalid-this this.skip(); } diff --git a/packages/@apphosting/adapter-nextjs/src/bin/build.ts b/packages/@apphosting/adapter-nextjs/src/bin/build.ts index eedb6bbb..6efe0923 100644 --- a/packages/@apphosting/adapter-nextjs/src/bin/build.ts +++ b/packages/@apphosting/adapter-nextjs/src/bin/build.ts @@ -35,11 +35,8 @@ const originalConfig = await loadConfig(root, opts.projectDirectory); * Note: loadConfig always returns a fileName (default: next.config.js) even if * one does not exist in the app's root: https://github.com/vercel/next.js/blob/23681508ca34b66a6ef55965c5eac57de20eb67f/packages/next/src/server/config.ts#L1115 */ -const originalConfigPath = join(root, originalConfig.configFileName); -if (await exists(originalConfigPath)) { - await overrideNextConfig(root, originalConfig.configFileName); - await validateNextConfigOverride(root, opts.projectDirectory, originalConfig.configFileName); -} +await overrideNextConfig(root, originalConfig.configFileName); +await validateNextConfigOverride(root, opts.projectDirectory, originalConfig.configFileName); await runBuild(); diff --git a/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts b/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts index 2af93b1d..b60b5a17 100644 --- a/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts +++ b/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts @@ -197,7 +197,7 @@ describe("next config overrides", () => { } module.exports = nextConfig - ` + `; beforeEach(() => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "test-overrides")); }); @@ -331,9 +331,7 @@ describe("next config overrides", () => { const { overrideNextConfig } = await importOverrides; await overrideNextConfig(tmpDir, "next.config.js"); const updatedConfig = fs.readFileSync(path.join(tmpDir, "next.config.js"), "utf-8"); - assert.equal( - normalizeWhitespace(updatedConfig), normalizeWhitespace(defaultNextConfig), - ); + assert.equal(normalizeWhitespace(updatedConfig), normalizeWhitespace(defaultNextConfig)); }); }); @@ -362,25 +360,12 @@ describe("validateNextConfigOverride", () => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); - it("should throw an error when new config file doesn't exist", async () => { - fs.writeFileSync(originalConfigPath, "module.exports = {}"); - - const { validateNextConfigOverride } = await importOverrides; - - await assert.rejects( - async () => await validateNextConfigOverride(root, projectRoot, originalConfigFileName), - /New Next.js config file not found/, - ); - }); - - it("should throw an error when original config file doesn't exist", async () => { - fs.writeFileSync(newConfigPath, "module.exports = {}"); - + it("should throw an error if next config file doesn't exist", async () => { const { validateNextConfigOverride } = await importOverrides; await assert.rejects( async () => await validateNextConfigOverride(root, projectRoot, originalConfigFileName), - /Original Next.js config file not found/, + /Next.js config file not found/, ); }); }); diff --git a/packages/@apphosting/adapter-nextjs/src/overrides.ts b/packages/@apphosting/adapter-nextjs/src/overrides.ts index 9fdce496..e0a39527 100644 --- a/packages/@apphosting/adapter-nextjs/src/overrides.ts +++ b/packages/@apphosting/adapter-nextjs/src/overrides.ts @@ -10,7 +10,7 @@ import { import { join, extname } from "path"; import { rename as renamePromise } from "fs/promises"; -const DEFAULT_NEXT_CONFIG_FILE = 'next.config.js' +const DEFAULT_NEXT_CONFIG_FILE = "next.config.js"; /** * Overrides the user's Next Config file (next.config.[ts|js|mjs]) to add configs @@ -21,17 +21,20 @@ export async function overrideNextConfig(projectRoot: string, nextConfigFileName // Check if the file exists in the current working directory const configPath = join(projectRoot, nextConfigFileName); - if (!(await exists(configPath))){ - console.log(`No Next config file found at ${configPath}, creating one with Firebase App Hosting overrides`); + if (!(await exists(configPath))) { + console.log( + `No Next config file found at ${configPath}, creating one with Firebase App Hosting overrides`, + ); try { await writeFile(join(projectRoot, DEFAULT_NEXT_CONFIG_FILE), defaultNextConfigForFAH()); + console.log( + `Successfully created ${DEFAULT_NEXT_CONFIG_FILE} with Firebase App Hosting overrides`, + ); } catch (error) { console.error(`Error creating ${DEFAULT_NEXT_CONFIG_FILE}: ${error}`); throw error; } - - console.log(`Successfully created ${DEFAULT_NEXT_CONFIG_FILE} with Firebase App Hosting overrides`); - return + return; } // A Next Config already exists in the user's project, so it needs to be overriden @@ -133,34 +136,25 @@ function defaultNextConfigForFAH() { } module.exports = nextConfig - ` + `; } /** - * This function is used to validate the state of an app after running the + * This function is used to validate the state of an app after running * overrideNextConfig. It validates that: - * 1. original next config is preserved - * 2. a new next config is created - * 3. new next config can be loaded by NextJs without any issues. + * 1. a next config is exists (should be created with FAH overrides + * even if user did not create one) + * 3. next config can be loaded by NextJs without any issues. */ export async function validateNextConfigOverride( root: string, projectRoot: string, originalConfigFileName: string, ) { - const originalConfigExtension = extname(originalConfigFileName); - const newConfigFileName = `next.config.original${originalConfigExtension}`; - const newConfigFilePath = join(root, newConfigFileName); - if (!(await exists(newConfigFilePath))) { - throw new Error( - `Next Config Override Failed: New Next.js config file not found at ${newConfigFilePath}`, - ); - } - const originalNextConfigFilePath = join(root, originalConfigFileName); if (!(await exists(originalNextConfigFilePath))) { throw new Error( - `Next Config Override Failed: Original Next.js config file not found at ${originalNextConfigFilePath}`, + `Next Config Override Failed: Next.js config file not found at ${originalNextConfigFilePath}`, ); } From ece5dfd35e43402e873ffe21caf2a3b89ee166a8 Mon Sep 17 00:00:00 2001 From: Mathusan Selvarajah <mathusan@google.com> Date: Tue, 1 Apr 2025 20:33:19 +0000 Subject: [PATCH 03/10] fix linting issues --- packages/@apphosting/adapter-nextjs/e2e/run-local.ts | 2 +- packages/@apphosting/adapter-nextjs/src/bin/build.ts | 1 - packages/@apphosting/adapter-nextjs/src/overrides.spec.ts | 6 ------ 3 files changed, 1 insertion(+), 8 deletions(-) diff --git a/packages/@apphosting/adapter-nextjs/e2e/run-local.ts b/packages/@apphosting/adapter-nextjs/e2e/run-local.ts index 4c09dd66..40080050 100644 --- a/packages/@apphosting/adapter-nextjs/e2e/run-local.ts +++ b/packages/@apphosting/adapter-nextjs/e2e/run-local.ts @@ -68,7 +68,7 @@ const scenarios: Scenario[] = [ // skip creating the test config if data is not provided if (!scenario.config || !scenario.file) { - return + return; } const configContent = scenario.config; diff --git a/packages/@apphosting/adapter-nextjs/src/bin/build.ts b/packages/@apphosting/adapter-nextjs/src/bin/build.ts index 6efe0923..3184b413 100644 --- a/packages/@apphosting/adapter-nextjs/src/bin/build.ts +++ b/packages/@apphosting/adapter-nextjs/src/bin/build.ts @@ -5,7 +5,6 @@ import { generateBuildOutput, validateOutputDirectory, getAdapterMetadata, - exists, } from "../utils.js"; import { join } from "path"; import { getBuildOptions, runBuild } from "@apphosting/common"; diff --git a/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts b/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts index b60b5a17..fe7e9080 100644 --- a/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts +++ b/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts @@ -340,18 +340,12 @@ describe("validateNextConfigOverride", () => { let root: string; let projectRoot: string; let originalConfigFileName: string; - let newConfigFileName: string; - let originalConfigPath: string; - let newConfigPath: string; beforeEach(() => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "test-next-config-override")); root = tmpDir; projectRoot = tmpDir; originalConfigFileName = "next.config.js"; - newConfigFileName = "next.config.original.js"; - originalConfigPath = path.join(root, originalConfigFileName); - newConfigPath = path.join(root, newConfigFileName); fs.mkdirSync(root, { recursive: true }); }); From 4a85c0a7facaf85f3afb8f95097f4cb434723855 Mon Sep 17 00:00:00 2001 From: Mathusan Selvarajah <mathusan@google.com> Date: Mon, 7 Apr 2025 23:19:19 +0000 Subject: [PATCH 04/10] add back some tests --- .../adapter-nextjs/src/bin/build.ts | 13 ++++++-- .../adapter-nextjs/src/overrides.ts | 32 +++++++++++++------ 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/packages/@apphosting/adapter-nextjs/src/bin/build.ts b/packages/@apphosting/adapter-nextjs/src/bin/build.ts index 3184b413..93b3d7e5 100644 --- a/packages/@apphosting/adapter-nextjs/src/bin/build.ts +++ b/packages/@apphosting/adapter-nextjs/src/bin/build.ts @@ -5,6 +5,7 @@ import { generateBuildOutput, validateOutputDirectory, getAdapterMetadata, + exists, } from "../utils.js"; import { join } from "path"; import { getBuildOptions, runBuild } from "@apphosting/common"; @@ -29,13 +30,19 @@ const originalConfig = await loadConfig(root, opts.projectDirectory); * load. * * If the app does not have a next.config.[js|mjs|ts] file in the first place, - * then can skip config override. + * then one is created with the overrides. * * Note: loadConfig always returns a fileName (default: next.config.js) even if * one does not exist in the app's root: https://github.com/vercel/next.js/blob/23681508ca34b66a6ef55965c5eac57de20eb67f/packages/next/src/server/config.ts#L1115 */ -await overrideNextConfig(root, originalConfig.configFileName); -await validateNextConfigOverride(root, opts.projectDirectory, originalConfig.configFileName); +const userNextConfigExists = await exists(join(root, originalConfig.configFileName)); +await overrideNextConfig(root, originalConfig.configFileName, userNextConfigExists); +await validateNextConfigOverride( + root, + opts.projectDirectory, + originalConfig.configFileName, + userNextConfigExists, +); await runBuild(); diff --git a/packages/@apphosting/adapter-nextjs/src/overrides.ts b/packages/@apphosting/adapter-nextjs/src/overrides.ts index 0ebf6d24..bb781280 100644 --- a/packages/@apphosting/adapter-nextjs/src/overrides.ts +++ b/packages/@apphosting/adapter-nextjs/src/overrides.ts @@ -16,15 +16,14 @@ const DEFAULT_NEXT_CONFIG_FILE = "next.config.js"; * Overrides the user's Next Config file (next.config.[ts|js|mjs]) to add configs * optimized for Firebase App Hosting. */ -export async function overrideNextConfig(projectRoot: string, nextConfigFileName: string) { +export async function overrideNextConfig( + projectRoot: string, + nextConfigFileName: string, + userNextConfigExists: boolean, +) { console.log(`Overriding Next Config to add configs optmized for Firebase App Hosting`); - // Check if the file exists in the current working directory - const configPath = join(projectRoot, nextConfigFileName); - - if (!(await exists(configPath))) { - console.log( - `No Next config file found at ${configPath}, creating one with Firebase App Hosting overrides`, - ); + if (!userNextConfigExists) { + console.log(`No Next config file found, creating one with Firebase App Hosting overrides`); try { await writeFile(join(projectRoot, DEFAULT_NEXT_CONFIG_FILE), defaultNextConfigForFAH()); console.log( @@ -45,6 +44,7 @@ export async function overrideNextConfig(projectRoot: string, nextConfigFileName // Rename the original config file try { + const configPath = join(projectRoot, nextConfigFileName); const originalPath = join(projectRoot, originalConfigName); await renamePromise(configPath, originalPath); @@ -141,7 +141,8 @@ function defaultNextConfigForFAH() { /** * This function is used to validate the state of an app after running * overrideNextConfig. It validates that: - * 1. a next config is exists (should be created with FAH overrides + * 1. if user has a next config it is preserved in a next.config.original.[js|ts|mjs] file + * 2. a next config exists (should be created with FAH overrides * even if user did not create one) * 3. next config can be loaded by NextJs without any issues. */ @@ -149,7 +150,20 @@ export async function validateNextConfigOverride( root: string, projectRoot: string, originalConfigFileName: string, + userNextConfigExists: boolean, ) { + if (userNextConfigExists) { + // Ensure user's existing next config is preserved in a next.config.origin.* file + const originalConfigExtension = extname(originalConfigFileName); + const prservedConfigFileName = `next.config.original${originalConfigExtension}`; + const preservedConfigFilePath = join(root, prservedConfigFileName); + if (!(await exists(preservedConfigFilePath))) { + throw new Error( + `Next Config Override Failed: User's original Next.js config file not preserved ${preservedConfigFilePath}`, + ); + } + } + const originalNextConfigFilePath = join(root, originalConfigFileName); if (!(await exists(originalNextConfigFilePath))) { throw new Error( From a6480252eb4a26b7a7ece8300a737762f9eeceee Mon Sep 17 00:00:00 2001 From: Mathusan Selvarajah <mathusans52@gmail.com> Date: Mon, 7 Apr 2025 20:39:27 -0400 Subject: [PATCH 05/10] fix tets --- .../adapter-nextjs/src/overrides.spec.ts | 64 ++++++++++++++++--- 1 file changed, 55 insertions(+), 9 deletions(-) diff --git a/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts b/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts index fe7e9080..54f2146e 100644 --- a/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts +++ b/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts @@ -216,7 +216,7 @@ describe("next config overrides", () => { `; fs.writeFileSync(path.join(tmpDir, "next.config.js"), originalConfig); - await overrideNextConfig(tmpDir, "next.config.js"); + await overrideNextConfig(tmpDir, "next.config.js", /* userNextConfigExists = */ true); const updatedConfig = fs.readFileSync(path.join(tmpDir, "next.config.js"), "utf-8"); @@ -249,7 +249,7 @@ describe("next config overrides", () => { `; fs.writeFileSync(path.join(tmpDir, "next.config.mjs"), originalConfig); - await overrideNextConfig(tmpDir, "next.config.mjs"); + await overrideNextConfig(tmpDir, "next.config.mjs", /* userNextConfigExists = */ true); const updatedConfig = fs.readFileSync(path.join(tmpDir, "next.config.mjs"), "utf-8"); assert.equal( @@ -282,7 +282,7 @@ describe("next config overrides", () => { `; fs.writeFileSync(path.join(tmpDir, "next.config.mjs"), originalConfig); - await overrideNextConfig(tmpDir, "next.config.mjs"); + await overrideNextConfig(tmpDir, "next.config.mjs", /* userNextConfigExists = */ true); const updatedConfig = fs.readFileSync(path.join(tmpDir, "next.config.mjs"), "utf-8"); assert.equal( @@ -311,7 +311,7 @@ describe("next config overrides", () => { `; fs.writeFileSync(path.join(tmpDir, "next.config.ts"), originalConfig); - await overrideNextConfig(tmpDir, "next.config.ts"); + await overrideNextConfig(tmpDir, "next.config.ts", /* userNextConfigExists = */ true); const updatedConfig = fs.readFileSync(path.join(tmpDir, "next.config.ts"), "utf-8"); assert.equal( @@ -329,7 +329,7 @@ describe("next config overrides", () => { it("should create a default next.config.js file if one does not exist yet", async () => { const { overrideNextConfig } = await importOverrides; - await overrideNextConfig(tmpDir, "next.config.js"); + await overrideNextConfig(tmpDir, "next.config.js", /* userNextConfigExists = */ false); const updatedConfig = fs.readFileSync(path.join(tmpDir, "next.config.js"), "utf-8"); assert.equal(normalizeWhitespace(updatedConfig), normalizeWhitespace(defaultNextConfig)); }); @@ -339,13 +339,19 @@ describe("validateNextConfigOverride", () => { let tmpDir: string; let root: string; let projectRoot: string; - let originalConfigFileName: string; + let configFileName: string; + let configFilePath: string; + let preservedConfigFileName: string; + let preservedConfigFilePath: string; beforeEach(() => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "test-next-config-override")); root = tmpDir; projectRoot = tmpDir; - originalConfigFileName = "next.config.js"; + configFileName = "next.config.js"; + configFilePath = path.join(root, configFileName); + preservedConfigFileName = "next.config.original.js"; + preservedConfigFilePath = path.join(root, preservedConfigFileName); fs.mkdirSync(root, { recursive: true }); }); @@ -354,14 +360,54 @@ describe("validateNextConfigOverride", () => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); - it("should throw an error if next config file doesn't exist", async () => { + it("should throw an error if a next config file was not created because the user did not have one", async () => { const { validateNextConfigOverride } = await importOverrides; await assert.rejects( - async () => await validateNextConfigOverride(root, projectRoot, originalConfigFileName), + async () => + await validateNextConfigOverride( + root, + projectRoot, + configFileName, + /* userNextConfigExists = */ false, + ), /Next.js config file not found/, ); }); + + it("should throw an error when main config file doesn't exist", async () => { + fs.writeFileSync(preservedConfigFilePath, "module.exports = {}"); + + const { validateNextConfigOverride } = await importOverrides; + + await assert.rejects( + async () => + await validateNextConfigOverride( + root, + projectRoot, + configFileName, + /* userNextConfigExists = */ true, + ), + /Next Config Override Failed: Next.js config file not found/, + ); + }); + + it("should throw an error when preserveed config file doesn't exist", async () => { + fs.writeFileSync(configFilePath, "module.exports = {}"); + + const { validateNextConfigOverride } = await importOverrides; + + await assert.rejects( + async () => + await validateNextConfigOverride( + root, + projectRoot, + configFileName, + /* userNextConfigExists = */ true, + ), + /User's original Next.js config file not preserved/, + ); + }); }); // Normalize whitespace for comparison From dd303293af87d76ffaedfd71890c4d440c1532d2 Mon Sep 17 00:00:00 2001 From: Mathusan Selvarajah <mathusan@google.com> Date: Wed, 9 Apr 2025 20:10:30 +0000 Subject: [PATCH 06/10] refactor based on pr comments --- .../adapter-nextjs/src/bin/build.ts | 11 +++----- .../adapter-nextjs/src/overrides.ts | 26 +++++++++---------- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/packages/@apphosting/adapter-nextjs/src/bin/build.ts b/packages/@apphosting/adapter-nextjs/src/bin/build.ts index 93b3d7e5..28fc21e0 100644 --- a/packages/@apphosting/adapter-nextjs/src/bin/build.ts +++ b/packages/@apphosting/adapter-nextjs/src/bin/build.ts @@ -35,14 +35,9 @@ const originalConfig = await loadConfig(root, opts.projectDirectory); * Note: loadConfig always returns a fileName (default: next.config.js) even if * one does not exist in the app's root: https://github.com/vercel/next.js/blob/23681508ca34b66a6ef55965c5eac57de20eb67f/packages/next/src/server/config.ts#L1115 */ -const userNextConfigExists = await exists(join(root, originalConfig.configFileName)); -await overrideNextConfig(root, originalConfig.configFileName, userNextConfigExists); -await validateNextConfigOverride( - root, - opts.projectDirectory, - originalConfig.configFileName, - userNextConfigExists, -); + +await overrideNextConfig(root, originalConfig.configFileName); +await validateNextConfigOverride(root, opts.projectDirectory, originalConfig.configFileName); await runBuild(); diff --git a/packages/@apphosting/adapter-nextjs/src/overrides.ts b/packages/@apphosting/adapter-nextjs/src/overrides.ts index bb781280..7b3f31fd 100644 --- a/packages/@apphosting/adapter-nextjs/src/overrides.ts +++ b/packages/@apphosting/adapter-nextjs/src/overrides.ts @@ -16,12 +16,9 @@ const DEFAULT_NEXT_CONFIG_FILE = "next.config.js"; * Overrides the user's Next Config file (next.config.[ts|js|mjs]) to add configs * optimized for Firebase App Hosting. */ -export async function overrideNextConfig( - projectRoot: string, - nextConfigFileName: string, - userNextConfigExists: boolean, -) { +export async function overrideNextConfig(projectRoot: string, nextConfigFileName: string) { console.log(`Overriding Next Config to add configs optmized for Firebase App Hosting`); + const userNextConfigExists = await exists(join(projectRoot, nextConfigFileName)); if (!userNextConfigExists) { console.log(`No Next config file found, creating one with Firebase App Hosting overrides`); try { @@ -149,14 +146,14 @@ function defaultNextConfigForFAH() { export async function validateNextConfigOverride( root: string, projectRoot: string, - originalConfigFileName: string, - userNextConfigExists: boolean, + configFileName: string, ) { + const userNextConfigExists = await exists(join(projectRoot, configFileName)); if (userNextConfigExists) { // Ensure user's existing next config is preserved in a next.config.origin.* file - const originalConfigExtension = extname(originalConfigFileName); - const prservedConfigFileName = `next.config.original${originalConfigExtension}`; - const preservedConfigFilePath = join(root, prservedConfigFileName); + const originalConfigExtension = extname(configFileName); + const preservedConfigFileName = `next.config.original${originalConfigExtension}`; + const preservedConfigFilePath = join(root, preservedConfigFileName); if (!(await exists(preservedConfigFilePath))) { throw new Error( `Next Config Override Failed: User's original Next.js config file not preserved ${preservedConfigFilePath}`, @@ -164,10 +161,13 @@ export async function validateNextConfigOverride( } } - const originalNextConfigFilePath = join(root, originalConfigFileName); - if (!(await exists(originalNextConfigFilePath))) { + const configFilePath = join( + root, + userNextConfigExists ? configFileName : DEFAULT_NEXT_CONFIG_FILE, + ); + if (!(await exists(configFilePath))) { throw new Error( - `Next Config Override Failed: Next.js config file not found at ${originalNextConfigFilePath}`, + `Next Config Override Failed: Next.js config file not found at ${configFilePath}`, ); } From f0b543360b256ef6b63bc07096cda3cb2e04891b Mon Sep 17 00:00:00 2001 From: Mathusan Selvarajah <mathusan@google.com> Date: Thu, 10 Apr 2025 15:52:39 +0000 Subject: [PATCH 07/10] fix test --- .../adapter-nextjs/src/overrides.spec.ts | 34 +++++-------------- 1 file changed, 8 insertions(+), 26 deletions(-) diff --git a/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts b/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts index 54f2146e..6f647ca9 100644 --- a/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts +++ b/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts @@ -216,7 +216,7 @@ describe("next config overrides", () => { `; fs.writeFileSync(path.join(tmpDir, "next.config.js"), originalConfig); - await overrideNextConfig(tmpDir, "next.config.js", /* userNextConfigExists = */ true); + await overrideNextConfig(tmpDir, "next.config.js"); const updatedConfig = fs.readFileSync(path.join(tmpDir, "next.config.js"), "utf-8"); @@ -249,7 +249,7 @@ describe("next config overrides", () => { `; fs.writeFileSync(path.join(tmpDir, "next.config.mjs"), originalConfig); - await overrideNextConfig(tmpDir, "next.config.mjs", /* userNextConfigExists = */ true); + await overrideNextConfig(tmpDir, "next.config.mjs"); const updatedConfig = fs.readFileSync(path.join(tmpDir, "next.config.mjs"), "utf-8"); assert.equal( @@ -282,7 +282,7 @@ describe("next config overrides", () => { `; fs.writeFileSync(path.join(tmpDir, "next.config.mjs"), originalConfig); - await overrideNextConfig(tmpDir, "next.config.mjs", /* userNextConfigExists = */ true); + await overrideNextConfig(tmpDir, "next.config.mjs"); const updatedConfig = fs.readFileSync(path.join(tmpDir, "next.config.mjs"), "utf-8"); assert.equal( @@ -311,7 +311,7 @@ describe("next config overrides", () => { `; fs.writeFileSync(path.join(tmpDir, "next.config.ts"), originalConfig); - await overrideNextConfig(tmpDir, "next.config.ts", /* userNextConfigExists = */ true); + await overrideNextConfig(tmpDir, "next.config.ts"); const updatedConfig = fs.readFileSync(path.join(tmpDir, "next.config.ts"), "utf-8"); assert.equal( @@ -329,7 +329,7 @@ describe("next config overrides", () => { it("should create a default next.config.js file if one does not exist yet", async () => { const { overrideNextConfig } = await importOverrides; - await overrideNextConfig(tmpDir, "next.config.js", /* userNextConfigExists = */ false); + await overrideNextConfig(tmpDir, "next.config.js"); const updatedConfig = fs.readFileSync(path.join(tmpDir, "next.config.js"), "utf-8"); assert.equal(normalizeWhitespace(updatedConfig), normalizeWhitespace(defaultNextConfig)); }); @@ -364,13 +364,7 @@ describe("validateNextConfigOverride", () => { const { validateNextConfigOverride } = await importOverrides; await assert.rejects( - async () => - await validateNextConfigOverride( - root, - projectRoot, - configFileName, - /* userNextConfigExists = */ false, - ), + async () => await validateNextConfigOverride(root, projectRoot, configFileName), /Next.js config file not found/, ); }); @@ -381,13 +375,7 @@ describe("validateNextConfigOverride", () => { const { validateNextConfigOverride } = await importOverrides; await assert.rejects( - async () => - await validateNextConfigOverride( - root, - projectRoot, - configFileName, - /* userNextConfigExists = */ true, - ), + async () => await validateNextConfigOverride(root, projectRoot, configFileName), /Next Config Override Failed: Next.js config file not found/, ); }); @@ -398,13 +386,7 @@ describe("validateNextConfigOverride", () => { const { validateNextConfigOverride } = await importOverrides; await assert.rejects( - async () => - await validateNextConfigOverride( - root, - projectRoot, - configFileName, - /* userNextConfigExists = */ true, - ), + async () => await validateNextConfigOverride(root, projectRoot, configFileName), /User's original Next.js config file not preserved/, ); }); From af55882cff6d8db7e1f15934f8607c17877eab37 Mon Sep 17 00:00:00 2001 From: Mathusan Selvarajah <mathusan@google.com> Date: Thu, 10 Apr 2025 16:08:31 +0000 Subject: [PATCH 08/10] fix lint --- packages/@apphosting/adapter-nextjs/src/bin/build.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@apphosting/adapter-nextjs/src/bin/build.ts b/packages/@apphosting/adapter-nextjs/src/bin/build.ts index 28fc21e0..b7fb4d37 100644 --- a/packages/@apphosting/adapter-nextjs/src/bin/build.ts +++ b/packages/@apphosting/adapter-nextjs/src/bin/build.ts @@ -5,7 +5,6 @@ import { generateBuildOutput, validateOutputDirectory, getAdapterMetadata, - exists, } from "../utils.js"; import { join } from "path"; import { getBuildOptions, runBuild } from "@apphosting/common"; From 4944f6c1c984081dfc05d5f836b872778e331b55 Mon Sep 17 00:00:00 2001 From: Mathusan Selvarajah <mathusan@google.com> Date: Thu, 10 Apr 2025 20:27:15 +0000 Subject: [PATCH 09/10] remove validation for preserved config file --- .../adapter-nextjs/src/overrides.spec.ts | 11 ----------- .../@apphosting/adapter-nextjs/src/overrides.ts | 15 ++------------- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts b/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts index 6f647ca9..66f2f0ef 100644 --- a/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts +++ b/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts @@ -379,17 +379,6 @@ describe("validateNextConfigOverride", () => { /Next Config Override Failed: Next.js config file not found/, ); }); - - it("should throw an error when preserveed config file doesn't exist", async () => { - fs.writeFileSync(configFilePath, "module.exports = {}"); - - const { validateNextConfigOverride } = await importOverrides; - - await assert.rejects( - async () => await validateNextConfigOverride(root, projectRoot, configFileName), - /User's original Next.js config file not preserved/, - ); - }); }); // Normalize whitespace for comparison diff --git a/packages/@apphosting/adapter-nextjs/src/overrides.ts b/packages/@apphosting/adapter-nextjs/src/overrides.ts index 7b3f31fd..1b1f57bd 100644 --- a/packages/@apphosting/adapter-nextjs/src/overrides.ts +++ b/packages/@apphosting/adapter-nextjs/src/overrides.ts @@ -148,23 +148,12 @@ export async function validateNextConfigOverride( projectRoot: string, configFileName: string, ) { - const userNextConfigExists = await exists(join(projectRoot, configFileName)); - if (userNextConfigExists) { - // Ensure user's existing next config is preserved in a next.config.origin.* file - const originalConfigExtension = extname(configFileName); - const preservedConfigFileName = `next.config.original${originalConfigExtension}`; - const preservedConfigFilePath = join(root, preservedConfigFileName); - if (!(await exists(preservedConfigFilePath))) { - throw new Error( - `Next Config Override Failed: User's original Next.js config file not preserved ${preservedConfigFilePath}`, - ); - } - } - + const userNextConfigExists = await exists(join(root, configFileName)); const configFilePath = join( root, userNextConfigExists ? configFileName : DEFAULT_NEXT_CONFIG_FILE, ); + if (!(await exists(configFilePath))) { throw new Error( `Next Config Override Failed: Next.js config file not found at ${configFilePath}`, From 1266f8418c56b25cd1047022e877a63cb3c3571e Mon Sep 17 00:00:00 2001 From: Mathusan Selvarajah <mathusan@google.com> Date: Thu, 10 Apr 2025 20:37:51 +0000 Subject: [PATCH 10/10] fix lint --- packages/@apphosting/adapter-nextjs/src/overrides.spec.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts b/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts index 66f2f0ef..172478a6 100644 --- a/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts +++ b/packages/@apphosting/adapter-nextjs/src/overrides.spec.ts @@ -340,7 +340,6 @@ describe("validateNextConfigOverride", () => { let root: string; let projectRoot: string; let configFileName: string; - let configFilePath: string; let preservedConfigFileName: string; let preservedConfigFilePath: string; @@ -349,7 +348,6 @@ describe("validateNextConfigOverride", () => { root = tmpDir; projectRoot = tmpDir; configFileName = "next.config.js"; - configFilePath = path.join(root, configFileName); preservedConfigFileName = "next.config.original.js"; preservedConfigFilePath = path.join(root, preservedConfigFileName);