Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/deploy/extensions/prepare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,14 @@ export async function prepareDynamicExtensions(
const filters = getEndpointFilters(options, functionsConfig);
const extensions = extractExtensionsFromBuilds(builds, filters);
const projectId = needProjectId(options);

// Skip extensions API checks if no extensions are defined in the builds.
// This avoids unnecessary API calls and permission errors for projects
// that don't use Firebase Extensions but still deploy functions.
if (Object.keys(extensions).length === 0) {
return;
}
Comment on lines +178 to +180
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This change correctly avoids API calls when no extensions are defined in code, which fixes the issue for users not using extensions. However, this introduces a change in behavior. Previously, removing a dynamic extension from functions code and re-deploying would trigger its deletion. With this early return, deletion of dynamic extensions during a function deploy will no longer occur. Users will have to use firebase ext:uninstall instead. Is this change in workflow intended?


const projectNumber = await needProjectNumber(options);

await ensureExtensionsApiEnabled(options);
Expand All @@ -181,8 +189,8 @@ export async function prepareDynamicExtensions(
extensionMatchesAnyFilter(e.labels?.codebase, e.instanceId, filters),
);

if (Object.keys(extensions).length === 0 && haveExtensions.length === 0) {
// Nothing defined, and nothing to delete
if (haveExtensions.length === 0) {
// Nothing to delete
return;
}
Comment on lines +192 to 195
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

This new condition appears to introduce a bug. If a user is adding dynamic extensions to a project for the first time, haveExtensions will be empty, and this early return will prevent the new extensions from being created.

Given the check for Object.keys(extensions).length === 0 at the beginning of the function, this block is no longer necessary and can be removed entirely.


Expand Down