Conversation
### Description Added a warning handler in the CLI entry point to ignore the `DEP0040` (punycode deprecation) warning. This warning is triggered by dependencies (like `tr46`) on Node 22 because Node 22 deprecates the built-in `punycode` module. Since we cannot easily update the dependencies to avoid it without risking regressions or security issues, ignoring the specific warning is the safest approach. Fixes #10385 ### Scenarios Tested - Verified that unit tests pass (4269 passing). - Verified that the changes are limited to `src/bin/firebase.ts` and `CHANGELOG.md`. ### Sample Commands None (this is an internal warning suppression).
There was a problem hiding this comment.
Code Review
This pull request introduces several significant updates, including SSE mode support for the Firebase MCP server, a new MCP App for environment management, and enhanced security checks for hosting deployments. It also implements secret resolution for local App Hosting builds with a new flag and interactive prompts. Feedback addresses a typo in the changelog and highlights several instances where the "any" type was used in violation of the repository's style guide, particularly in the new test files and MCP application logic.
I am having trouble creating individual review comments. Click here to see my feedback.
CHANGELOG.md (2)
There seems to be a typo in the pull request number. The PR description for this fix refers to #10380, but the changelog says #10376. For better traceability, it would be good to keep these consistent.
- Fixed an issue where hosting deploy allowed publishing to a site in a different project. (#10380)
src/appdistribution/distribution.spec.ts (71)
This file makes extensive use of any and unknown as any, which is discouraged by the repository's style guide (GEMINI.md, line 38: "Never use any or unknown as an escape hatch. Define proper interfaces/types or use type guards.").
While this is a test file, adhering to stricter typing improves test reliability and maintainability. Consider using partial types (e.g., Partial<Release>) or defining mock objects that more closely match the required interfaces to avoid these type assertions.
For example, here on line 71, instead of casting to any, you could define a more complete mock object for the release.
References
- Never use
anyorunknownas an escape hatch. Define proper interfaces/types or use type guards. (link)
src/mcp/apps/update_environment/mcp-app.ts (15)
This file uses the any type in several places (e.g., for projects, ctx in onhostcontextchanged, and when parsing tool results), which goes against the repository's style guide (GEMINI.md, line 38: "Never use any or unknown as an escape hatch. Define proper interfaces/types or use type guards.").
To improve type safety and code clarity, please define interfaces for these objects. For instance, for the projects array, you could define a Project interface:
interface Project {
projectId: string;
displayName?: string;
}let projects: { projectId: string; displayName?: string }[] = [];
References
- Never use
anyorunknownas an escape hatch. Define proper interfaces/types or use type guards. (link)
aalej
left a comment
There was a problem hiding this comment.
LGTM! Tested locally and the warning message is no longer present
- on branch
jh-fix-punycode
$ firebase deploy --dry-run
=== Deploying to 'PROJECT_ID'...
i deploying functions
Running command: npm --prefix "$RESOURCE_DIR" run lint
npm i -g firebase-tools
$ firebase deploy --dry-run
(node:28010) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
=== Deploying to 'PROJECT_ID'...
i deploying functions
Running command: npm --prefix "$RESOURCE_DIR" run lint
Description
Silence the
punycodedeprecation warning that was being printed pout during deploy on Node 22. This is coming from a transitive dependency and has no effect on our users, so this was pure noise. Fixes #10385