Skip to content

fix(nextjs): restore the user config when the override fails - #682

Open
Om-singhaI wants to merge 1 commit into
firebase:mainfrom
Om-singhaI:fix/nextjs-mts-config-support
Open

Om-singhaI wants to merge 1 commit into
firebase:mainfrom
Om-singhaI:fix/nextjs-mts-config-support

Conversation

@Om-singhaI

@Om-singhaI Om-singhaI commented Aug 24, 2026

Copy link
Copy Markdown

overrideNextConfig renames the user's Next config before anything that can throw, and
bin/build.ts called it outside the try whose finally restores it. Any failure after the
rename left the project holding next.config.original.<ext> and no config at all, and a later
run does not recover it: loadConfig then reports the default next.config.js, so
restoreNextConfig looks for a backup under the wrong extension and returns early.

Cloud Build workspaces are ephemeral, but the adapter also runs locally through
npm exec apphosting-adapter-nextjs-build, where this mutates the developer's tree and does
not put it back.

The extension check and the config content now happen before the rename, the original is
restored if the write fails, and the override and its validation moved inside the try.

The new test covers the path that throws before the rename. The restore in the catch is
covered by inspection only, since making writeFile fail after a successful rename in the
same directory is not something I could drive from a unit test.

#684 shipped .mts support, which was the other half of #680.

Fixes #680

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds support for the .mts file extension in Next.js configuration overrides and improves error handling during the override process. Specifically, validation of the file extension is now performed before renaming the original configuration file, and a fallback mechanism has been added to restore the original configuration if the override process fails. Unit tests have also been added to verify these changes. I have no feedback to provide.

overrideNextConfig renamed the user's next config before it could throw, and
bin/build.ts called it outside the try whose finally restores. Any failure
after the rename left the project with no next config at all.

I build the replacement content before renaming and restore the original if
the write fails. The override and validation calls moved inside the try.
@Om-singhaI
Om-singhaI force-pushed the fix/nextjs-mts-config-support branch from 8bea898 to 6e595d4 Compare September 4, 2026 08:37
@Om-singhaI Om-singhaI changed the title fix(adapter-nextjs): support next.config.mts and stop stranding the user config fix(nextjs): restore the user config when the override fails Sep 4, 2026
@Om-singhaI

Copy link
Copy Markdown
Author

#684 covered the .mts half of #680, so I have rescoped this to item 2 of that issue, the rename that is not undone. It is independent of the extension: a writeFile failure strands a .ts project the same way, and a later run cannot recover it because restoreNextConfig then looks for the backup under the default next.config.js extension.

The branch is rebuilt on current main and shares no lines with #684. The extension check moved above the rename, the original is restored if the write fails, and the override and its validation moved inside the try whose finally already restores.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

adapter-nextjs: next.config.mts is rejected, and the failure leaves the config renamed away

1 participant