Skip to content
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
5 changes: 5 additions & 0 deletions .changeset/lazy-firebase-admin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"files-sdk": patch
---

Avoid loading Firebase Admin when the Firebase Storage adapter receives an initialized bucket.
28 changes: 28 additions & 0 deletions packages/files-sdk/src/firebase-storage/firebase-admin-loader.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { createRequire } from "node:module";

import type {
applicationDefault,
cert,
getApps,
initializeApp,
} from "firebase-admin/app";
import type { getStorage } from "firebase-admin/storage";

const require = createRequire(import.meta.url);

interface FirebaseAdminAppModule {
applicationDefault: typeof applicationDefault;
cert: typeof cert;
getApps: typeof getApps;
initializeApp: typeof initializeApp;
}

interface FirebaseAdminStorageModule {
getStorage: typeof getStorage;
}

export const loadFirebaseAdminApp = (): FirebaseAdminAppModule =>
require("firebase-admin/app") as FirebaseAdminAppModule;

export const loadFirebaseAdminStorage = (): FirebaseAdminStorageModule =>
require("firebase-admin/storage") as FirebaseAdminStorageModule;
19 changes: 10 additions & 9 deletions packages/files-sdk/src/firebase-storage/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,6 @@ import type {
GenerateSignedPostPolicyV4Options,
} from "@google-cloud/storage";
import type { App } from "firebase-admin/app";
import {
applicationDefault,
cert,
getApps,
initializeApp,
} from "firebase-admin/app";
import { getStorage } from "firebase-admin/storage";

import type {
Adapter,
Expand All @@ -38,6 +31,10 @@ import { readEnv } from "../internal/env.js";
import { FilesError } from "../internal/errors.js";
import { createGcsResumableDriver } from "../internal/gcs-resumable.js";
import { createStoredFile } from "../internal/stored-file.js";
import {
loadFirebaseAdminApp,
loadFirebaseAdminStorage,
} from "./firebase-admin-loader.js";

export interface FirebaseStorageAdapterOptions {
/**
Expand Down Expand Up @@ -231,10 +228,12 @@ const resolveBucketName = (
// it as ours to keep the adapter's types consistent regardless of which copy
// each dependency resolves to. Without this, the build's tsc fails on Linux CI
// where the two copies diverge (7.19 vs 7.21) even though it passes on macOS.
const adminBucket = (app: App, name?: string): Bucket =>
(name
const adminBucket = (app: App, name?: string): Bucket => {
const { getStorage } = loadFirebaseAdminStorage();
return (name
? getStorage(app).bucket(name)
: getStorage(app).bucket()) as unknown as Bucket;
};

const buildBucket = (opts: FirebaseStorageAdapterOptions): Bucket => {
if (opts.app) {
Expand Down Expand Up @@ -278,6 +277,8 @@ const buildBucket = (opts: FirebaseStorageAdapterOptions): Bucket => {
const appName =
opts.appName ?? `files-sdk:${projectId ?? "default"}:${storageBucket}`;

const { applicationDefault, cert, getApps, initializeApp } =
loadFirebaseAdminApp();
const existing = getApps().find((a) => a.name === appName);
const app =
existing ??
Expand Down
16 changes: 16 additions & 0 deletions packages/files-sdk/test/build-output.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,22 @@ test(
COLD_BUILD_TIMEOUT_MS
);

// The firebase-storage entry reaches `firebase-admin` only through the
// `createRequire` loader — invisible to bundlers — so an injected `Bucket`
// works without the peer being resolvable. Guard against a top-level
// `firebase-admin` import creeping back into the graph.
test(
"firebase-storage bundle never statically imports an optional peer, even across dynamic chunks",
() => {
ensureBuilt();
const firebaseBundle = path.resolve(distDir, "firebase-storage/index.js");
Comment thread
haydenbleasel marked this conversation as resolved.
expect(
offendingOptionalPeers(firebaseBundle, { followDynamic: true })
).toEqual([]);
},
COLD_BUILD_TIMEOUT_MS
);

test(
"react bundle is a `use client` module importing only react",
() => {
Expand Down
21 changes: 19 additions & 2 deletions packages/files-sdk/test/firebase-storage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ const certMock = mock(
);
const applicationDefaultMock = mock(() => ({ _kind: "adc" }));

mock.module("firebase-admin/app", () => ({
const loadFirebaseAdminAppMock = mock(() => ({
applicationDefault: applicationDefaultMock,
cert: certMock,
getApp: getAppMock,
Expand All @@ -121,10 +121,15 @@ const getStorageMock = mock((_app?: FakeApp) => ({
bucket: (_name?: string) => fakeBucket,
}));

mock.module("firebase-admin/storage", () => ({
const loadFirebaseAdminStorageMock = mock(() => ({
getStorage: getStorageMock,
}));

mock.module("../src/firebase-storage/firebase-admin-loader.js", () => ({
loadFirebaseAdminApp: loadFirebaseAdminAppMock,
loadFirebaseAdminStorage: loadFirebaseAdminStorageMock,
}));

const { firebaseStorage, mapFirebaseStorageError } =
await import("../src/firebase-storage/index.js");

Expand Down Expand Up @@ -153,6 +158,8 @@ beforeEach(() => {
certMock.mockClear();
applicationDefaultMock.mockClear();
getStorageMock.mockClear();
loadFirebaseAdminAppMock.mockClear();
loadFirebaseAdminStorageMock.mockClear();
initializedApps.length = 0;

saveMock.mockImplementation(async () => {});
Expand Down Expand Up @@ -204,6 +211,16 @@ beforeEach(() => {
});

describe("firebase-storage adapter", () => {
test("does not load firebase-admin for an injected Bucket", () => {
firebaseStorage({
app: fakeBucket as unknown as NonNullable<
Parameters<typeof firebaseStorage>[0]
>["app"],
});
expect(loadFirebaseAdminAppMock).not.toHaveBeenCalled();
expect(loadFirebaseAdminStorageMock).not.toHaveBeenCalled();
});

test("missing bucket and projectId throws at construction", () => {
expect(() => firebaseStorage()).toThrow(/bucket/u);
});
Expand Down