Skip to content

Commit fbf2413

Browse files
authored
refactor: make resolveBackend side-effect free (#9041)
Refactors `resolveBackend` to be a pure function by removing filesystem calls to write the resolved parameter values. This separation is useful later on as we add support for things like remote source or deploying extensions through the CLI. - Made `resolveBackend` pure - no longer writes to filesystem - Added `writeResolvedParams` helper in `src/functions/env.ts` for param persistence - Updated all callsites to handle writing separately - Fixed malformed import paths (extra dots) - Added unit tests for new helper No user-visible changes. New params are still written to `.env.local` (emulator) or `.env.<projectId>` (deployment).
1 parent ad3051f commit fbf2413

File tree

7 files changed

+115
-22
lines changed

7 files changed

+115
-22
lines changed

src/deploy/functions/build.ts

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
import * as backend from "./backend";
22
import * as proto from "../../gcp/proto";
3-
import * as api from "../../.../../api";
3+
import * as api from "../../api";
44
import * as params from "./params";
55
import { FirebaseError } from "../../error";
66
import { assertExhaustive, mapObject, nullsafeVisitor } from "../../functional";
7-
import { UserEnvsOpts, writeUserEnvs } from "../../functions/env";
87
import { FirebaseConfig } from "./args";
98
import { Runtime } from "./runtimes/supported";
109
import { ExprParseError } from "./cel";
@@ -284,39 +283,26 @@ export type DynamicExtension = {
284283
interface ResolveBackendOpts {
285284
build: Build;
286285
firebaseConfig: FirebaseConfig;
287-
userEnvOpt: UserEnvsOpts;
288286
userEnvs: Record<string, string>;
289287
nonInteractive?: boolean;
290288
isEmulator?: boolean;
291289
}
292290

293291
/**
294-
* Resolves user-defined parameters inside a Build, and generates a Backend.
295-
* Returns both the Backend and the literal resolved values of any params, since
296-
* the latter also have to be uploaded so user code can see them in process.env
292+
* Resolves user-defined parameters inside a Build and generates a Backend.
293+
* Callers are responsible for persisting resolved env vars.
297294
*/
298295
export async function resolveBackend(
299296
opts: ResolveBackendOpts,
300297
): Promise<{ backend: backend.Backend; envs: Record<string, params.ParamValue> }> {
301-
let paramValues: Record<string, params.ParamValue> = {};
302-
paramValues = await params.resolveParams(
298+
const paramValues = await params.resolveParams(
303299
opts.build.params,
304300
opts.firebaseConfig,
305301
envWithTypes(opts.build.params, opts.userEnvs),
306302
opts.nonInteractive,
307303
opts.isEmulator,
308304
);
309305

310-
const toWrite: Record<string, string> = {};
311-
for (const paramName of Object.keys(paramValues)) {
312-
const paramValue = paramValues[paramName];
313-
if (Object.prototype.hasOwnProperty.call(opts.userEnvs, paramName) || paramValue.internal) {
314-
continue;
315-
}
316-
toWrite[paramName] = paramValue.toString();
317-
}
318-
writeUserEnvs(toWrite, opts.userEnvOpt);
319-
320306
return { backend: toBackend(opts.build, paramValues), envs: paramValues };
321307
}
322308

src/deploy/functions/prepare.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,12 +135,13 @@ export async function prepare(
135135
const { backend: wantBackend, envs: resolvedEnvs } = await build.resolveBackend({
136136
build: wantBuild,
137137
firebaseConfig,
138-
userEnvOpt,
139138
userEnvs,
140139
nonInteractive: options.nonInteractive,
141140
isEmulator: false,
142141
});
143142

143+
functionsEnv.writeResolvedParams(resolvedEnvs, userEnvs, userEnvOpt);
144+
144145
let hasEnvsFromParams = false;
145146
wantBackend.environmentVariables = envs;
146147
for (const envName of Object.keys(resolvedEnvs)) {

src/deploy/functions/runtimes/discovery/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import * as yaml from "yaml";
55
import { ChildProcess } from "child_process";
66

77
import { logger } from "../../../../logger";
8-
import * as api from "../../.../../../../api";
8+
import * as api from "../../../../api";
99
import * as build from "../../build";
1010
import { Runtime } from "../supported";
1111
import * as v1alpha1 from "./v1alpha1";

src/deploy/functions/runtimes/node/parseTriggers.spec.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ async function resolveBackend(bd: build.Build): Promise<backend.Backend> {
1717
storageBucket: "foo.appspot.com",
1818
databaseURL: "https://foo.firebaseio.com",
1919
},
20-
userEnvOpt: { functionsSource: "", projectId: "PROJECT" },
2120
userEnvs: {},
2221
})
2322
).backend;

src/emulator/functionsEmulator.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -580,11 +580,12 @@ export class FunctionsEmulator implements EmulatorInstance {
580580
const resolution = await resolveBackend({
581581
build: discoveredBuild,
582582
firebaseConfig: JSON.parse(firebaseConfig),
583-
userEnvOpt,
584583
userEnvs,
585584
nonInteractive: false,
586585
isEmulator: true,
587586
});
587+
588+
functionsEnv.writeResolvedParams(resolution.envs, userEnvs, userEnvOpt);
588589
const discoveredBackend = resolution.backend;
589590
const endpoints = backend.allEndpoints(discoveredBackend);
590591
prepareEndpoints(endpoints);

src/functions/env.spec.ts

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { expect } from "chai";
66

77
import * as env from "./env";
88
import { FirebaseError } from "../error";
9+
import { ParamValue } from "../deploy/functions/params";
910

1011
describe("functions/env", () => {
1112
describe("parse", () => {
@@ -758,4 +759,87 @@ FOO=foo
758759
expect(env.parseStrict(input)).to.deep.equal(expected);
759760
});
760761
});
762+
763+
describe("writeResolvedParams", () => {
764+
let tmpdir: string;
765+
766+
beforeEach(() => {
767+
tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), "firebase-functions-test-"));
768+
});
769+
770+
afterEach(() => {
771+
rmSync(tmpdir, { recursive: true, force: true });
772+
});
773+
774+
it("should write only new, non-internal params", () => {
775+
const resolvedEnvs = {
776+
EXISTING_PARAM: new ParamValue("existing", false, { string: true }),
777+
NEW_PARAM: new ParamValue("new_value", false, { string: true }),
778+
INTERNAL_PARAM: new ParamValue("internal", true, { string: true }),
779+
};
780+
const userEnvs = { EXISTING_PARAM: "old_value" };
781+
const userEnvOpt = {
782+
projectId: "test-project",
783+
functionsSource: tmpdir,
784+
};
785+
786+
env.writeResolvedParams(resolvedEnvs, userEnvs, userEnvOpt);
787+
788+
const writtenContent = fs.readFileSync(path.join(tmpdir, ".env.test-project"), "utf-8");
789+
expect(writtenContent).to.include("NEW_PARAM=new_value");
790+
expect(writtenContent).not.to.include("EXISTING_PARAM");
791+
expect(writtenContent).not.to.include("INTERNAL_PARAM");
792+
});
793+
794+
it("should not create file when no params to write", () => {
795+
const resolvedEnvs = {
796+
EXISTING_PARAM: new ParamValue("existing", false, { string: true }),
797+
INTERNAL_PARAM: new ParamValue("internal", true, { string: true }),
798+
};
799+
const userEnvs = { EXISTING_PARAM: "old_value" };
800+
const userEnvOpt = {
801+
projectId: "test-project",
802+
functionsSource: tmpdir,
803+
};
804+
805+
env.writeResolvedParams(resolvedEnvs, userEnvs, userEnvOpt);
806+
807+
const envFile = path.join(tmpdir, ".env.test-project");
808+
expect(fs.existsSync(envFile)).to.be.false;
809+
});
810+
811+
it("should write to .env.local for emulator", () => {
812+
const resolvedEnvs = {
813+
NEW_PARAM: new ParamValue("emulator_value", false, { string: true }),
814+
};
815+
const userEnvs = {};
816+
const userEnvOpt = {
817+
projectId: "test-project",
818+
functionsSource: tmpdir,
819+
isEmulator: true,
820+
};
821+
822+
env.writeResolvedParams(resolvedEnvs, userEnvs, userEnvOpt);
823+
824+
const writtenContent = fs.readFileSync(path.join(tmpdir, ".env.local"), "utf-8");
825+
expect(writtenContent).to.include("NEW_PARAM=emulator_value");
826+
expect(fs.existsSync(path.join(tmpdir, ".env.test-project"))).to.be.false;
827+
});
828+
829+
it("should handle params with special characters in values", () => {
830+
const resolvedEnvs = {
831+
NEW_PARAM: new ParamValue("value with\nnewline", false, { string: true }),
832+
};
833+
const userEnvs = {};
834+
const userEnvOpt = {
835+
projectId: "test-project",
836+
functionsSource: tmpdir,
837+
};
838+
839+
env.writeResolvedParams(resolvedEnvs, userEnvs, userEnvOpt);
840+
841+
const writtenContent = fs.readFileSync(path.join(tmpdir, ".env.test-project"), "utf-8");
842+
expect(writtenContent).to.include('NEW_PARAM="value with\\nnewline"');
843+
});
844+
});
761845
});

src/functions/env.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import * as clc from "colorette";
22
import * as fs from "fs";
33
import * as path from "path";
4+
import { ParamValue } from "../deploy/functions/params";
45

56
import { FirebaseError } from "../error";
67
import { logger } from "../logger";
@@ -414,3 +415,24 @@ export function loadFirebaseEnvs(
414415
GCLOUD_PROJECT: projectId,
415416
};
416417
}
418+
419+
/**
420+
* Writes newly resolved params to the appropriate .env file.
421+
* Skips internal params and params that already exist in userEnvs.
422+
*/
423+
export function writeResolvedParams(
424+
resolvedEnvs: Readonly<Record<string, ParamValue>>,
425+
userEnvs: Readonly<Record<string, string>>,
426+
userEnvOpt: UserEnvsOpts,
427+
): void {
428+
const toWrite: Record<string, string> = {};
429+
430+
for (const paramName of Object.keys(resolvedEnvs)) {
431+
const paramValue = resolvedEnvs[paramName];
432+
if (!paramValue.internal && !Object.prototype.hasOwnProperty.call(userEnvs, paramName)) {
433+
toWrite[paramName] = paramValue.toString();
434+
}
435+
}
436+
437+
writeUserEnvs(toWrite, userEnvOpt);
438+
}

0 commit comments

Comments
 (0)