fix(nextjs): restore the user config when the override fails - #682
Om-singhaI wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
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.
8bea898 to
6e595d4
Compare
|
#684 covered the 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 |
overrideNextConfigrenames the user's Next config before anything that can throw, andbin/build.tscalled it outside thetrywhosefinallyrestores it. Any failure after therename left the project holding
next.config.original.<ext>and no config at all, and a laterrun does not recover it:
loadConfigthen reports the defaultnext.config.js, sorestoreNextConfiglooks 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 doesnot 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
catchiscovered by inspection only, since making
writeFilefail after a successfulrenamein thesame directory is not something I could drive from a unit test.
#684 shipped
.mtssupport, which was the other half of #680.Fixes #680