-
Notifications
You must be signed in to change notification settings - Fork 1k
feat: Add support for function prefixes #8911
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
This change introduces the ability to deploy multiple instances of the same function source, each with its own configuration. This is achieved by allowing multiple function configurations in `firebase.json`. If multiple configurations are present, each must have a unique `codebase` identifier. This change is the first step towards implementing "Function Kits", which will allow developers to easily add pre-built backend functionality to their Firebase projects. This change is backward-compatible. Existing `firebase.json` configurations will continue to work as before.
This change introduces two new validation rules for function configurations in `firebase.json`: 1. The `prefix` property, if specified, must only contain lowercase letters, numbers, and hyphens. 2. The combination of `source` and `prefix` must be unique across all function configurations. These changes ensure that function deployments with prefixes are valid and that there are no conflicts when deploying multiple functions from the same source directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @taeold, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances the flexibility and robustness of Firebase Functions configurations by introducing support for function name prefixes. The primary goal is to enable developers to define and deploy multiple distinct function configurations from a single source directory, while simultaneously enforcing strict validation rules on these prefixes to prevent naming collisions and ensure deployment integrity. This change impacts both the configuration schema and the core logic for building, deploying, and emulating functions.
Highlights
- New
prefix
property for Functions: Introduced a new optionalprefix
property for function configurations withinfirebase.json
. This allows developers to define a prefix that will be prepended to the function's ID during deployment, enabling more flexible naming conventions and supporting multiple function configurations from the same source directory. - Prefix Validation Rules: Implemented strict validation for the new
prefix
property. The prefix must now consist only of lowercase letters, numbers, and hyphens, and has a maximum length of 30 characters. This ensures valid and consistent function naming. - Unique Source/Prefix Combination Enforcement: Added a crucial validation rule that requires the combination of a function's
source
directory and itsprefix
to be unique across all function configurations. This prevents naming conflicts when deploying multiple functions that share the same source code but are intended to be distinct entities. - Emulator and Build Process Integration: Updated the Firebase Functions emulator and the build/deployment pipeline to correctly recognize and apply the new
prefix
property. This ensures that functions are correctly named and discoverable both during local development and actual deployments.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
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 introduces validation for function prefixes and ensures uniqueness for source/prefix pairs, which is a valuable addition for managing complex function deployments. The implementation is solid and well-tested. I have a few suggestions to enhance the robustness of the validation and improve test coverage. My main concern is with the prefix validation logic, which could allow for invalid function names to be constructed. I've provided a critical-severity comment with a suggested fix for this. I've also suggested a minor code simplification and an additional test case to cover an important edge case. Overall, great work on this feature!
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This commit includes several improvements to the function configuration validation: - The error message for duplicate `source`/`prefix` pairs is now more descriptive and suggests a solution. - The test suite is more robust, with more specific error message checks and improved typing. - Linter warnings have been addressed.
…onfig validation Previously, validation errors (like duplicate source/prefix pairs) were caught and hidden with a generic warning about missing source directories. This caused confusing downstream failures like "Extensions Emulator is running but Functions emulator is not". Now validation errors are properly surfaced with clear error messages that match what users see during deploy, making it easier to diagnose configuration issues.
/gemini review |
There was a problem hiding this comment.
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 function prefixes, allowing multiple function configurations to share the same source directory. The changes include updates to the configuration schema, deployment preparation logic, and emulator functionality to handle these prefixes. New validation rules are also introduced to ensure the correctness of prefixed function configurations.
My review has identified a high-severity bug in the validation logic that could lead to incorrect behavior with certain source and prefix combinations. I've also pointed out an opportunity to refactor duplicated code to improve maintainability. Please see the detailed comments for suggestions.
Refactor prefix application logic into a centralized applyPrefix function to improve maintainability and consistency across the codebase. The function updates endpoint keys in build objects and handles empty prefixes correctly. Also fixes projectConfig unique key generation to use JSON.stringify for more robust comparison of source-prefix pairs.
/gemini review |
There was a problem hiding this comment.
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 introduces a prefix
property for Firebase Functions, enabling multiple configurations to share a source directory by prepending a unique string to function names. The implementation includes necessary validation for the prefix format and ensures the uniqueness of source-prefix combinations. The changes are well-integrated into the configuration, deployment, and emulator components. My feedback includes a couple of suggestions to improve test clarity and remove a redundant line of code for better maintainability.
### Description - Refactor applyPrefix test to use factory function and separate test cases to avoid mutation side effects - Remove redundant runtime assignment in prepare.ts since wantBuilds[codebase] is already assigned to discoveredBuild ### Scenarios Tested - applyPrefix function with prefix and empty prefix cases - Existing test suite continues to pass ### Sample Commands npm test npm run mocha:fast
/gemini review |
There was a problem hiding this comment.
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 introduces support for a prefix
property in firebase.json
for function configurations. This allows multiple function groups to be defined from the same source directory by using unique prefixes. The changes span across configuration validation, the build process, and emulator support, ensuring that prefixed functions are deployed and emulated correctly. The implementation is well-tested. I have one concern about a potential regression in src/deploy/functions/prepare.ts
where runtime information might not be correctly propagated to the build object.
When using prefixed multi-instance function deployments, secret names in secretEnvironmentVariables are now also prefixed to ensure proper isolation between different instances. The environment variable keys remain unchanged so function code doesn't need modification. Also fixes missing runtime assignment that was accidentally removed in a previous refactor and adds test coverage for runtime preservation.
/gemini review |
There was a problem hiding this comment.
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 effectively introduces support for function prefixes, allowing for more flexible deployment configurations. The changes are well-implemented across the schema, validation logic, and emulator. The new validation rules for prefix
and the uniqueness of source
/prefix
pairs are robust and well-tested. The addition of tests for the new functionality is comprehensive. I've also noticed a nice bug fix for handling environment variables, which improves code robustness. Overall, this is a solid contribution.
…s; JSON schema description; tests for overflow and invalid prefix
… files - Fix 'should support multiple codebases with the same source and apply prefixes' test - Register emulator with EmulatorRegistry before calling connect() to provide host/port info - Remove auto-generated test_output.log from repository - Add test_output.log and scripts/emulator-tests/functions/index.js to .gitignore
This file is auto-generated during test runs and should not be committed.
- Document source, ignore, runtime, and codebase properties - Provide examples and clarify default behaviors - Match documentation style used for prefix property
Clarify that the function prefixes both endpoint IDs and secret names to ensure isolation between different codebases
- Update prefix test to properly manage emulator lifecycle through registry - Fix FunctionsEmulator to close file watchers on stop() to prevent hanging - Store watchers created in connect() and close them in stop() - This fixes test hanging issue that occurred when connect() was called
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful!
- Use Object.entries() instead of Object.keys() in build.ts - Use latest('nodejs') for runtime in prepare.spec.ts - Reorganize imports in prepare.spec.ts to group asterisk imports
This change adds a new
prefix
property to firebase functions configuration in firebase.json. The new property can be used to prepend function name with the given prefix for all functions in the codebase.Note that prefix is also used for all secret associated with the codebase to allow different codebases to refer to different configuration value.
The new feature also requires two new validation rules for function configurations in
firebase.json
:prefix
property, if specified, must only contain lowercase letters, numbers, and hyphens.source
andprefix
must be unique across all function configurations.These changes ensure that function deployments with prefixes are valid and that there are no conflicts when deploying multiple functions from the same source directory.