From 1dba6feda0541ab6ed31f2958e6fc491c65d0121 Mon Sep 17 00:00:00 2001 From: Izaak Gough Date: Tue, 8 Sep 2026 18:21:39 +0100 Subject: [PATCH 1/3] refactor(firestore-send-email): construct MailService from the named export The default import of @sendgrid/mail only resolved through tsc's esModuleInterop helper, so the transport broke silently whenever that compiler option changed. A named import emits a plain property access and needs no helper at all. Each transport now builds its own MailService instead of calling setApiKey on the module singleton, so two transports cannot clobber each other's key. Drops the compiled-output test that guarded the interop, which the import no longer depends on. --- .../src/nodemailer-sendgrid/index.ts | 12 ++++--- .../tests/build-interop.test.ts | 31 ------------------- .../tests/helpers.test.ts | 9 ++++-- .../tests/nodemailer-sendgrid.test.ts | 10 ++++-- 4 files changed, 21 insertions(+), 41 deletions(-) delete mode 100644 kits/firestore-send-email/tests/build-interop.test.ts diff --git a/kits/firestore-send-email/src/nodemailer-sendgrid/index.ts b/kits/firestore-send-email/src/nodemailer-sendgrid/index.ts index 0f83a0f73..29d7bce41 100644 --- a/kits/firestore-send-email/src/nodemailer-sendgrid/index.ts +++ b/kits/firestore-send-email/src/nodemailer-sendgrid/index.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import sgMail from "@sendgrid/mail"; +import { MailService } from "@sendgrid/mail"; import type { Address, MailSource, @@ -29,9 +29,13 @@ export class SendGridTransport { public readonly name = "firebase-extensions-nodemailer-sendgrid"; public readonly version = "0.0.1"; + // @sendgrid/mail's default export is a shared MailService singleton, so + // setApiKey on it would reconfigure every other transport in the process. + private readonly client = new MailService(); + constructor(options: SendGridTransportOptions = {}) { if (options.apiKey) { - sgMail.setApiKey(options.apiKey); + this.client.setApiKey(options.apiKey); } } @@ -164,8 +168,8 @@ export class SendGridTransport { } } - sgMail - .send(msg as Parameters[0]) + this.client + .send(msg as Parameters[0]) .then(([response]) => { const rawQueueId = (response.headers["x-message-id"] || response.headers["X-Message-Id"]) as string | undefined; diff --git a/kits/firestore-send-email/tests/build-interop.test.ts b/kits/firestore-send-email/tests/build-interop.test.ts deleted file mode 100644 index d9e7be082..000000000 --- a/kits/firestore-send-email/tests/build-interop.test.ts +++ /dev/null @@ -1,31 +0,0 @@ -/** - * Copyright 2026 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { createRequire } from "node:module"; -import { describe, expect, test } from "vitest"; - -// The subject is the compiled output, not src: only tsc's esModuleInterop -// helper drops the prototype methods off the instance @sendgrid/mail exports, -// and vitest's own transform does not reproduce that. Needs `npm run build`, -// which CI runs before `npm test`. -const requireBuilt = createRequire(import.meta.url); - -describe("built SendGridTransport", () => { - test("reaches the methods on the @sendgrid/mail instance", () => { - const { SendGridTransport } = requireBuilt("../lib/nodemailer-sendgrid"); - expect(() => new SendGridTransport({ apiKey: "SG.test" })).not.toThrow(); - }); -}); diff --git a/kits/firestore-send-email/tests/helpers.test.ts b/kits/firestore-send-email/tests/helpers.test.ts index 035538d34..c194e60c8 100644 --- a/kits/firestore-send-email/tests/helpers.test.ts +++ b/kits/firestore-send-email/tests/helpers.test.ts @@ -18,11 +18,14 @@ import { logger } from "firebase-functions"; import Mail from "nodemailer/lib/mailer"; import { beforeEach, describe, expect, test, vi } from "vitest"; -// @sendgrid/mail exports a single MailService instance, so the mock has to be -// reachable as both the default and the named exports. +// @sendgrid/mail exports a MailService instance alongside the class, and the +// transport constructs its own. Every construction hands back the same spies. vi.mock("@sendgrid/mail", () => { const mail = { setApiKey: vi.fn(), send: vi.fn() }; - return { ...mail, default: mail }; + const MailService = vi.fn(function () { + return mail; + }); + return { ...mail, MailService, default: mail }; }); import * as sgMail from "@sendgrid/mail"; diff --git a/kits/firestore-send-email/tests/nodemailer-sendgrid.test.ts b/kits/firestore-send-email/tests/nodemailer-sendgrid.test.ts index 12824eecf..02e97ec3f 100644 --- a/kits/firestore-send-email/tests/nodemailer-sendgrid.test.ts +++ b/kits/firestore-send-email/tests/nodemailer-sendgrid.test.ts @@ -16,8 +16,9 @@ import { beforeEach, describe, expect, test, vi } from "vitest"; -// @sendgrid/mail exports a single MailService instance, so the mock has to be -// reachable as both the default and the named exports. +// @sendgrid/mail exports a single MailService instance alongside the class. +// Every construction hands back the same spies so assertions do not need a +// handle on the instance the transport built. vi.mock("@sendgrid/mail", () => { const mail = { setApiKey: vi.fn(), @@ -29,7 +30,10 @@ vi.mock("@sendgrid/mail", () => { {}, ]), }; - return { ...mail, default: mail }; + const MailService = vi.fn(function () { + return mail; + }); + return { ...mail, MailService, default: mail }; }); import * as sgMail from "@sendgrid/mail"; From 7bc92395ed1c3efc8b114628c49ba137bba59495 Mon Sep 17 00:00:00 2001 From: Izaak Gough Date: Tue, 8 Sep 2026 20:35:35 +0100 Subject: [PATCH 2/3] test(firestore-send-email): pin the transport to its own MailService The mocks returned the same spies for the singleton and for every construction, so a revert to sgMail.setApiKey passed the whole suite. Two tests now assert the transport constructs a MailService and that two transports keep separate keys without touching the singleton. Also drops `default: mail` from both mocks, unused since the transport stopped default-importing the package. --- .../tests/helpers.test.ts | 8 +++-- .../tests/nodemailer-sendgrid.test.ts | 34 ++++++++++++++++--- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/kits/firestore-send-email/tests/helpers.test.ts b/kits/firestore-send-email/tests/helpers.test.ts index c194e60c8..f2207d758 100644 --- a/kits/firestore-send-email/tests/helpers.test.ts +++ b/kits/firestore-send-email/tests/helpers.test.ts @@ -18,14 +18,16 @@ import { logger } from "firebase-functions"; import Mail from "nodemailer/lib/mailer"; import { beforeEach, describe, expect, test, vi } from "vitest"; -// @sendgrid/mail exports a MailService instance alongside the class, and the -// transport constructs its own. Every construction hands back the same spies. +// @sendgrid/mail exports a MailService singleton whose methods are also the +// module's own exports, plus the MailService class. Constructing the class +// hands back the same spies as the singleton, so the module-level ones see +// whatever the transport's own client was configured with. vi.mock("@sendgrid/mail", () => { const mail = { setApiKey: vi.fn(), send: vi.fn() }; const MailService = vi.fn(function () { return mail; }); - return { ...mail, MailService, default: mail }; + return { ...mail, MailService }; }); import * as sgMail from "@sendgrid/mail"; diff --git a/kits/firestore-send-email/tests/nodemailer-sendgrid.test.ts b/kits/firestore-send-email/tests/nodemailer-sendgrid.test.ts index 02e97ec3f..d60857aae 100644 --- a/kits/firestore-send-email/tests/nodemailer-sendgrid.test.ts +++ b/kits/firestore-send-email/tests/nodemailer-sendgrid.test.ts @@ -16,9 +16,11 @@ import { beforeEach, describe, expect, test, vi } from "vitest"; -// @sendgrid/mail exports a single MailService instance alongside the class. -// Every construction hands back the same spies so assertions do not need a -// handle on the instance the transport built. +// @sendgrid/mail exports a MailService singleton whose methods are also the +// module's own exports, plus the MailService class. Constructing the class +// hands back the same spies as the singleton, so most tests can assert against +// the module-level ones; the isolation tests override the implementation to +// tell the two apart. vi.mock("@sendgrid/mail", () => { const mail = { setApiKey: vi.fn(), @@ -33,7 +35,7 @@ vi.mock("@sendgrid/mail", () => { const MailService = vi.fn(function () { return mail; }); - return { ...mail, MailService, default: mail }; + return { ...mail, MailService }; }); import * as sgMail from "@sendgrid/mail"; @@ -47,6 +49,7 @@ import type { const setApiKey = vi.mocked(sgMail.setApiKey); const send = vi.mocked(sgMail.send); +const MailServiceMock = vi.mocked(sgMail.MailService); /** Wraps a normalized mail source in the minimal MailSource shape the transport consumes. */ function mailFrom(source: Partial): MailSource { @@ -88,6 +91,29 @@ describe("SendGridTransport", () => { expect(setApiKey).not.toHaveBeenCalled(); }); + test("builds its own MailService rather than using the module singleton", () => { + new SendGridTransport({ apiKey: "API-KEY-123" }); + expect(MailServiceMock).toHaveBeenCalledTimes(1); + }); + + test("keeps API keys separate across transports", () => { + const first = { setApiKey: vi.fn(), send: vi.fn() }; + const second = { setApiKey: vi.fn(), send: vi.fn() }; + MailServiceMock.mockImplementationOnce(function () { + return first as never; + }).mockImplementationOnce(function () { + return second as never; + }); + + new SendGridTransport({ apiKey: "FIRST" }); + new SendGridTransport({ apiKey: "SECOND" }); + + expect(first.setApiKey.mock.calls).toEqual([["FIRST"]]); + expect(second.setApiKey.mock.calls).toEqual([["SECOND"]]); + // The shared singleton the module exports is never reconfigured. + expect(setApiKey).not.toHaveBeenCalled(); + }); + test("calls back with the normalize error and never sends", async () => { const transport = new SendGridTransport({ apiKey: "X" }); const normalizeError = new Error("normalize failed"); From 7366b1b9427bde7767252cee12855de899508767 Mon Sep 17 00:00:00 2001 From: Izaak Gough Date: Wed, 9 Sep 2026 13:32:24 +0100 Subject: [PATCH 3/3] test(firestore-send-email): restore the build interop test behind a pretest hook The test requires lib/, and the release workflow's test job runs npm test without building, so it failed there. A pretest hook makes npm test build first wherever it runs. No other kit needs one, hence the note in scripts. --- kits/firestore-send-email/package.json | 2 ++ .../tests/build-interop.test.ts | 32 +++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 kits/firestore-send-email/tests/build-interop.test.ts diff --git a/kits/firestore-send-email/package.json b/kits/firestore-send-email/package.json index abf679e55..fd8ee5a2f 100644 --- a/kits/firestore-send-email/package.json +++ b/kits/firestore-send-email/package.json @@ -26,6 +26,8 @@ "scripts": { "build": "tsc -b", "clean": "tsc -b --clean", + "//pretest": "Only this kit has a test that requires lib/. The release workflow's test job runs npm test without building, so the hook has to live here rather than in CI.", + "pretest": "npm run build", "test": "vitest run", "deploy": "pnpm build && firebase deploy --only functions", "serve": "firebase emulators:start --only functions" diff --git a/kits/firestore-send-email/tests/build-interop.test.ts b/kits/firestore-send-email/tests/build-interop.test.ts new file mode 100644 index 000000000..d2014be53 --- /dev/null +++ b/kits/firestore-send-email/tests/build-interop.test.ts @@ -0,0 +1,32 @@ +/** + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { createRequire } from "node:module"; +import { describe, expect, test } from "vitest"; + +// The subject is the compiled output, not src: tsc's esModuleInterop helper +// strips the prototype methods off the instance @sendgrid/mail exports, and +// vitest's own transform does not reproduce that. The named import in +// nodemailer-sendgrid emits no helper; this guards a revert to a default one. +// The pretest hook builds lib/ before this runs. +const requireBuilt = createRequire(import.meta.url); + +describe("built SendGridTransport", () => { + test("reaches the methods on the @sendgrid/mail instance", () => { + const { SendGridTransport } = requireBuilt("../lib/nodemailer-sendgrid"); + expect(() => new SendGridTransport({ apiKey: "SG.test" })).not.toThrow(); + }); +});