Repository navigation
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors local build preparation and deployment for App Hosting and Cloud Run by extracting and unifying the buildDirectory helper into a shared utility. This ensures that local builds in monorepos (such as Turborepo or Nx) run from the monorepo root, while other apps build from their own subfolders. The archiving and uploading processes are updated to use the correct build directory rather than the scratch directory root. Feedback on the changes suggests specifying a non-zero exit code ({ exit: 1 }) when throwing a FirebaseError for missing local build properties, in accordance with the repository style guide.
| if (!outputFiles || !localBuildScratchDir || !buildDir) { | ||
| throw new FirebaseError(`No local build output files found for ${cfg.backendId}`); | ||
| } |
There was a problem hiding this comment.
According to the repository style guide, for expected errors due to a violation of a precondition (such as these required local build properties being undefined), we should specify a non-zero exit code ({ exit: 1 }).
| if (!outputFiles || !localBuildScratchDir || !buildDir) { | |
| throw new FirebaseError(`No local build output files found for ${cfg.backendId}`); | |
| } | |
| if (!outputFiles || !localBuildScratchDir || !buildDir) { | |
| throw new FirebaseError("No local build output files found for " + cfg.backendId, { exit: 1 }); | |
| } |
References
- Throw FirebaseError for expected, user-facing errors. If the error is due to a violation of a precondition (e.g. something that is null but should never be), specify a non-zero exit code ({ exit: 1 }). (link)
Local Builds, for both Cloud Run and App Hosting, copied the whole project and always built from its root with GOOGLE_BUILDABLE set to rootDir. That fails when the project's root has no package.json, like a self-contained app in a subfolder (firebase.json + functions/ + web/) or a Turborepo in a subfolder: Universal Maker finds no buildpack group (b/576681181). Local Builds now use buildDirectory(), the rule that #11282 added for Cloud Run source deploys: - The scratch copy still has the whole project, so all of its .gitignore files still apply. - The build runs in the Nx or Turborepo root, with GOOGLE_BUILDABLE set to the app's path, or else in the app's own folder. The built app is uploaded from that folder. - App Hosting's rollout sends rootDirectory relative to that upload, as the API defines it. buildDirectory() moves to deploy/apphosting/util.ts so that the shared local build code can use it. Its outside-the-project check now uses the new isInDirectory(), which also handles paths on another drive.
84ca862 to
baeb216
Compare
Description
Stacked on #11403. Local Builds always built from the project root with
GOOGLE_BUILDABLE=<rootDir>, which fails when the root has nopackage.json(e.g. an app inweb/, or a Turborepo in a subfolder). They now build where #11282'sbuildDirectory()says: the Nx/Turborepo root, or else the app's folder. App Hosting's rollout sendsrootDirectoryrelative to that folder.Scenarios Tested
Sample Commands
# With "rootDir": "web" and "localBuild": true in firebase.json: firebase deploy --only run:web firebase deploy --only apphosting:web