Skip to content
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

Better null handling for extensions want #7576

Merged
merged 1 commit into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fixed an issue where functions deployment would fail if `firebase.json#extensions` was undefined. (#7575)
10 changes: 8 additions & 2 deletions src/deploy/extensions/planner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@
return i.extension;
}

/** Caching fetcher for the corresponding ExtensionSpec for an instance spec.

Check warning on line 87 in src/deploy/extensions/planner.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Should have no text on the "0th" line (after the `/**`)
*/
export async function getExtensionSpec(i: InstanceSpec): Promise<ExtensionSpec> {
if (!i.extensionSpec) {
Expand All @@ -93,24 +93,24 @@
i.extensionSpec = extensionVersion.spec;
} else if (i.localPath) {
i.extensionSpec = await readExtensionYaml(i.localPath);
i.extensionSpec!.postinstallContent = await readPostinstall(i.localPath);

Check warning on line 96 in src/deploy/extensions/planner.ts

View workflow job for this annotation

GitHub Actions / lint (20)

This assertion is unnecessary since it does not change the type of the expression

Check warning on line 96 in src/deploy/extensions/planner.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Forbidden non-null assertion
} else {
throw new FirebaseError("InstanceSpec had no ref or localPath, unable to get extensionSpec");
}
}
return i.extensionSpec!;

Check warning on line 101 in src/deploy/extensions/planner.ts

View workflow job for this annotation

GitHub Actions / lint (20)

This assertion is unnecessary since it does not change the type of the expression

Check warning on line 101 in src/deploy/extensions/planner.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Forbidden non-null assertion
}

/**
* have checks a project for what extension instances are currently installed,
* and returns them as a list of instanceSpecs.
* @param projectId

Check warning on line 107 in src/deploy/extensions/planner.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing JSDoc @param "projectId" description
*/
export async function have(projectId: string): Promise<DeploymentInstanceSpec[]> {
const instances = await extensionsApi.listInstances(projectId);
return instances.map((i) => {
const dep: DeploymentInstanceSpec = {
instanceId: i.name.split("/").pop()!,

Check warning on line 113 in src/deploy/extensions/planner.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Forbidden non-null assertion
params: i.config.params,
systemParams: i.config.systemParams ?? {},
allowedEventTypes: i.config.allowedEventTypes,
Expand All @@ -129,11 +129,11 @@
/**
* wantDynamic checks the user's code for Extension SDKs usage and returns
* any extensions the user has defined that way.
* @param args.projectId The project we are deploying to

Check warning on line 132 in src/deploy/extensions/planner.ts

View workflow job for this annotation

GitHub Actions / lint (20)

@param path declaration ("args.projectId") appears before any real parameter
* @param args.projectNumber The project number we are deploying to.
* @param args.extensions The extensions section of firebase.jsonm
* @param args.extensions The extensions section of firebase.json
* @param args.emulatorMode Whether the output will be used by the Extensions emulator.
* If true, this will check {instanceId}.env.local for params a
* If true, this will check {instanceId}.env.local for params
*/
export async function wantDynamic(args: {
projectId: string;
Expand All @@ -143,6 +143,9 @@
}): Promise<DeploymentInstanceSpec[]> {
const instanceSpecs: DeploymentInstanceSpec[] = [];
const errors: FirebaseError[] = [];
if (!args.extensions) {
return [];
}
for (const [instanceId, ext] of Object.entries(args.extensions)) {
const autoPopulatedParams = await getFirebaseProjectParams(args.projectId, args.emulatorMode);
const subbedParams = substituteParams(ext.params, autoPopulatedParams);
Expand Down Expand Up @@ -189,7 +192,7 @@
/**
* want checks firebase.json and the extensions directory for which extensions
* the user wants installed on their project.
* @param args.projectId The project we are deploying to

Check warning on line 195 in src/deploy/extensions/planner.ts

View workflow job for this annotation

GitHub Actions / lint (20)

@param path declaration ("args.projectId") appears before any real parameter
* @param args.projectNumber The project number we are deploying to. Used for checking .env files.
* @param args.aliases An array of aliases for the project we are deploying to. Used for checking .env files.
* @param args.projectDir The directory containing firebase.json and extensions/
Expand All @@ -207,6 +210,9 @@
}): Promise<DeploymentInstanceSpec[]> {
const instanceSpecs: DeploymentInstanceSpec[] = [];
const errors: FirebaseError[] = [];
if (!args.extensions) {
return [];
}
for (const e of Object.entries(args.extensions)) {
try {
const instanceId = e[0];
Expand Down Expand Up @@ -257,7 +263,7 @@
eventarcChannel: eventarcChannel,
});
}
} catch (err: any) {

Check warning on line 266 in src/deploy/extensions/planner.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
logger.debug(`Got error reading extensions entry ${e}: ${err}`);
errors.push(err as FirebaseError);
}
Expand Down
4 changes: 2 additions & 2 deletions src/deploy/extensions/prepare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ export async function prepareDynamicExtensions(
projectNumber,
aliases,
projectDir,
extensions: options.config.get("extensions"),
extensions: options.config.get("extensions", {}),
});
noDeleteExtensions = noDeleteExtensions.concat(firebaseJsonWant);
if (hasNonDeployingCodebases(options)) {
Expand Down Expand Up @@ -242,7 +242,7 @@ export async function prepare(context: Context, options: Options, payload: Paylo
projectNumber,
aliases,
projectDir,
extensions: options.config.get("extensions"),
extensions: options.config.get("extensions", {}),
});
const dynamicWant = await planner.wantDynamic({
projectId,
Expand Down
Loading