diff --git a/AGENTS.md b/AGENTS.md index 43295a7cbf..ae640057db 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,7 +12,7 @@ - Follow local package patterns; check `type-test.ts`, `__tests__/`, and plugin dirs before public API/platform changes. - Start with `okf-bundle/index.md` for repo-specific implementation/testing/maintenance knowledge. - Use package indexes under `okf-bundle/packages/` for package-specific workflows and active work queues. -- Follow `okf-bundle/documentation-policy.md`: durable knowledge in reference docs; ephemeral state only in explicit work queues. +- Follow `okf-bundle/documentation-policy.md`: durable knowledge in reference docs; ephemeral state only in explicit work queues; commits are documentation; single-commit PR titles must match the commit subject exactly. - Testing entry points: `okf-bundle/testing/index.md`; validation requirements: `okf-bundle/testing/validation-checklist.md`. - Match validation to the **work type** and **validation tier** in OKF ([iteration vocabulary](okf-bundle/testing/iteration-vocabulary.md), package workflows, active work queue gates). diff --git a/okf-bundle/documentation-policy.md b/okf-bundle/documentation-policy.md index 462a82bee6..4475b4295e 100644 --- a/okf-bundle/documentation-policy.md +++ b/okf-bundle/documentation-policy.md @@ -23,9 +23,15 @@ Single source of truth for OKF knowledge and commit wording. Other OKF docs/work 2. Ephemeral state lives **only** in work queues. When an item closes, durable outcomes move to reference docs; queue rows may archive/delete. 3. Durable docs may link to a work queue for current status; do not duplicate ephemeral fields. -## Commits +## Commits as documentation -Commit messages use Conventional Commits and describe durable product/process deliverables: what changed and why, not probe IDs, gates, e2e counts, or “phase X complete”. +We treat **git commits** as durable documentation: they are the canonical record of what changed, when, and why — for humans and agents reviewing history later, not only for the current PR thread. + +Commit messages use [Conventional Commits](https://www.conventionalcommits.org/) and describe durable product/process deliverables: what changed and why, not probe IDs, gates, e2e counts, or “phase X complete”. + +## Pull requests + +When a PR contains **exactly one commit**, the **PR title must match that commit's subject line exactly** (character-for-character). Multi-commit PRs use a summary title that describes the overall change set. ## OKF update contract diff --git a/okf-bundle/index.md b/okf-bundle/index.md index 4e52f00fb2..a11d4053c1 100644 --- a/okf-bundle/index.md +++ b/okf-bundle/index.md @@ -4,7 +4,7 @@ okf_version: "0.1" # React Native Firebase knowledge bundle -* [Documentation/commit policy](/documentation-policy.md) — durable vs ephemeral, commit wording, OKF consistency +* [Documentation/commit policy](/documentation-policy.md) — durable vs ephemeral, commits as documentation, PR titles, OKF consistency # CI workflows diff --git a/packages/functions/__tests__/httpsCallable-timeout-native.test.ts b/packages/functions/__tests__/httpsCallable-timeout-native.test.ts new file mode 100644 index 0000000000..6edfcbb432 --- /dev/null +++ b/packages/functions/__tests__/httpsCallable-timeout-native.test.ts @@ -0,0 +1,76 @@ +import { afterAll, beforeAll, beforeEach, describe, expect, it, jest } from '@jest/globals'; + +jest.mock('@react-native-firebase/app/dist/module/common', () => { + const actualCommon = jest.requireActual( + '@react-native-firebase/app/dist/module/common', + ) as Record; + return { + ...actualCommon, + isOther: false, + isAndroid: true, + }; +}); + +import { firebase } from '../lib'; + +function timeoutFromNativeCall(nativeMock: jest.Mock): number { + const call = nativeMock.mock.calls[0] as unknown[] | undefined; + expect(call).toBeDefined(); + const options = call?.[4] as { timeout?: number } | undefined; + expect(options?.timeout).toBeDefined(); + return options?.timeout as number; +} + +describe('httpsCallable timeout units on native platforms', function () { + let httpsCallableNative: jest.Mock; + + beforeAll(function () { + // @ts-ignore test-only global + globalThis.RNFB_SILENCE_MODULAR_DEPRECATION_WARNINGS = true; + }); + + afterAll(function () { + // @ts-ignore test-only global + globalThis.RNFB_SILENCE_MODULAR_DEPRECATION_WARNINGS = false; + }); + + beforeEach(function () { + httpsCallableNative = jest + .fn<(...args: unknown[]) => Promise<{ data: null }>>() + .mockResolvedValue({ data: null }); + + const functions = firebase.app().functions(); + // @ts-ignore test-only internal cache + functions._nativeModule = { + httpsCallable: ( + _host: unknown, + _port: unknown, + _name: unknown, + _wrapper: unknown, + options: { timeout?: number }, + ) => httpsCallableNative(_host, _port, _name, _wrapper, options), + }; + }); + + it('converts milliseconds to seconds for httpsCallable (Android/iOS)', async function () { + const runner = firebase.app().functions().httpsCallable('example', { timeout: 30000 }); + await runner(); + + expect(httpsCallableNative).toHaveBeenCalledTimes(1); + expect(timeoutFromNativeCall(httpsCallableNative)).toBe(30); + }); + + it('does not mutate a reused options object on native platforms', async function () { + const sharedOptions = { timeout: 30000 }; + const runnerA = firebase.app().functions().httpsCallable('exampleA', sharedOptions); + const runnerB = firebase.app().functions().httpsCallable('exampleB', sharedOptions); + + await runnerA(); + await runnerB(); + + expect(sharedOptions.timeout).toBe(30000); + expect(httpsCallableNative).toHaveBeenCalledTimes(2); + expect((httpsCallableNative.mock.calls[0]?.[4] as { timeout?: number }).timeout).toBe(30); + expect((httpsCallableNative.mock.calls[1]?.[4] as { timeout?: number }).timeout).toBe(30); + }); +}); diff --git a/packages/functions/__tests__/httpsCallable-timeout-other.test.ts b/packages/functions/__tests__/httpsCallable-timeout-other.test.ts new file mode 100644 index 0000000000..24b3382e7f --- /dev/null +++ b/packages/functions/__tests__/httpsCallable-timeout-other.test.ts @@ -0,0 +1,62 @@ +import { afterAll, beforeAll, beforeEach, describe, expect, it, jest } from '@jest/globals'; + +jest.mock('@react-native-firebase/app/dist/module/common', () => { + const actualCommon = jest.requireActual( + '@react-native-firebase/app/dist/module/common', + ) as Record; + return { + ...actualCommon, + isOther: true, + isAndroid: false, + }; +}); + +import { firebase } from '../lib'; + +function timeoutFromNativeCall(nativeMock: jest.Mock): number { + const call = nativeMock.mock.calls[0] as unknown[] | undefined; + expect(call).toBeDefined(); + const options = call?.[4] as { timeout?: number } | undefined; + expect(options?.timeout).toBeDefined(); + return options?.timeout as number; +} + +describe('httpsCallable timeout units on other platforms', function () { + let httpsCallableNative: jest.Mock; + + beforeAll(function () { + // @ts-ignore test-only global + globalThis.RNFB_SILENCE_MODULAR_DEPRECATION_WARNINGS = true; + }); + + afterAll(function () { + // @ts-ignore test-only global + globalThis.RNFB_SILENCE_MODULAR_DEPRECATION_WARNINGS = false; + }); + + beforeEach(function () { + httpsCallableNative = jest + .fn<(...args: unknown[]) => Promise<{ data: null }>>() + .mockResolvedValue({ data: null }); + + const functions = firebase.app().functions(); + // @ts-ignore test-only internal cache + functions._nativeModule = { + httpsCallable: ( + _host: unknown, + _port: unknown, + _name: unknown, + _wrapper: unknown, + options: { timeout?: number }, + ) => httpsCallableNative(_host, _port, _name, _wrapper, options), + }; + }); + + it('keeps milliseconds for httpsCallable (web/macos)', async function () { + const runner = firebase.app().functions().httpsCallable('example', { timeout: 30000 }); + await runner(); + + expect(httpsCallableNative).toHaveBeenCalledTimes(1); + expect(timeoutFromNativeCall(httpsCallableNative)).toBe(30000); + }); +}); diff --git a/packages/functions/e2e/functions.e2e.js b/packages/functions/e2e/functions.e2e.js index 5818d4df05..85e892eeeb 100644 --- a/packages/functions/e2e/functions.e2e.js +++ b/packages/functions/e2e/functions.e2e.js @@ -511,6 +511,18 @@ describe('functions() modular', function () { const response = await httpsCallable(functions, fnName)(); response.data.should.equal('Hello from Firebase!'); }); + + it('HttpsCallableOptions.timeout honors millisecond values on web/macos', async function () { + if (!Platform.other) { + return this.skip(); + } + const { getApp } = modular; + const { getFunctions, httpsCallable, connectFunctionsEmulator } = functionsModular; + const functions = getFunctions(getApp(), 'us-central1'); + connectFunctionsEmulator(functions, 'localhost', 5001); + const response = await httpsCallable(functions, 'helloWorldV2', { timeout: 10000 })(); + response.data.should.equal('Hello from Firebase!'); + }); }); describe('httpsCallableFromUrl()', function () { diff --git a/packages/functions/lib/namespaced.ts b/packages/functions/lib/namespaced.ts index 4a1fbf802f..c51ef3ea94 100644 --- a/packages/functions/lib/namespaced.ts +++ b/packages/functions/lib/namespaced.ts @@ -83,6 +83,22 @@ const statics = { HttpsErrorCode, }; +function normalizeHttpsCallableTimeoutOptions(options: HttpsCallableOptions): HttpsCallableOptions { + if (!options.timeout) { + return options; + } + if (!isNumber(options.timeout)) { + throw new Error('HttpsCallableOptions.timeout expected a Number in milliseconds'); + } + if (isOther) { + return options; + } + return { + ...options, + timeout: options.timeout / 1000, + }; +} + let _id_functions_streaming_event = 0; class FirebaseFunctionsModule extends FirebaseModule { @@ -278,13 +294,7 @@ class FirebaseFunctionsModule extends FirebaseModule { } httpsCallable(name: string, options: HttpsCallableOptions = {}) { - if (options.timeout) { - if (isNumber(options.timeout)) { - options.timeout = options.timeout / 1000; - } else { - throw new Error('HttpsCallableOptions.timeout expected a Number in milliseconds'); - } - } + const normalizedOptions = normalizeHttpsCallableTimeoutOptions(options); const callableFunction = ((data?: unknown) => { const nativePromise = this.native.httpsCallable( @@ -294,7 +304,7 @@ class FirebaseFunctionsModule extends FirebaseModule { { data, }, - options, + normalizedOptions, ); return nativePromise.catch((nativeError: NativeError) => { const { code, message, details } = nativeError.userInfo || {}; @@ -314,9 +324,9 @@ class FirebaseFunctionsModule extends FirebaseModule { streamOptions?: HttpsCallableStreamOptions, ) => { const platformOptions = !isOther - ? options + ? normalizedOptions : ({ - ...options, + ...normalizedOptions, httpsCallableStreamOptions: streamOptions || {}, } as CustomHttpsCallableOptions); return this._createStreamHandler(listenerId => { @@ -335,13 +345,7 @@ class FirebaseFunctionsModule extends FirebaseModule { } httpsCallableFromUrl(url: string, options: HttpsCallableOptions = {}) { - if (options.timeout) { - if (isNumber(options.timeout)) { - options.timeout = options.timeout / 1000; - } else { - throw new Error('HttpsCallableOptions.timeout expected a Number in milliseconds'); - } - } + const normalizedOptions = normalizeHttpsCallableTimeoutOptions(options); const callableFunction = ((data?: unknown) => { const nativePromise = this.native.httpsCallableFromUrl( @@ -351,7 +355,7 @@ class FirebaseFunctionsModule extends FirebaseModule { { data, }, - options, + normalizedOptions, ); return nativePromise.catch((nativeError: NativeError) => { const { code, message, details } = nativeError.userInfo || {}; @@ -371,9 +375,9 @@ class FirebaseFunctionsModule extends FirebaseModule { streamOptions?: HttpsCallableStreamOptions, ) => { const platformOptions = !isOther - ? options + ? normalizedOptions : ({ - ...options, + ...normalizedOptions, httpsCallableStreamOptions: streamOptions || {}, } as CustomHttpsCallableOptions); return this._createStreamHandler(listenerId => {