From 1bc91c5fdfaddfad78765c2f5653c1657879aec5 Mon Sep 17 00:00:00 2001 From: Tyler Dixon Date: Mon, 27 Jul 2026 13:44:52 -0700 Subject: [PATCH 1/5] ci: add built-artifact release gate (#765, #749) Two dist-level regressions shipped as patch releases with nothing in the release flow comparing the built artifact: the 4.2.4 ObservableStatus type break (#749) and the 4.2.4/4.2.5 App Router / Vite crash caused by the CJS use-sync-external-store shim being inlined into the ESM output (#759, fixed in #760). In both cases the source was fine and only the emitted artifact regressed. Adds scripts/release-gate.mjs, run in CI as a new "Verify built artifact" job between build and publish. It verifies the packed reactfire.tgz that the publish job uploads verbatim, so it checks the exact bytes that ship, and it is dependency-free so the job needs no npm ci. Checks (#765 items 1, 2, 4, 5 plus #749): - no-cjs-in-esm: no CJS interop or dynamic require in the ESM entry - externals: externals stay external, nothing unexpected is inlined - exports-map: every path in exports/main/module/typings is in the tarball - types: emitted .d.ts match a checked-in accepted baseline - size: packed tarball and entry-point gzip within 10% of baseline Verified against the published tarballs: 4.2.5 fails no-cjs-in-esm and externals, 4.2.4 fails those plus types (surfacing the ObservableStatus union as a readable diff), and the current build passes. #749 asks for enforcement rather than a warning, but a textual diff cannot classify additive versus non-additive on its own. So the gate fails on any delta and the acknowledgement is `npm run gate:accept` plus a committed baseline, which puts the semver decision in front of a reviewer as a diff. Also repairs the size-limit config, which pointed at TSDX-era filenames (dist/reactfire.esm.js) that the vite build has not emitted for some time, so `npm run size` was silently checking nothing. Items 3 and 7 of #765 (entry-point load test, runtime smoke render) are not included; see release-gate/README.md. --- .github/workflows/test.yaml | 20 +- package.json | 10 +- release-gate/README.md | 86 ++++ release-gate/baseline/metrics.json | 13 + .../baseline/types/SuspenseSubject.d.ts | 27 ++ release-gate/baseline/types/auth.d.ts | 104 ++++ release-gate/baseline/types/database.d.ts | 23 + release-gate/baseline/types/firebaseApp.d.ts | 13 + release-gate/baseline/types/firestore.d.ts | 35 ++ release-gate/baseline/types/functions.d.ts | 12 + release-gate/baseline/types/index.d.ts | 36 ++ release-gate/baseline/types/performance.d.ts | 7 + .../baseline/types/remote-config.d.ts | 30 ++ release-gate/baseline/types/sdk.d.ts | 70 +++ release-gate/baseline/types/storage.d.ts | 25 + .../baseline/types/useObservable.d.ts | 41 ++ scripts/release-gate.mjs | 449 ++++++++++++++++++ 17 files changed, 996 insertions(+), 5 deletions(-) create mode 100644 release-gate/README.md create mode 100644 release-gate/baseline/metrics.json create mode 100644 release-gate/baseline/types/SuspenseSubject.d.ts create mode 100644 release-gate/baseline/types/auth.d.ts create mode 100644 release-gate/baseline/types/database.d.ts create mode 100644 release-gate/baseline/types/firebaseApp.d.ts create mode 100644 release-gate/baseline/types/firestore.d.ts create mode 100644 release-gate/baseline/types/functions.d.ts create mode 100644 release-gate/baseline/types/index.d.ts create mode 100644 release-gate/baseline/types/performance.d.ts create mode 100644 release-gate/baseline/types/remote-config.d.ts create mode 100644 release-gate/baseline/types/sdk.d.ts create mode 100644 release-gate/baseline/types/storage.d.ts create mode 100644 release-gate/baseline/types/useObservable.d.ts create mode 100644 scripts/release-gate.mjs diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 1d76cd68..209b4842 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -39,6 +39,24 @@ jobs: publish.sh unpack.sh retention-days: 1 + verify-package: + runs-on: ubuntu-latest + needs: build + name: Verify built artifact + steps: + - name: Checkout + uses: actions/checkout@v4 + - name: Setup node + uses: actions/setup-node@v4 + with: + node-version: '24' + - name: 'Download Artifacts' + uses: actions/download-artifact@v4 + # Checks the packed tarball the publish job uploads verbatim, so this + # verifies the exact bytes that ship. No deps needed; the gate is + # dependency-free by design. + - name: Release gate + run: node scripts/release-gate.mjs reactfire-${{ github.run_id }}/reactfire.tgz test: runs-on: ubuntu-latest needs: build @@ -105,7 +123,7 @@ jobs: publish: runs-on: ubuntu-latest name: Publish (NPM) - needs: test + needs: [test, verify-package] if: ${{ github.ref == 'refs/heads/main' || github.event_name == 'release' }} steps: - name: Setup node diff --git a/package.json b/package.json index 6b6b707f..77286258 100644 --- a/package.json +++ b/package.json @@ -32,6 +32,8 @@ "lint": "eslint src/* test/* vite.config.ts", "size": "size-limit", "analyze": "size-limit --why", + "gate": "node scripts/release-gate.mjs", + "gate:accept": "node scripts/release-gate.mjs --accept", "docs": "typedoc --options typedoc.json && markdown-toc -i docs/use.md", "docs:fork": "typedoc --options typedoc.json --gitRemote upstream && markdown-toc -i docs/use.md" }, @@ -62,12 +64,12 @@ "module": "./dist/index.js", "size-limit": [ { - "path": "dist/reactfire.cjs.production.min.js", - "limit": "10 KB" + "path": "dist/index.js", + "limit": "18 KB" }, { - "path": "dist/reactfire.esm.js", - "limit": "10 KB" + "path": "dist/index.umd.cjs", + "limit": "16 KB" } ], "devDependencies": { diff --git a/release-gate/README.md b/release-gate/README.md new file mode 100644 index 00000000..8925f7dc --- /dev/null +++ b/release-gate/README.md @@ -0,0 +1,86 @@ +# Built-artifact release gate + +Verifies the packed tarball before it is published. Runs in CI as the +**Verify built artifact** job, between `build` and `publish`, against the +`reactfire.tgz` that the publish job uploads verbatim, so it checks the exact +bytes that ship. + +Implemented by [`scripts/release-gate.mjs`](../scripts/release-gate.mjs). +Dependency-free by design: it needs no `npm ci`, so the CI job is fast and +cannot itself be broken by a dependency change. + +## Why + +Two dist-level regressions shipped as patch releases with nothing in the release +flow comparing the built artifact: + +- **4.2.4** changed `ObservableStatus` from a flat interface into a + discriminated union. Type-only, but it broke strict-TS consumers on a patch + bump ([#749](https://github.com/FirebaseExtended/reactfire/issues/749)). +- **4.2.4 / 4.2.5** bundled the CJS `use-sync-external-store/shim` into the ESM + output, producing a dynamic `require()` that threw in any browser bundle + ([#759](https://github.com/FirebaseExtended/reactfire/issues/759), fixed by + [#760](https://github.com/FirebaseExtended/reactfire/pull/760)). + +In both cases the source was fine and only the emitted artifact regressed. Both +are caught by this gate (verified by running it against the published 4.2.4 and +4.2.5 tarballs). + +## Checks + +| Check | What it catches | +| --- | --- | +| `no-cjs-in-esm` | A CJS module inlined into the ESM entry, i.e. the #759 crash. Matches the bare `require` identifier, since the output is minified and the shipped 4.2.5 bundle never literally wrote `require(`. | +| `externals` | Externals getting inlined (duplicate React instance, the shim regression) or new dependencies leaking out as runtime imports. | +| `exports-map` | Any path in `exports` / `main` / `module` / `typings` missing from the tarball. | +| `types` | Any change to the emitted `.d.ts` versus the accepted baseline, i.e. the #749 class. | +| `size` | Packed tarball and entry-point gzip size moving more than ±10%, a proxy for accidental inlining or for `files` sweeping something in. | + +## Running it + +```sh +npm run build # or: npx tsc && npx vite build +npm run gate # packs and verifies +npm run gate -- path/to/reactfire.tgz # verify a specific tarball +``` + +The local pack stages from `git ls-files` plus `dist/`, not the working tree. +`files` includes `src`, so packing the working tree directly would sweep in any +untracked scratch work under `src/`. Staging keeps a local run comparable with +what CI packs from a clean checkout. + +## Accepting a change + +`baseline/types/` and `baseline/metrics.json` are checked in. When a change to +the type surface or the bundle size is intended: + +```sh +npm run gate:accept +``` + +Then **commit the updated baseline in the same pull request**. This is the +enforcement [#749](https://github.com/FirebaseExtended/reactfire/issues/749) +asks for: a textual diff cannot classify additive versus non-additive on its +own, so instead the gate fails on any delta and the acknowledgement is a +reviewable diff. Reviewing that diff means answering: + +- Is this additive (a patch bump is fine), or non-additive (needs a minor or + major bump plus a changelog note)? + +Semantic additive/non-additive classification via api-extractor would remove the +judgement call, but it is a much larger project and deliberately not attempted +here. + +## Not covered + +Items 3 and 7 of [#765](https://github.com/FirebaseExtended/reactfire/issues/765) +are not implemented: + +- **Both entry points load** (`import('reactfire')` and `require('reactfire')` + in a fixture with `react` and `firebase` installed). +- **Runtime smoke render in CI** against a Next App Router and a Vite app. This + is the strongest check and the only one that catches runtime regressions the + static checks miss, but it is roughly a day of work on its own and adds real + CI minutes and flake surface. + +Neither is required to close the two holes that actually shipped. diff --git a/release-gate/baseline/metrics.json b/release-gate/baseline/metrics.json new file mode 100644 index 00000000..4e82535f --- /dev/null +++ b/release-gate/baseline/metrics.json @@ -0,0 +1,13 @@ +{ + "packed": 130974, + "sizes": { + "dist/index.js": { + "bytes": 63875, + "gzip": 16068 + }, + "dist/index.umd.cjs": { + "bytes": 46910, + "gzip": 14058 + } + } +} diff --git a/release-gate/baseline/types/SuspenseSubject.d.ts b/release-gate/baseline/types/SuspenseSubject.d.ts new file mode 100644 index 00000000..d7ba1a74 --- /dev/null +++ b/release-gate/baseline/types/SuspenseSubject.d.ts @@ -0,0 +1,27 @@ +import { Observable, Subject, Subscriber, Subscription } from 'rxjs'; +import { ObservableStatus } from './useObservable'; +export declare class SuspenseSubject extends Subject { + private _timeoutWindow; + private _suspenseEnabled; + private _value; + private _hasValue; + private _timeoutHandler; + private _firstEmission; + private _error; + private _innerObservable; + private _warmupSubscription; + private _immutableStatus; + private _isComplete; + private _innerSubscriber; + private _resolveFirstEmission; + constructor(innerObservable: Observable, _timeoutWindow: number, _suspenseEnabled: boolean); + get hasValue(): boolean; + get value(): T; + get firstEmission(): Promise; + private _updateImmutableStatus; + private _next; + private _reset; + _subscribe(subscriber: Subscriber): Subscription; + get ourError(): any; + get immutableStatus(): ObservableStatus; +} diff --git a/release-gate/baseline/types/auth.d.ts b/release-gate/baseline/types/auth.d.ts new file mode 100644 index 00000000..4afcc497 --- /dev/null +++ b/release-gate/baseline/types/auth.d.ts @@ -0,0 +1,104 @@ +import * as React from 'react'; +import { ReactFireOptions, ObservableStatus } from './'; +import type { Auth, User, IdTokenResult } from 'firebase/auth'; +type Claims = IdTokenResult['claims']; +export declare function preloadUser(authResolver: () => Promise): Promise; +/** + * Subscribe to Firebase auth state changes, including token refresh + * + * @param options + */ +export declare function useUser(options?: ReactFireOptions): ObservableStatus; +export declare function useIdTokenResult(user: User, forceRefresh?: boolean, options?: ReactFireOptions): ObservableStatus; +export interface AuthCheckProps { + fallback: React.ReactNode; + children: React.ReactNode; + requiredClaims?: Object; +} +export interface ClaimsCheckProps { + user: User; + fallback: React.ReactNode; + children: React.ReactNode; + requiredClaims: { + [key: string]: any; + }; +} +export interface ClaimCheckErrors { + [key: string]: any[]; +} +export type SigninCheckResult = { + signedIn: false; + hasRequiredClaims: false; + errors: {}; + user: null; +} | { + signedIn: true; + hasRequiredClaims: boolean; + errors: ClaimCheckErrors; + user: User; +}; +export interface SignInCheckOptionsBasic extends ReactFireOptions { + forceRefresh?: boolean; +} +export interface SignInCheckOptionsClaimsObject extends SignInCheckOptionsBasic { + requiredClaims: Claims; +} +export interface ClaimsValidator { + (claims: Claims): { + hasRequiredClaims: boolean; + errors: ClaimCheckErrors | {}; + }; +} +export interface SignInCheckOptionsClaimsValidator extends SignInCheckOptionsBasic { + validateCustomClaims: ClaimsValidator; +} +/** + * Subscribe to the signed-in status of a user. + * + * ```ts + * const { status, data:signInCheckResult } = useSigninCheck(); + * + * if (status === 'loading') { + * return } + * + * + * if (signInCheckResult.signedIn === true) { + * return + * } else { + * return + * } + * ``` + * + * Optionally check [custom claims](https://firebase.google.com/docs/auth/admin/custom-claims) of a user as well. + * + * ```ts + * // pass in an object describing the custom claims a user must have + * const {status, data: signInCheckResult} = useSigninCheck({requiredClaims: {admin: true}}); + * + * // pass in a custom claims validator function + * const {status, data: signInCheckResult} = useSigninCheck({validateCustomClaims: (userClaims) => { + * // custom validation logic... + * }}); + * + * // You can optionally force-refresh the token + * const {status, data: signInCheckResult} = useSigninCheck({forceRefresh: true, requiredClaims: {admin: true}}); + * ``` + */ +export declare function useSigninCheck(options?: SignInCheckOptionsBasic | SignInCheckOptionsClaimsObject | SignInCheckOptionsClaimsValidator): ObservableStatus; +/** + * @deprecated Use `useSigninCheck` instead + * + * Conditionally render children based on [custom claims](https://firebase.google.com/docs/auth/admin/custom-claims). + * + * Meant for Concurrent mode only (``). [More detail](https://github.com/FirebaseExtended/reactfire/issues/325#issuecomment-827654376). + */ +export declare function ClaimsCheck({ user, fallback, children, requiredClaims }: ClaimsCheckProps): React.JSX.Element; +/** + * @deprecated Use `useSigninCheck` instead + * + * Conditionally render children based on signed-in status and [custom claims](https://firebase.google.com/docs/auth/admin/custom-claims). + * + * Meant for Concurrent mode only (``). [More detail](https://github.com/FirebaseExtended/reactfire/issues/325#issuecomment-827654376). + */ +export declare function AuthCheck({ fallback, children, requiredClaims }: AuthCheckProps): React.ReactElement; +export {}; diff --git a/release-gate/baseline/types/database.d.ts b/release-gate/baseline/types/database.d.ts new file mode 100644 index 00000000..0d08da45 --- /dev/null +++ b/release-gate/baseline/types/database.d.ts @@ -0,0 +1,23 @@ +import { QueryChange } from 'rxfire/database'; +import { ReactFireOptions, ObservableStatus } from './'; +import type { Query as DatabaseQuery, DatabaseReference } from 'firebase/database'; +/** + * Subscribe to a Realtime Database object + * + * @param ref - Reference to the DB object you want to listen to + * @param options + */ +export declare function useDatabaseObject(ref: DatabaseReference, options?: ReactFireOptions): ObservableStatus; +export declare function useDatabaseObjectData(ref: DatabaseReference, options?: ReactFireOptions): ObservableStatus; +/** + * Subscribe to a Realtime Database list + * + * @param ref - Reference to the DB List you want to listen to + * @param options + */ +export declare function useDatabaseList(ref: DatabaseReference | DatabaseQuery, options?: ReactFireOptions): ObservableStatus; +export declare function useDatabaseListData(ref: DatabaseReference | DatabaseQuery, options?: ReactFireOptions): ObservableStatus; diff --git a/release-gate/baseline/types/firebaseApp.d.ts b/release-gate/baseline/types/firebaseApp.d.ts new file mode 100644 index 00000000..3a6eb7ae --- /dev/null +++ b/release-gate/baseline/types/firebaseApp.d.ts @@ -0,0 +1,13 @@ +import * as React from 'react'; +import type { FirebaseApp, FirebaseOptions } from 'firebase/app'; +export interface FirebaseAppProviderProps { + firebaseApp?: FirebaseApp; + firebaseConfig?: FirebaseOptions; + appName?: string; + suspense?: boolean; +} +export declare const version: string; +export declare function FirebaseAppProvider(props: React.PropsWithChildren): React.JSX.Element; +export declare function useIsSuspenseEnabled(): boolean; +export declare function useSuspenseEnabledFromConfigAndContext(suspenseFromConfig?: boolean): boolean; +export declare function useFirebaseApp(): FirebaseApp; diff --git a/release-gate/baseline/types/firestore.d.ts b/release-gate/baseline/types/firestore.d.ts new file mode 100644 index 00000000..444a5a6e --- /dev/null +++ b/release-gate/baseline/types/firestore.d.ts @@ -0,0 +1,35 @@ +import { ReactFireOptions } from './'; +import { ObservableStatus } from './useObservable'; +import { Query as FirestoreQuery, QuerySnapshot, DocumentReference, DocumentData, DocumentSnapshot } from 'firebase/firestore'; +/** + * Preload a subscription to a Firestore document reference. + * + * Use this to warm up `useFirestoreDoc` for a specific document + */ +export declare function preloadFirestoreDoc(refProvider: () => Promise): Promise>>; +/** + * Subscribe to Firestore Document changes + * + * You can preload data for this hook by calling `preloadFirestoreDoc` + */ +export declare function useFirestoreDoc(ref: DocumentReference, options?: ReactFireOptions): ObservableStatus>; +/** + * Get a firestore document and don't subscribe to changes + */ +export declare function useFirestoreDocOnce(ref: DocumentReference, options?: ReactFireOptions): ObservableStatus>; +/** + * Subscribe to Firestore Document changes and unwrap the document into a plain object + */ +export declare function useFirestoreDocData(ref: DocumentReference, options?: ReactFireOptions): ObservableStatus; +/** + * Get a Firestore document, unwrap the document into a plain object, and don't subscribe to changes + */ +export declare function useFirestoreDocDataOnce(ref: DocumentReference, options?: ReactFireOptions): ObservableStatus; +/** + * Subscribe to a Firestore collection + */ +export declare function useFirestoreCollection(query: FirestoreQuery, options?: ReactFireOptions): ObservableStatus>; +/** + * Subscribe to a Firestore collection and unwrap the snapshot into an array. + */ +export declare function useFirestoreCollectionData(query: FirestoreQuery, options?: ReactFireOptions): ObservableStatus; diff --git a/release-gate/baseline/types/functions.d.ts b/release-gate/baseline/types/functions.d.ts new file mode 100644 index 00000000..01f120c5 --- /dev/null +++ b/release-gate/baseline/types/functions.d.ts @@ -0,0 +1,12 @@ +import { ReactFireOptions, ObservableStatus } from './'; +import type { HttpsCallableOptions } from 'firebase/functions'; +/** + * Calls a callable function. + * + * @param functionName - The name of the function to call + * @param options + */ +export declare function useCallableFunctionResponse(functionName: string, options?: ReactFireOptions & { + httpsCallableOptions?: HttpsCallableOptions; + data?: RequestData; +}): ObservableStatus; diff --git a/release-gate/baseline/types/index.d.ts b/release-gate/baseline/types/index.d.ts new file mode 100644 index 00000000..41e72b29 --- /dev/null +++ b/release-gate/baseline/types/index.d.ts @@ -0,0 +1,36 @@ +import { SuspenseSubject } from './SuspenseSubject'; +import type { Query as FirestoreQuery } from 'firebase/firestore'; +import type { Query as DatabaseQuery } from 'firebase/database'; +export type ReactFireGlobals = { + _reactFireDatabaseCachedQueries: Array; + _reactFireFirestoreQueryCache: Array; + _reactFirePreloadedObservables: Map>; +}; +export declare class ReactFireError extends Error { + readonly code: string; + customData?: Record | undefined; + readonly name = "ReactFireError"; + constructor(code: string, message: string, customData?: Record | undefined); +} +export interface ReactFireOptions { + idField?: string; + initialData?: T | any; + /** + * @deprecated use initialData instead + */ + startWithValue?: T | any; + suspense?: boolean; +} +export declare function checkOptions(options: ReactFireOptions, field: string): any; +export declare function checkinitialData(options: ReactFireOptions): any; +export declare function checkIdField(options: ReactFireOptions): any; +export * from './auth'; +export * from './database'; +export * from './firebaseApp'; +export * from './firestore'; +export * from './functions'; +export * from './performance'; +export * from './remote-config'; +export * from './storage'; +export * from './useObservable'; +export * from './sdk'; diff --git a/release-gate/baseline/types/performance.d.ts b/release-gate/baseline/types/performance.d.ts new file mode 100644 index 00000000..2468b117 --- /dev/null +++ b/release-gate/baseline/types/performance.d.ts @@ -0,0 +1,7 @@ +import * as React from 'react'; +export interface SuspensePerfProps { + children: React.ReactNode; + traceId: string; + fallback: React.ReactNode; +} +export declare function SuspenseWithPerf({ children, traceId, fallback }: SuspensePerfProps): React.ReactElement; diff --git a/release-gate/baseline/types/remote-config.d.ts b/release-gate/baseline/types/remote-config.d.ts new file mode 100644 index 00000000..5f9c15c3 --- /dev/null +++ b/release-gate/baseline/types/remote-config.d.ts @@ -0,0 +1,30 @@ +import { ObservableStatus } from './useObservable'; +import { AllParameters } from 'rxfire/remote-config'; +import type { Value as RemoteConfigValue } from 'firebase/remote-config'; +/** + * Accepts a key and optionally a Remote Config instance. Returns a + * Remote Config Value. + * + * @param key The parameter key in Remote Config + */ +export declare function useRemoteConfigValue(key: string): ObservableStatus; +/** + * Convience method similar to useRemoteConfigValue. Returns a `string` from a Remote Config parameter. + * @param key The parameter key in Remote Config + */ +export declare function useRemoteConfigString(key: string): ObservableStatus; +/** + * Convience method similar to useRemoteConfigValue. Returns a `number` from a Remote Config parameter. + * @param key The parameter key in Remote Config + */ +export declare function useRemoteConfigNumber(key: string): ObservableStatus; +/** + * Convience method similar to useRemoteConfigValue. Returns a `boolean` from a Remote Config parameter. + * @param key The parameter key in Remote Config + */ +export declare function useRemoteConfigBoolean(key: string): ObservableStatus; +/** + * Convience method similar to useRemoteConfigValue. Returns allRemote Config parameters. + * @param key The parameter key in Remote Config + */ +export declare function useRemoteConfigAll(key: string): ObservableStatus; diff --git a/release-gate/baseline/types/sdk.d.ts b/release-gate/baseline/types/sdk.d.ts new file mode 100644 index 00000000..ce1e21d4 --- /dev/null +++ b/release-gate/baseline/types/sdk.d.ts @@ -0,0 +1,70 @@ +import * as React from 'react'; +import type { AppCheck } from 'firebase/app-check'; +import type { Auth } from 'firebase/auth'; +import type { Analytics } from 'firebase/analytics'; +import type { Database } from 'firebase/database'; +import type { Firestore } from 'firebase/firestore'; +import type { Functions } from 'firebase/functions'; +import type { FirebasePerformance } from 'firebase/performance'; +import type { FirebaseStorage } from 'firebase/storage'; +import type { RemoteConfig } from 'firebase/remote-config'; +import { FirebaseApp } from 'firebase/app'; +import { ObservableStatus } from './useObservable'; +import { ReactFireOptions } from '.'; +export declare const AppCheckSdkContext: React.Context; +export declare const AuthSdkContext: React.Context; +export declare const AnalyticsSdkContext: React.Context; +export declare const DatabaseSdkContext: React.Context; +export declare const FirestoreSdkContext: React.Context; +export declare const FunctionsSdkContext: React.Context; +export declare const StorageSdkContext: React.Context; +export declare const PerformanceSdkContext: React.Context; +export declare const RemoteConfigSdkContext: React.Context; +type FirebaseSdks = Analytics | AppCheck | Auth | Database | Firestore | FirebasePerformance | FirebaseStorage | Functions | RemoteConfig; +export declare const AppCheckProvider: (props: React.PropsWithChildren<{ + sdk: AppCheck; +}>) => React.ReactElement; +export declare const AuthProvider: (props: React.PropsWithChildren<{ + sdk: Auth; +}>) => React.ReactElement; +export declare const AnalyticsProvider: (props: React.PropsWithChildren<{ + sdk: Analytics; +}>) => React.ReactElement; +export declare const DatabaseProvider: (props: React.PropsWithChildren<{ + sdk: Database; +}>) => React.ReactElement; +export declare const FirestoreProvider: (props: React.PropsWithChildren<{ + sdk: Firestore; +}>) => React.ReactElement; +export declare const FunctionsProvider: (props: React.PropsWithChildren<{ + sdk: Functions; +}>) => React.ReactElement; +export declare const PerformanceProvider: (props: React.PropsWithChildren<{ + sdk: FirebasePerformance; +}>) => React.ReactElement; +export declare const StorageProvider: (props: React.PropsWithChildren<{ + sdk: FirebaseStorage; +}>) => React.ReactElement; +export declare const RemoteConfigProvider: (props: React.PropsWithChildren<{ + sdk: RemoteConfig; +}>) => React.ReactElement; +export declare const useAppCheck: () => AppCheck; +export declare const useAuth: () => Auth; +export declare const useAnalytics: () => Analytics; +export declare const useDatabase: () => Database; +export declare const useFirestore: () => Firestore; +export declare const useFunctions: () => Functions; +export declare const usePerformance: () => FirebasePerformance; +export declare const useStorage: () => FirebaseStorage; +export declare const useRemoteConfig: () => RemoteConfig; +type InitSdkHook = (initializer: (firebaseApp: FirebaseApp) => Promise, options?: ReactFireOptions) => ObservableStatus; +export declare const useInitAppCheck: InitSdkHook; +export declare const useInitAuth: InitSdkHook; +export declare const useInitAnalytics: InitSdkHook; +export declare const useInitDatabase: InitSdkHook; +export declare const useInitFirestore: InitSdkHook; +export declare const useInitFunctions: InitSdkHook; +export declare const useInitPerformance: InitSdkHook; +export declare const useInitRemoteConfig: InitSdkHook; +export declare const useInitStorage: InitSdkHook; +export {}; diff --git a/release-gate/baseline/types/storage.d.ts b/release-gate/baseline/types/storage.d.ts new file mode 100644 index 00000000..07c748bf --- /dev/null +++ b/release-gate/baseline/types/storage.d.ts @@ -0,0 +1,25 @@ +import * as React from 'react'; +import { ReactFireOptions, ObservableStatus } from './'; +import type { UploadTask, UploadTaskSnapshot, StorageReference, FirebaseStorage } from 'firebase/storage'; +/** + * Subscribe to the progress of a storage task + * + * @param task - the task you want to listen to + * @param ref - reference to the blob the task is acting on + * @param options + */ +export declare function useStorageTask(task: UploadTask, ref: StorageReference, options?: ReactFireOptions): ObservableStatus; +/** + * Subscribe to a storage ref's download URL + * + * @param ref - reference to the blob you want to download + * @param options + */ +export declare function useStorageDownloadURL(ref: StorageReference, options?: ReactFireOptions): ObservableStatus; +export type StorageImageProps = { + storagePath: string; + storage?: FirebaseStorage; + suspense?: boolean; + placeHolder?: React.ReactNode; +}; +export declare function StorageImage(props: StorageImageProps & React.DetailedHTMLProps, HTMLImageElement>): React.JSX.Element; diff --git a/release-gate/baseline/types/useObservable.d.ts b/release-gate/baseline/types/useObservable.d.ts new file mode 100644 index 00000000..b5caaf1a --- /dev/null +++ b/release-gate/baseline/types/useObservable.d.ts @@ -0,0 +1,41 @@ +import { Observable } from 'rxjs'; +import { SuspenseSubject } from './SuspenseSubject'; +import { ReactFireOptions } from './'; +export declare function preloadObservable(source: Observable, id: string, suspenseEnabled?: boolean): SuspenseSubject; +export interface ObservableStatus { + /** + * The loading status. + * + * - `loading`: Waiting for the first value from an observable + * - `error`: Something went wrong. Check `ObservableStatus.error` for more details + * - `success`: The hook has emitted at least one value + * + * If `initialData` is passed in, this will skip `loading` and go straight to `success`. + */ + status: 'loading' | 'error' | 'success'; + /** + * Indicates whether the hook has emitted a value at some point + * + * If `initialData` is passed in, this will be `true`. + */ + hasEmitted: boolean; + /** + * If this is `true`, the hook will be emitting no further items. + */ + isComplete: boolean; + /** + * The most recent value. + * + * If `initialData` is passed in, the first value of `data` will be the valuea provided in `initialData` **UNLESS** the underlying observable is ready, in which case it will skip `initialData`. + */ + data: T; + /** + * Any error that may have occurred in the underlying observable + */ + error: Error | undefined; + /** + * Promise that resolves after first emit from observable + */ + firstValuePromise: Promise; +} +export declare function useObservable(observableId: string, source: Observable, config?: ReactFireOptions): ObservableStatus; diff --git a/scripts/release-gate.mjs b/scripts/release-gate.mjs new file mode 100644 index 00000000..4149ba65 --- /dev/null +++ b/scripts/release-gate.mjs @@ -0,0 +1,449 @@ +#!/usr/bin/env node +/** + * Built-artifact release gate. + * + * Verifies the packed tarball (the exact bytes CI publishes) rather than the + * working `dist/`, so what we check is what ships. + * + * Checks: + * 1. no CJS interop / dynamic `require` in the ESM entry (issue #765, item 1) + * 2. externals stay external, nothing unexpected is inlined (issue #765, item 2) + * 3. every path in `exports`/`main`/`module`/`typings` exists (issue #765, item 4) + * 4. emitted `.d.ts` match the accepted baseline (issue #749) + * 5. bundle size stays within tolerance of the baseline (issue #765, item 5) + * + * Usage: + * node scripts/release-gate.mjs [tarball] verify (packs one if not given) + * node scripts/release-gate.mjs --accept re-record the baseline + * + * Checks 4 and 5 compare against `release-gate/baseline/`, which is checked in. + * Updating it is a deliberate act (`npm run gate:accept`) that shows the type and + * size delta as a reviewable diff in the pull request, which is what #749 asks + * for: a non-additive type change cannot land without someone acknowledging it + * and making the semver call. + */ + +import { execFileSync } from 'node:child_process'; +import { gzipSync } from 'node:zlib'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +const ROOT = path.resolve(import.meta.dirname, '..'); +const BASELINE_DIR = path.join(ROOT, 'release-gate', 'baseline'); +const BASELINE_TYPES = path.join(BASELINE_DIR, 'types'); +const BASELINE_METRICS = path.join(BASELINE_DIR, 'metrics.json'); + +// Bare specifiers the ESM build is allowed to leave as runtime imports. +// Anything else showing up here means a dependency got externalized by accident; +// anything missing from MUST_BE_EXTERNAL means one got inlined by accident. +const ALLOWED_EXTERNALS = ['react', 'react-dom', 'use-sync-external-store/shim', /^firebase(\/.*)?$/, /^@firebase\/.*$/]; + +// Regressing either of these is what shipped as 4.2.4/4.2.5: the CJS +// `use-sync-external-store/shim` got bundled into the ESM output and became a +// dynamic `require()` that throws in any browser bundle. See #759 / #760. +const MUST_BE_EXTERNAL = ['react', 'use-sync-external-store/shim']; + +// Patterns that mean a CJS module was inlined into the ESM output. +// +// Match the bare `require` identifier, not `require(`. The 4.2.5 output that +// shipped the crash never wrote `require(`: rolldown emitted `typeof require` +// guards and `require.apply(this, arguments)`. For the same reason the helper +// names below are only useful on unminified output, since minification renames +// `__commonJS` to a single letter. The identifier check is the load-bearing one. +const CJS_MARKERS = [ + { name: 'require', re: /(^|[^.\w$])require\b/g }, + { name: 'createRequire', re: /createRequire\b/g }, + { name: '__commonJS', re: /__commonJS\b/g }, + { name: '__toCommonJS', re: /__toCommonJS\b/g }, + // The exact error rolldown's require shim throws in a browser bundle (#759). + { name: 'rolldown require shim', re: /doesn't expose the `?require`? function/g }, +]; + +// Size tolerance before the gate complains, as a fraction of the baseline. +const SIZE_TOLERANCE = 0.1; + +const failures = []; +const notes = []; + +function fail(check, message, detail) { + failures.push({ check, message, detail }); +} + +function readPackageJson(dir) { + return JSON.parse(fs.readFileSync(path.join(dir, 'package.json'), 'utf8')); +} + +/** + * Pack a tarball locally and return its path. + * + * Packs from a staging dir built out of `git ls-files` plus the built `dist/`, + * not the working tree directly. `files` includes `src`, so packing the working + * tree would sweep in any untracked scratch work under src/ (and npm does not + * reliably exclude a nested node_modules). Staging keeps a local run byte- + * comparable with what CI packs from a clean checkout, which is what makes the + * recorded baseline meaningful. + */ +function pack(outDir) { + const stage = path.join(outDir, 'stage'); + const tracked = execFileSync('git', ['ls-files', '-z'], { cwd: ROOT, encoding: 'utf8', maxBuffer: 64 * 1024 * 1024 }) + .split('\0') + .filter(Boolean); + + for (const rel of tracked) { + const dest = path.join(stage, rel); + fs.mkdirSync(path.dirname(dest), { recursive: true }); + fs.copyFileSync(path.join(ROOT, rel), dest); + } + + const dist = path.join(ROOT, 'dist'); + if (!fs.existsSync(dist)) { + throw new Error('dist/ not found. Run `npx tsc && npx vite build` (or `npm run build`) first.'); + } + fs.cpSync(dist, path.join(stage, 'dist'), { recursive: true }); + + const stdout = execFileSync('npm', ['pack', '--json', '--pack-destination', outDir], { + cwd: stage, + // `npm pack` lists every packed file on stderr; drop it. + stdio: ['ignore', 'pipe', 'ignore'], + encoding: 'utf8', + maxBuffer: 64 * 1024 * 1024, + }); + // npm 8/9 emit an array here, npm 10+ an object keyed by package name. + const report = JSON.parse(stdout); + const entry = Array.isArray(report) ? report[0] : Object.values(report)[0]; + return path.join(outDir, entry.filename); +} + +/** Extract a tarball and return the path to its `package/` directory. */ +function extract(tarball, outDir) { + execFileSync('tar', ['-xzf', tarball, '-C', outDir]); + return path.join(outDir, 'package'); +} + +/** + * Collect the bare module specifiers an ESM bundle imports at runtime. + * Covers static import/export-from and dynamic import(). + */ +function collectImports(source) { + const specifiers = new Set(); + const patterns = [/\b(?:import|export)\s[\s\S]*?\bfrom\s*["']([^"']+)["']/g, /\bimport\s*["']([^"']+)["']/g, /\bimport\s*\(\s*["']([^"']+)["']\s*\)/g]; + for (const re of patterns) { + for (const match of source.matchAll(re)) { + const spec = match[1]; + // Relative and absolute specifiers resolve inside the package. + if (spec.startsWith('.') || spec.startsWith('/')) continue; + specifiers.add(spec); + } + } + return [...specifiers].sort(); +} + +function isAllowedExternal(spec) { + return ALLOWED_EXTERNALS.some((rule) => (typeof rule === 'string' ? rule === spec : rule.test(spec))); +} + +/** Minimal unified-ish diff so a type change is readable in CI logs. */ +function diffLines(before, after) { + const a = before.split('\n'); + const b = after.split('\n'); + // LCS table. The .d.ts files are small (hundreds of lines), so this is fine. + const lcs = Array.from({ length: a.length + 1 }, () => new Uint32Array(b.length + 1)); + for (let i = a.length - 1; i >= 0; i--) { + for (let j = b.length - 1; j >= 0; j--) { + lcs[i][j] = a[i] === b[j] ? lcs[i + 1][j + 1] + 1 : Math.max(lcs[i + 1][j], lcs[i][j + 1]); + } + } + const out = []; + let i = 0; + let j = 0; + while (i < a.length && j < b.length) { + if (a[i] === b[j]) { + i++; + j++; + } else if (lcs[i + 1][j] >= lcs[i][j + 1]) { + out.push(` - ${a[i++]}`); + } else { + out.push(` + ${b[j++]}`); + } + } + while (i < a.length) out.push(` - ${a[i++]}`); + while (j < b.length) out.push(` + ${b[j++]}`); + return out.join('\n'); +} + +function listTypeFiles(dir) { + if (!fs.existsSync(dir)) return []; + return fs + .readdirSync(dir) + .filter((f) => f.endsWith('.d.ts')) + .sort(); +} + +function measure(file) { + const bytes = fs.readFileSync(file); + return { bytes: bytes.length, gzip: gzipSync(bytes, { level: 9 }).length }; +} + +// --------------------------------------------------------------------------- +// Checks +// --------------------------------------------------------------------------- + +function checkNoCjsInEsm(pkgDir, pkg) { + const esmRelative = pkg.module ?? pkg.exports?.['.']?.import; + const esm = path.join(pkgDir, esmRelative); + if (!fs.existsSync(esm)) { + fail('no-cjs-in-esm', `ESM entry ${esmRelative} is missing from the package`); + return; + } + const source = fs.readFileSync(esm, 'utf8'); + for (const { name, re } of CJS_MARKERS) { + const hits = [...source.matchAll(re)]; + if (hits.length === 0) continue; + const lines = hits.slice(0, 5).map((hit) => { + const line = source.slice(0, hit.index).split('\n').length; + return ` ${esmRelative}:${line}`; + }); + fail( + 'no-cjs-in-esm', + `found ${hits.length} occurrence(s) of \`${name}\` in the ESM entry ${esmRelative}`, + [ + ...lines, + '', + ' A CJS module was inlined into the ESM build. This throws', + ' "Calling `require` for ... in an environment that doesn\'t expose', + ' the require function" in any browser bundle. Externalize it in', + ' vite.config.ts. See #759 / #760.', + ].join('\n'), + ); + } +} + +function checkExternals(pkgDir, pkg) { + const esmRelative = pkg.module ?? pkg.exports?.['.']?.import; + const esm = path.join(pkgDir, esmRelative); + if (!fs.existsSync(esm)) return; // already reported + const imports = collectImports(fs.readFileSync(esm, 'utf8')); + + const unexpected = imports.filter((spec) => !isAllowedExternal(spec)); + if (unexpected.length > 0) { + fail( + 'externals', + `unexpected external import(s) in ${esmRelative}: ${unexpected.join(', ')}`, + [ + ' These are imported at runtime but are not declared externals.', + ' Either bundle them, or add them to ALLOWED_EXTERNALS and make sure', + ' they are declared as dependencies or peerDependencies.', + ].join('\n'), + ); + } + + const inlined = MUST_BE_EXTERNAL.filter((spec) => !imports.includes(spec)); + if (inlined.length > 0) { + fail( + 'externals', + `expected external(s) no longer imported by ${esmRelative}: ${inlined.join(', ')}`, + [ + ' These must stay external. Losing the import means the module was', + ' inlined, which risks a duplicate React instance or a dynamic', + ' require() in the ESM output. See #759 / #760.', + ].join('\n'), + ); + } + + notes.push(`external imports in ${esmRelative}: ${imports.join(', ') || '(none)'}`); +} + +function checkExportsMap(pkgDir, pkg) { + const referenced = new Set(); + const walk = (node) => { + if (typeof node === 'string') { + referenced.add(node); + } else if (node && typeof node === 'object') { + Object.values(node).forEach(walk); + } + }; + walk(pkg.exports); + for (const field of ['main', 'module', 'typings', 'types', 'browser']) { + if (typeof pkg[field] === 'string') referenced.add(pkg[field]); + } + + const missing = [...referenced].filter((rel) => rel.startsWith('.')).filter((rel) => !fs.existsSync(path.join(pkgDir, rel))); + + if (missing.length > 0) { + fail( + 'exports-map', + `path(s) referenced by package.json are not in the tarball: ${missing.join(', ')}`, + [' The `files` allowlist or the build output and the exports map have', ' drifted apart. Consumers will fail to resolve these.'].join('\n'), + ); + } +} + +function checkTypes(pkgDir, { accept }) { + const distTypes = path.join(pkgDir, 'dist'); + const current = listTypeFiles(distTypes); + + if (accept) { + fs.rmSync(BASELINE_TYPES, { recursive: true, force: true }); + fs.mkdirSync(BASELINE_TYPES, { recursive: true }); + for (const file of current) { + fs.copyFileSync(path.join(distTypes, file), path.join(BASELINE_TYPES, file)); + } + console.log(`recorded ${current.length} type file(s) to release-gate/baseline/types/`); + return; + } + + const baseline = listTypeFiles(BASELINE_TYPES); + if (baseline.length === 0) { + fail('types', 'no accepted type baseline found', [' Run `npm run gate:accept` to record one, and commit the result.'].join('\n')); + return; + } + + const added = current.filter((f) => !baseline.includes(f)); + const removed = baseline.filter((f) => !current.includes(f)); + const changed = []; + for (const file of current.filter((f) => baseline.includes(f))) { + const before = fs.readFileSync(path.join(BASELINE_TYPES, file), 'utf8'); + const after = fs.readFileSync(path.join(distTypes, file), 'utf8'); + if (before !== after) changed.push({ file, diff: diffLines(before, after) }); + } + + if (added.length === 0 && removed.length === 0 && changed.length === 0) return; + + const detail = []; + if (removed.length > 0) detail.push(` removed declaration file(s): ${removed.join(', ')}`); + if (added.length > 0) detail.push(` new declaration file(s): ${added.join(', ')}`); + for (const { file, diff } of changed) { + detail.push(` --- dist/${file}`, diff); + } + detail.push( + '', + ' The published type surface changed. Decide whether this is additive', + ' (patch is fine) or non-additive (needs a minor or major bump plus a', + ' changelog note), then run `npm run gate:accept` and commit the', + ' updated baseline in the same pull request. See #749.', + ); + fail( + 'types', + `emitted .d.ts differ from the accepted baseline (${changed.length} changed, ${added.length} added, ${removed.length} removed)`, + detail.join('\n'), + ); +} + +function checkSize(pkgDir, pkg, tarball, { accept }) { + // `module` is written "./dist/index.js" and `main` "dist/index.umd.cjs"; + // normalize so the baseline keys do not churn if package.json is tidied. + const entries = [pkg.module, pkg.main].filter((rel) => typeof rel === 'string').map((rel) => rel.replace(/^\.\//, '')); + const current = {}; + for (const rel of entries) { + const file = path.join(pkgDir, rel); + if (fs.existsSync(file)) current[rel] = measure(file); + } + const packed = fs.statSync(tarball).size; + + if (accept) { + fs.mkdirSync(BASELINE_DIR, { recursive: true }); + fs.writeFileSync(BASELINE_METRICS, `${JSON.stringify({ packed, sizes: current }, null, 2)}\n`); + console.log(`recorded bundle sizes to release-gate/baseline/metrics.json`); + return; + } + + if (!fs.existsSync(BASELINE_METRICS)) { + fail('size', 'no accepted size baseline found', ' Run `npm run gate:accept` to record one, and commit the result.'); + return; + } + + const recorded = JSON.parse(fs.readFileSync(BASELINE_METRICS, 'utf8')); + + // Guards the whole tarball, not just the entry points. `files` includes + // `src`, so anything that lands under src/ ships to npm, including a nested + // node_modules if one is ever present at pack time. + if (typeof recorded.packed === 'number') { + const delta = (packed - recorded.packed) / recorded.packed; + const pct = `${delta >= 0 ? '+' : ''}${(delta * 100).toFixed(1)}%`; + notes.push(`tarball: ${packed} B (${pct} vs baseline ${recorded.packed} B)`); + if (Math.abs(delta) > SIZE_TOLERANCE) { + fail( + 'size', + `packed tarball size moved ${pct} (${recorded.packed} B -> ${packed} B), tolerance is ±${SIZE_TOLERANCE * 100}%`, + [ + ' Check what the `files` allowlist is picking up (`npm pack --dry-run`).', + ' If the change is intended, run `npm run gate:accept` and commit the', + ' updated baseline.', + ].join('\n'), + ); + } + } + + const baseline = recorded.sizes ?? {}; + for (const [rel, now] of Object.entries(current)) { + const before = baseline[rel]; + if (!before) { + fail('size', `no size baseline for ${rel}`, ' Run `npm run gate:accept` and commit the result.'); + continue; + } + const delta = (now.gzip - before.gzip) / before.gzip; + const pct = `${delta >= 0 ? '+' : ''}${(delta * 100).toFixed(1)}%`; + notes.push(`${rel}: ${now.gzip} B gzip (${pct} vs baseline ${before.gzip} B)`); + if (Math.abs(delta) > SIZE_TOLERANCE) { + fail( + 'size', + `${rel} gzip size moved ${pct} (${before.gzip} B -> ${now.gzip} B), tolerance is ±${SIZE_TOLERANCE * 100}%`, + [ + ' A jump usually means a dependency got inlined; a drop usually means', + ' something stopped being bundled. If the change is intended, run', + ' `npm run gate:accept` and commit the updated baseline.', + ].join('\n'), + ); + } + } +} + +// --------------------------------------------------------------------------- + +function main() { + const args = process.argv.slice(2); + const accept = args.includes('--accept'); + const tarballArg = args.find((a) => !a.startsWith('--')); + + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'reactfire-gate-')); + try { + const tarball = tarballArg ? path.resolve(tarballArg) : pack(tmp); + const pkgDir = extract(tarball, tmp); + const pkg = readPackageJson(pkgDir); + + console.log(`release gate: reactfire@${pkg.version} (${path.basename(tarball)})\n`); + + if (accept) { + checkTypes(pkgDir, { accept }); + checkSize(pkgDir, pkg, tarball, { accept }); + console.log('\nbaseline updated. Review the diff and commit it.'); + return 0; + } + + checkNoCjsInEsm(pkgDir, pkg); + checkExternals(pkgDir, pkg); + checkExportsMap(pkgDir, pkg); + checkTypes(pkgDir, { accept }); + checkSize(pkgDir, pkg, tarball, { accept }); + + for (const note of notes) console.log(` ${note}`); + + if (failures.length === 0) { + console.log('\nall checks passed'); + return 0; + } + + console.error(`\n${failures.length} check(s) failed:\n`); + for (const { check, message, detail } of failures) { + console.error(` [${check}] ${message}`); + if (detail) console.error(detail); + console.error(''); + } + return 1; + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } +} + +process.exit(main()); From f62e6a365ffb8a45262bde249ad403c381654e3c Mon Sep 17 00:00:00 2001 From: Tyler Dixon Date: Mon, 27 Jul 2026 15:28:50 -0700 Subject: [PATCH 2/5] test: pin release gate behavior, fix two detection gaps Follow-up to review feedback on the release gate. Three fixes, all cases where the gate quietly under-delivered. 1. Add test/release-gate.test.mjs (21 tests, no emulators needed). The gate had no tests, which is the exact failure mode it exists to prevent: edit a marker regex, break detection, and every run still goes green. A silently disarmed gate is worse than no gate because it manufactures confidence. Fixtures are byte-faithful to what shipped, including the 4.2.5 require shim, and cover the checks end to end against synthetic packages. Required a small refactor for testability: checks now take a report object instead of mutating module-level arrays, helpers are exported, and main() only runs when the script is invoked directly. 2. Fix collectImports missing every minified import form. The patterns required whitespace after the keyword, so `import{a}from"react"`, `export{a as b}from"react"` and `export*from"rxjs"` all returned nothing. It worked only because rolldown happens to emit spaced import statements while mangling identifiers, which is an incidental output detail rather than a contract. That is the same class of build-output change that caused #759, so the gate's own parser was vulnerable to what it exists to detect. Partial failure was the dangerous case: full failure trips MUST_BE_EXTERNAL loudly, but a partial format change silently weakens the unexpected-externals half. 3. Make listTypeFiles recursive. tsconfig emits with rootDir ./src, so a subdirectory of src/ emits nested declarations. The flat readdir meant dist/nextjs/*.d.ts would sit entirely outside the #749 check the moment #739 lands src/nextjs. Also adds an eslint override so plain .mjs is linted as Node with vitest globals (typescript-eslint disables no-undef for .ts, but not .mjs), and extends the lint and format globs to cover scripts/. --- .eslintrc.json | 16 +++ package.json | 5 +- release-gate/README.md | 13 ++ scripts/release-gate.mjs | 190 +++++++++++++++++++---------- test/release-gate.test.mjs | 240 +++++++++++++++++++++++++++++++++++++ 5 files changed, 396 insertions(+), 68 deletions(-) create mode 100644 test/release-gate.test.mjs diff --git a/.eslintrc.json b/.eslintrc.json index ed012a26..a5c1082b 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -23,6 +23,22 @@ "@typescript-eslint", "no-only-tests" ], + "overrides": [ + { + "files": ["**/*.mjs"], + "env": { + "browser": false, + "node": true + }, + "globals": { + "describe": "readonly", + "it": "readonly", + "expect": "readonly", + "beforeEach": "readonly", + "afterEach": "readonly" + } + } + ], "rules": { "no-only-tests/no-only-tests": "error", "@typescript-eslint/no-explicit-any": "warn", diff --git a/package.json b/package.json index 77286258..021e000d 100644 --- a/package.json +++ b/package.json @@ -28,8 +28,9 @@ "test:storage": "firebase emulators:exec --only storage --project=rxfire-525a3 \"vitest storage\"", "test:useObservable": "vitest useObservable", "test:firebaseApp": "vitest firebaseApp", - "format": "prettier src test vite.config.ts -w", - "lint": "eslint src/* test/* vite.config.ts", + "test:gate": "vitest release-gate", + "format": "prettier src test scripts vite.config.ts -w", + "lint": "eslint src/* test/* scripts/* vite.config.ts", "size": "size-limit", "analyze": "size-limit --why", "gate": "node scripts/release-gate.mjs", diff --git a/release-gate/README.md b/release-gate/README.md index 8925f7dc..ce8a1050 100644 --- a/release-gate/README.md +++ b/release-gate/README.md @@ -36,6 +36,19 @@ are caught by this gate (verified by running it against the published 4.2.4 and | `types` | Any change to the emitted `.d.ts` versus the accepted baseline, i.e. the #749 class. | | `size` | Packed tarball and entry-point gzip size moving more than ±10%, a proxy for accidental inlining or for `files` sweeping something in. | +## Tests + +The detection logic is pinned by `test/release-gate.test.mjs`: + +```sh +npm run test:gate # no emulators needed +``` + +A gate that silently stops gating is worse than no gate, so the checks are +covered by tests rather than by having been verified by hand once. The fixtures +are byte-faithful to the shapes that actually shipped, including the 4.2.5 +`require` shim. + ## Running it ```sh diff --git a/scripts/release-gate.mjs b/scripts/release-gate.mjs index 4149ba65..14862be2 100644 --- a/scripts/release-gate.mjs +++ b/scripts/release-gate.mjs @@ -21,9 +21,14 @@ * size delta as a reviewable diff in the pull request, which is what #749 asks * for: a non-additive type change cannot land without someone acknowledging it * and making the semver call. + * + * The exported helpers below are covered by test/release-gate.test.mjs. A gate + * that silently stops gating is worse than no gate, so the detection logic is + * pinned by tests rather than by having been checked by hand once. */ import { execFileSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; import { gzipSync } from 'node:zlib'; import fs from 'node:fs'; import os from 'node:os'; @@ -37,12 +42,12 @@ const BASELINE_METRICS = path.join(BASELINE_DIR, 'metrics.json'); // Bare specifiers the ESM build is allowed to leave as runtime imports. // Anything else showing up here means a dependency got externalized by accident; // anything missing from MUST_BE_EXTERNAL means one got inlined by accident. -const ALLOWED_EXTERNALS = ['react', 'react-dom', 'use-sync-external-store/shim', /^firebase(\/.*)?$/, /^@firebase\/.*$/]; +export const ALLOWED_EXTERNALS = ['react', 'react-dom', 'use-sync-external-store/shim', /^firebase(\/.*)?$/, /^@firebase\/.*$/]; // Regressing either of these is what shipped as 4.2.4/4.2.5: the CJS // `use-sync-external-store/shim` got bundled into the ESM output and became a // dynamic `require()` that throws in any browser bundle. See #759 / #760. -const MUST_BE_EXTERNAL = ['react', 'use-sync-external-store/shim']; +export const MUST_BE_EXTERNAL = ['react', 'use-sync-external-store/shim']; // Patterns that mean a CJS module was inlined into the ESM output. // @@ -51,7 +56,7 @@ const MUST_BE_EXTERNAL = ['react', 'use-sync-external-store/shim']; // guards and `require.apply(this, arguments)`. For the same reason the helper // names below are only useful on unminified output, since minification renames // `__commonJS` to a single letter. The identifier check is the load-bearing one. -const CJS_MARKERS = [ +export const CJS_MARKERS = [ { name: 'require', re: /(^|[^.\w$])require\b/g }, { name: 'createRequire', re: /createRequire\b/g }, { name: '__commonJS', re: /__commonJS\b/g }, @@ -61,13 +66,18 @@ const CJS_MARKERS = [ ]; // Size tolerance before the gate complains, as a fraction of the baseline. -const SIZE_TOLERANCE = 0.1; - -const failures = []; -const notes = []; - -function fail(check, message, detail) { - failures.push({ check, message, detail }); +export const SIZE_TOLERANCE = 0.1; + +/** Collects failures and informational notes for one run. */ +export function createReport() { + const failures = []; + const notes = []; + return { + failures, + notes, + fail: (check, message, detail) => failures.push({ check, message, detail }), + note: (message) => notes.push(message), + }; } function readPackageJson(dir) { @@ -123,14 +133,28 @@ function extract(tarball, outDir) { /** * Collect the bare module specifiers an ESM bundle imports at runtime. - * Covers static import/export-from and dynamic import(). + * Covers static import, export-from, star re-export and dynamic import(). + * + * The keyword may be followed by whitespace, `{`, `*` or a quote, because + * minified output writes `import{a}from"react"` and `export*from"rxjs"` with no + * space. Requiring whitespace here silently blinded the externals check on any + * minified build, which is the same class of build-output change that caused + * #759 in the first place. */ -function collectImports(source) { +export function collectImports(source) { const specifiers = new Set(); - const patterns = [/\b(?:import|export)\s[\s\S]*?\bfrom\s*["']([^"']+)["']/g, /\bimport\s*["']([^"']+)["']/g, /\bimport\s*\(\s*["']([^"']+)["']\s*\)/g]; + const patterns = [ + // import ... from "x" / export ... from "x" / export * from "x" + /\b(?:import|export)[\s{*]([\s\S]*?)\bfrom\s*["']([^"']+)["']/g, + // bare side-effect import: import"x" + /\bimport\s*["']([^"']+)["']/g, + // dynamic import("x") + /\bimport\s*\(\s*["']([^"']+)["']\s*\)/g, + ]; for (const re of patterns) { for (const match of source.matchAll(re)) { - const spec = match[1]; + // The first pattern captures the clause too; the specifier is always last. + const spec = match[match.length - 1]; // Relative and absolute specifiers resolve inside the package. if (spec.startsWith('.') || spec.startsWith('/')) continue; specifiers.add(spec); @@ -139,12 +163,27 @@ function collectImports(source) { return [...specifiers].sort(); } -function isAllowedExternal(spec) { +export function isAllowedExternal(spec) { return ALLOWED_EXTERNALS.some((rule) => (typeof rule === 'string' ? rule === spec : rule.test(spec))); } +/** Find CJS-interop markers in an ESM bundle. Returns one entry per marker hit. */ +export function findCjsMarkers(source) { + const found = []; + for (const { name, re } of CJS_MARKERS) { + const hits = [...source.matchAll(re)]; + if (hits.length === 0) continue; + found.push({ + name, + count: hits.length, + lines: hits.slice(0, 5).map((hit) => source.slice(0, hit.index).split('\n').length), + }); + } + return found; +} + /** Minimal unified-ish diff so a type change is readable in CI logs. */ -function diffLines(before, after) { +export function diffLines(before, after) { const a = before.split('\n'); const b = after.split('\n'); // LCS table. The .d.ts files are small (hundreds of lines), so this is fine. @@ -172,12 +211,25 @@ function diffLines(before, after) { return out.join('\n'); } -function listTypeFiles(dir) { +/** + * List declaration files under `dir`, recursively, as paths relative to `dir`. + * + * Recursive because tsconfig emits with `rootDir: ./src`, so a subdirectory of + * src/ (src/nextjs, pending #739) emits dist/nextjs/*.d.ts. A flat listing would + * leave that entire type surface silently outside the #749 check. + */ +export function listTypeFiles(dir, prefix = '') { if (!fs.existsSync(dir)) return []; - return fs - .readdirSync(dir) - .filter((f) => f.endsWith('.d.ts')) - .sort(); + const out = []; + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const rel = prefix ? `${prefix}/${entry.name}` : entry.name; + if (entry.isDirectory()) { + out.push(...listTypeFiles(path.join(dir, entry.name), rel)); + } else if (entry.name.endsWith('.d.ts')) { + out.push(rel); + } + } + return out.sort(); } function measure(file) { @@ -189,26 +241,20 @@ function measure(file) { // Checks // --------------------------------------------------------------------------- -function checkNoCjsInEsm(pkgDir, pkg) { +export function checkNoCjsInEsm(pkgDir, pkg, report) { const esmRelative = pkg.module ?? pkg.exports?.['.']?.import; const esm = path.join(pkgDir, esmRelative); if (!fs.existsSync(esm)) { - fail('no-cjs-in-esm', `ESM entry ${esmRelative} is missing from the package`); + report.fail('no-cjs-in-esm', `ESM entry ${esmRelative} is missing from the package`); return; } const source = fs.readFileSync(esm, 'utf8'); - for (const { name, re } of CJS_MARKERS) { - const hits = [...source.matchAll(re)]; - if (hits.length === 0) continue; - const lines = hits.slice(0, 5).map((hit) => { - const line = source.slice(0, hit.index).split('\n').length; - return ` ${esmRelative}:${line}`; - }); - fail( + for (const { name, count, lines } of findCjsMarkers(source)) { + report.fail( 'no-cjs-in-esm', - `found ${hits.length} occurrence(s) of \`${name}\` in the ESM entry ${esmRelative}`, + `found ${count} occurrence(s) of \`${name}\` in the ESM entry ${esmRelative}`, [ - ...lines, + ...lines.map((line) => ` ${esmRelative}:${line}`), '', ' A CJS module was inlined into the ESM build. This throws', ' "Calling `require` for ... in an environment that doesn\'t expose', @@ -219,7 +265,7 @@ function checkNoCjsInEsm(pkgDir, pkg) { } } -function checkExternals(pkgDir, pkg) { +export function checkExternals(pkgDir, pkg, report) { const esmRelative = pkg.module ?? pkg.exports?.['.']?.import; const esm = path.join(pkgDir, esmRelative); if (!fs.existsSync(esm)) return; // already reported @@ -227,7 +273,7 @@ function checkExternals(pkgDir, pkg) { const unexpected = imports.filter((spec) => !isAllowedExternal(spec)); if (unexpected.length > 0) { - fail( + report.fail( 'externals', `unexpected external import(s) in ${esmRelative}: ${unexpected.join(', ')}`, [ @@ -240,7 +286,7 @@ function checkExternals(pkgDir, pkg) { const inlined = MUST_BE_EXTERNAL.filter((spec) => !imports.includes(spec)); if (inlined.length > 0) { - fail( + report.fail( 'externals', `expected external(s) no longer imported by ${esmRelative}: ${inlined.join(', ')}`, [ @@ -251,10 +297,10 @@ function checkExternals(pkgDir, pkg) { ); } - notes.push(`external imports in ${esmRelative}: ${imports.join(', ') || '(none)'}`); + report.note(`external imports in ${esmRelative}: ${imports.join(', ') || '(none)'}`); } -function checkExportsMap(pkgDir, pkg) { +export function checkExportsMap(pkgDir, pkg, report) { const referenced = new Set(); const walk = (node) => { if (typeof node === 'string') { @@ -271,7 +317,7 @@ function checkExportsMap(pkgDir, pkg) { const missing = [...referenced].filter((rel) => rel.startsWith('.')).filter((rel) => !fs.existsSync(path.join(pkgDir, rel))); if (missing.length > 0) { - fail( + report.fail( 'exports-map', `path(s) referenced by package.json are not in the tarball: ${missing.join(', ')}`, [' The `files` allowlist or the build output and the exports map have', ' drifted apart. Consumers will fail to resolve these.'].join('\n'), @@ -279,23 +325,24 @@ function checkExportsMap(pkgDir, pkg) { } } -function checkTypes(pkgDir, { accept }) { +export function checkTypes(pkgDir, report, { accept = false, baselineTypes = BASELINE_TYPES } = {}) { const distTypes = path.join(pkgDir, 'dist'); const current = listTypeFiles(distTypes); if (accept) { - fs.rmSync(BASELINE_TYPES, { recursive: true, force: true }); - fs.mkdirSync(BASELINE_TYPES, { recursive: true }); + fs.rmSync(baselineTypes, { recursive: true, force: true }); for (const file of current) { - fs.copyFileSync(path.join(distTypes, file), path.join(BASELINE_TYPES, file)); + const dest = path.join(baselineTypes, file); + fs.mkdirSync(path.dirname(dest), { recursive: true }); + fs.copyFileSync(path.join(distTypes, file), dest); } console.log(`recorded ${current.length} type file(s) to release-gate/baseline/types/`); return; } - const baseline = listTypeFiles(BASELINE_TYPES); + const baseline = listTypeFiles(baselineTypes); if (baseline.length === 0) { - fail('types', 'no accepted type baseline found', [' Run `npm run gate:accept` to record one, and commit the result.'].join('\n')); + report.fail('types', 'no accepted type baseline found', ' Run `npm run gate:accept` to record one, and commit the result.'); return; } @@ -303,7 +350,7 @@ function checkTypes(pkgDir, { accept }) { const removed = baseline.filter((f) => !current.includes(f)); const changed = []; for (const file of current.filter((f) => baseline.includes(f))) { - const before = fs.readFileSync(path.join(BASELINE_TYPES, file), 'utf8'); + const before = fs.readFileSync(path.join(baselineTypes, file), 'utf8'); const after = fs.readFileSync(path.join(distTypes, file), 'utf8'); if (before !== after) changed.push({ file, diff: diffLines(before, after) }); } @@ -323,14 +370,14 @@ function checkTypes(pkgDir, { accept }) { ' changelog note), then run `npm run gate:accept` and commit the', ' updated baseline in the same pull request. See #749.', ); - fail( + report.fail( 'types', `emitted .d.ts differ from the accepted baseline (${changed.length} changed, ${added.length} added, ${removed.length} removed)`, detail.join('\n'), ); } -function checkSize(pkgDir, pkg, tarball, { accept }) { +export function checkSize(pkgDir, pkg, tarball, report, { accept = false, baselineMetrics = BASELINE_METRICS } = {}) { // `module` is written "./dist/index.js" and `main` "dist/index.umd.cjs"; // normalize so the baseline keys do not churn if package.json is tidied. const entries = [pkg.module, pkg.main].filter((rel) => typeof rel === 'string').map((rel) => rel.replace(/^\.\//, '')); @@ -342,18 +389,18 @@ function checkSize(pkgDir, pkg, tarball, { accept }) { const packed = fs.statSync(tarball).size; if (accept) { - fs.mkdirSync(BASELINE_DIR, { recursive: true }); - fs.writeFileSync(BASELINE_METRICS, `${JSON.stringify({ packed, sizes: current }, null, 2)}\n`); + fs.mkdirSync(path.dirname(baselineMetrics), { recursive: true }); + fs.writeFileSync(baselineMetrics, `${JSON.stringify({ packed, sizes: current }, null, 2)}\n`); console.log(`recorded bundle sizes to release-gate/baseline/metrics.json`); return; } - if (!fs.existsSync(BASELINE_METRICS)) { - fail('size', 'no accepted size baseline found', ' Run `npm run gate:accept` to record one, and commit the result.'); + if (!fs.existsSync(baselineMetrics)) { + report.fail('size', 'no accepted size baseline found', ' Run `npm run gate:accept` to record one, and commit the result.'); return; } - const recorded = JSON.parse(fs.readFileSync(BASELINE_METRICS, 'utf8')); + const recorded = JSON.parse(fs.readFileSync(baselineMetrics, 'utf8')); // Guards the whole tarball, not just the entry points. `files` includes // `src`, so anything that lands under src/ ships to npm, including a nested @@ -361,9 +408,9 @@ function checkSize(pkgDir, pkg, tarball, { accept }) { if (typeof recorded.packed === 'number') { const delta = (packed - recorded.packed) / recorded.packed; const pct = `${delta >= 0 ? '+' : ''}${(delta * 100).toFixed(1)}%`; - notes.push(`tarball: ${packed} B (${pct} vs baseline ${recorded.packed} B)`); + report.note(`tarball: ${packed} B (${pct} vs baseline ${recorded.packed} B)`); if (Math.abs(delta) > SIZE_TOLERANCE) { - fail( + report.fail( 'size', `packed tarball size moved ${pct} (${recorded.packed} B -> ${packed} B), tolerance is ±${SIZE_TOLERANCE * 100}%`, [ @@ -379,14 +426,14 @@ function checkSize(pkgDir, pkg, tarball, { accept }) { for (const [rel, now] of Object.entries(current)) { const before = baseline[rel]; if (!before) { - fail('size', `no size baseline for ${rel}`, ' Run `npm run gate:accept` and commit the result.'); + report.fail('size', `no size baseline for ${rel}`, ' Run `npm run gate:accept` and commit the result.'); continue; } const delta = (now.gzip - before.gzip) / before.gzip; const pct = `${delta >= 0 ? '+' : ''}${(delta * 100).toFixed(1)}%`; - notes.push(`${rel}: ${now.gzip} B gzip (${pct} vs baseline ${before.gzip} B)`); + report.note(`${rel}: ${now.gzip} B gzip (${pct} vs baseline ${before.gzip} B)`); if (Math.abs(delta) > SIZE_TOLERANCE) { - fail( + report.fail( 'size', `${rel} gzip size moved ${pct} (${before.gzip} B -> ${now.gzip} B), tolerance is ±${SIZE_TOLERANCE * 100}%`, [ @@ -399,6 +446,17 @@ function checkSize(pkgDir, pkg, tarball, { accept }) { } } +/** Run every read-only check against an extracted package directory. */ +export function runChecks(pkgDir, pkg, tarball, options = {}) { + const report = createReport(); + checkNoCjsInEsm(pkgDir, pkg, report); + checkExternals(pkgDir, pkg, report); + checkExportsMap(pkgDir, pkg, report); + checkTypes(pkgDir, report, options); + checkSize(pkgDir, pkg, tarball, report, options); + return report; +} + // --------------------------------------------------------------------------- function main() { @@ -415,17 +473,14 @@ function main() { console.log(`release gate: reactfire@${pkg.version} (${path.basename(tarball)})\n`); if (accept) { - checkTypes(pkgDir, { accept }); - checkSize(pkgDir, pkg, tarball, { accept }); + const report = createReport(); + checkTypes(pkgDir, report, { accept }); + checkSize(pkgDir, pkg, tarball, report, { accept }); console.log('\nbaseline updated. Review the diff and commit it.'); return 0; } - checkNoCjsInEsm(pkgDir, pkg); - checkExternals(pkgDir, pkg); - checkExportsMap(pkgDir, pkg); - checkTypes(pkgDir, { accept }); - checkSize(pkgDir, pkg, tarball, { accept }); + const { failures, notes } = runChecks(pkgDir, pkg, tarball); for (const note of notes) console.log(` ${note}`); @@ -446,4 +501,7 @@ function main() { } } -process.exit(main()); +// Only run when invoked directly, so tests can import the helpers above. +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + process.exit(main()); +} diff --git a/test/release-gate.test.mjs b/test/release-gate.test.mjs new file mode 100644 index 00000000..e4eb6a97 --- /dev/null +++ b/test/release-gate.test.mjs @@ -0,0 +1,240 @@ +import { gzipSync } from 'node:zlib'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { collectImports, findCjsMarkers, listTypeFiles, diffLines, isAllowedExternal, runChecks } from '../scripts/release-gate.mjs'; + +/** + * The release gate exists to catch dist-level regressions that shipped as patch + * releases (#749, #759). A gate that silently stops gating is worse than no + * gate, so these tests pin the detection logic against the shapes that actually + * shipped rather than relying on it having been verified by hand once. + */ + +// Condensed from the published reactfire@4.2.5 dist/index.js, which crashed in +// any browser bundle. Note it never writes `require(`: rolldown emits `typeof +// require` guards and `require.apply`. See #759 / #760. +// Built from single-quoted lines so the backticks below stay byte-faithful to +// the published file; a template literal would need them escaped. +const ESM_WITH_INLINED_CJS = [ + 'import * as e from "react";', + 'var oe = (e, t) => () => (t || (e((t = { exports: {} }).exports, t), e = null), t.exports), se = ((e) => typeof require < "u" ? require : e)(function(e) {', + '\tif (typeof require < "u") return require.apply(this, arguments);', + '\tthrow Error("Calling `require` for \\"" + e + "\\" in an environment that doesn\'t expose the `require` function. See https://rolldown.rs/in-depth/bundling-cjs for more details.");', + '});', +].join('\n'); + +// The shape 4.2.6 ships: the shim stays an external import, no require anywhere. +const ESM_CLEAN = ` +import * as e from "react"; +import { useSyncExternalStore as m } from "use-sync-external-store/shim"; +import { getApps as n } from "firebase/app"; +export { m as useObservable }; +`; + +describe('collectImports', () => { + it('finds specifiers in unminified import statements', () => { + expect(collectImports('import { a } from "react";')).toEqual(['react']); + }); + + // Regression: requiring whitespace after the keyword silently blinded the + // externals check on any minified build, which is the same class of + // build-output change that caused #759. + it('finds specifiers in minified forms with no whitespace', () => { + expect(collectImports('import{a}from"react";')).toEqual(['react']); + expect(collectImports('export{a as b}from"react";')).toEqual(['react']); + expect(collectImports('export*from"rxjs";')).toEqual(['rxjs']); + expect(collectImports('import*as e from"react";')).toEqual(['react']); + }); + + it('finds side-effect and dynamic imports', () => { + expect(collectImports('import"./polyfill";import("firebase/auth");')).toEqual(['firebase/auth']); + }); + + it('ignores relative and absolute specifiers', () => { + expect(collectImports('import { a } from "./local";import { b } from "/abs";')).toEqual([]); + }); + + it('reports the externals of a clean build', () => { + expect(collectImports(ESM_CLEAN)).toEqual(['firebase/app', 'react', 'use-sync-external-store/shim']); + }); +}); + +describe('isAllowedExternal', () => { + it('allows react, the shim, and firebase entry points', () => { + for (const spec of ['react', 'use-sync-external-store/shim', 'firebase', 'firebase/auth', '@firebase/app']) { + expect(isAllowedExternal(spec)).toBe(true); + } + }); + + it('rejects anything else', () => { + for (const spec of ['rxjs', 'rxfire/firestore', 'lodash']) { + expect(isAllowedExternal(spec)).toBe(false); + } + }); +}); + +describe('findCjsMarkers', () => { + it('detects the inlined CJS shim that shipped in 4.2.5', () => { + const names = findCjsMarkers(ESM_WITH_INLINED_CJS).map((m) => m.name); + expect(names).toContain('require'); + expect(names).toContain('rolldown require shim'); + }); + + it('reports line numbers for the first few hits', () => { + const [first] = findCjsMarkers(ESM_WITH_INLINED_CJS); + expect(first.count).toBeGreaterThan(0); + expect(first.lines.length).toBeGreaterThan(0); + expect(first.lines.length).toBeLessThanOrEqual(5); + }); + + it('stays quiet on a clean ESM bundle', () => { + expect(findCjsMarkers(ESM_CLEAN)).toEqual([]); + }); + + it('does not match property access or a longer word', () => { + expect(findCjsMarkers('foo.require;const required = 1;')).toEqual([]); + }); +}); + +describe('listTypeFiles', () => { + let dir; + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gate-types-')); + }); + + afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + + it('lists declaration files and ignores everything else', () => { + fs.writeFileSync(path.join(dir, 'index.d.ts'), ''); + fs.writeFileSync(path.join(dir, 'index.js'), ''); + expect(listTypeFiles(dir)).toEqual(['index.d.ts']); + }); + + // tsconfig emits with rootDir ./src, so src/nextjs (pending #739) would emit + // dist/nextjs/*.d.ts. A flat listing left that surface outside the #749 check. + it('recurses into subdirectories', () => { + fs.mkdirSync(path.join(dir, 'nextjs')); + fs.writeFileSync(path.join(dir, 'index.d.ts'), ''); + fs.writeFileSync(path.join(dir, 'nextjs', 'middleware.d.ts'), ''); + expect(listTypeFiles(dir)).toEqual(['index.d.ts', 'nextjs/middleware.d.ts']); + }); + + it('returns nothing for a missing directory', () => { + expect(listTypeFiles(path.join(dir, 'nope'))).toEqual([]); + }); +}); + +describe('diffLines', () => { + it('marks removed and added lines', () => { + const diff = diffLines('a\nb\nc', 'a\nB\nc'); + expect(diff).toContain('- b'); + expect(diff).toContain('+ B'); + }); + + it('is empty when the inputs match', () => { + expect(diffLines('a\nb', 'a\nb')).toBe(''); + }); +}); + +describe('runChecks', () => { + let dir; + + const build = ({ esm, types = { 'index.d.ts': 'export declare const a: string;\n' } }) => { + const pkgDir = path.join(dir, 'package'); + fs.mkdirSync(path.join(pkgDir, 'dist'), { recursive: true }); + fs.writeFileSync(path.join(pkgDir, 'dist', 'index.js'), esm); + fs.writeFileSync(path.join(pkgDir, 'dist', 'index.umd.cjs'), 'module.exports = {};'); + for (const [file, contents] of Object.entries(types)) { + fs.writeFileSync(path.join(pkgDir, 'dist', file), contents); + } + + const pkg = { + name: 'reactfire', + version: '0.0.0-test', + module: './dist/index.js', + main: 'dist/index.umd.cjs', + typings: 'dist/index.d.ts', + exports: { '.': { import: './dist/index.js', require: './dist/index.umd.cjs' } }, + }; + fs.writeFileSync(path.join(pkgDir, 'package.json'), JSON.stringify(pkg)); + + // Stand-ins for the real tarball and baseline; sizes are recorded from this + // build so the size check is a no-op and the other checks are isolated. + const tarball = path.join(dir, 'fake.tgz'); + fs.writeFileSync(tarball, 'x'.repeat(1000)); + + const baselineTypes = path.join(dir, 'baseline-types'); + fs.mkdirSync(baselineTypes, { recursive: true }); + for (const [file, contents] of Object.entries(types)) { + fs.writeFileSync(path.join(baselineTypes, file), contents); + } + + const gzipOf = (p) => gzipSync(fs.readFileSync(p), { level: 9 }).length; + const baselineMetrics = path.join(dir, 'metrics.json'); + fs.writeFileSync( + baselineMetrics, + JSON.stringify({ + packed: 1000, + sizes: { + 'dist/index.js': { gzip: gzipOf(path.join(pkgDir, 'dist', 'index.js')) }, + 'dist/index.umd.cjs': { gzip: gzipOf(path.join(pkgDir, 'dist', 'index.umd.cjs')) }, + }, + }), + ); + + return { pkgDir, pkg, tarball, options: { baselineTypes, baselineMetrics } }; + }; + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gate-run-')); + }); + + afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + + it('passes a clean build', () => { + const { pkgDir, pkg, tarball, options } = build({ esm: ESM_CLEAN }); + const { failures } = runChecks(pkgDir, pkg, tarball, options); + expect(failures).toEqual([]); + }); + + it('fails the 4.2.5 shape on both the CJS and externals checks', () => { + const { pkgDir, pkg, tarball, options } = build({ esm: ESM_WITH_INLINED_CJS }); + const { failures } = runChecks(pkgDir, pkg, tarball, options); + const checks = new Set(failures.map((f) => f.check)); + expect(checks).toContain('no-cjs-in-esm'); + expect(checks).toContain('externals'); + // The shim was inlined, so it is no longer imported. + expect(failures.find((f) => f.check === 'externals').message).toContain('use-sync-external-store/shim'); + }); + + it('fails when a dependency leaks out as an unexpected external', () => { + const { pkgDir, pkg, tarball, options } = build({ esm: `${ESM_CLEAN}\nimport { map } from "rxjs";` }); + const { failures } = runChecks(pkgDir, pkg, tarball, options); + const externals = failures.find((f) => f.check === 'externals'); + expect(externals?.message).toContain('rxjs'); + }); + + it('fails when the emitted types drift from the baseline', () => { + const { pkgDir, pkg, tarball, options } = build({ esm: ESM_CLEAN }); + fs.writeFileSync(path.join(pkgDir, 'dist', 'useObservable.d.ts'), 'export declare const changed: number;\n'); + fs.writeFileSync(path.join(options.baselineTypes, 'useObservable.d.ts'), 'export declare const original: string;\n'); + const { failures } = runChecks(pkgDir, pkg, tarball, options); + const types = failures.find((f) => f.check === 'types'); + expect(types).toBeDefined(); + expect(types.detail).toContain('- export declare const original: string;'); + expect(types.detail).toContain('+ export declare const changed: number;'); + }); + + it('fails when a path in the exports map is missing from the package', () => { + const { pkgDir, pkg, tarball, options } = build({ esm: ESM_CLEAN }); + fs.rmSync(path.join(pkgDir, 'dist', 'index.umd.cjs')); + const { failures } = runChecks(pkgDir, pkg, tarball, options); + expect(failures.find((f) => f.check === 'exports-map')?.message).toContain('dist/index.umd.cjs'); + }); +}); From 68fb6a0c8a447585d3a0026b917cf1739490114a Mon Sep 17 00:00:00 2001 From: Tyler Dixon Date: Mon, 27 Jul 2026 16:31:37 -0700 Subject: [PATCH 3/5] fix: close five gaps in the release gate found in review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An independent review found the gate claimed broader coverage than it had. All five are fixed, and every fix is pinned by a test that fails when the fix is reverted (verified by mutating each one back). 1. exports-map never checked `main` or `typings`. It filtered to paths starting with ".", but `main` is "dist/index.umd.cjs" and `typings` is "dist/index.d.ts", neither of which has the prefix. So the two fields its own failure message named were the two it skipped. Probed directly: with both missing and no exports map, it reported zero failures. It passed until now only because exports['.'].require duplicates `main`; `typings` has no equivalent and was genuinely unchecked. The test that appeared to cover this deleted the one path that was covered, so it passed for the wrong reason. 2. The size tolerance was too loose to catch what it advertised. Measured from the published tarballs, the #759 shim inlining moved the ESM entry +3.1% gzip and the tarball +0.3%, both well inside the old ±10% band. The claim that a size guard "might well have caught the shim getting inlined" was wrong, and is corrected in the README: size is a coarse guard against gross packaging changes, not an inlining detector. Tolerance is now ±2%, above CI's version-stamp churn (+0.5% tarball) and below a #759-sized change. The check also had zero test coverage; deleting it outright left every test green. It is now covered, including at the #759 magnitude. 3. Added and removed .d.ts detection was unpinned. The logic was correct but nothing held it there. This is the case the recursion fix in the previous commit exists to feed: a new entry point emitting dist/nextjs/*.d.ts is exactly #739, and a disappearing declaration file is a hard breaking change. 4. Both bundle checks only ever opened the `module` entry. Nothing guarantees a single ESM chunk. A second entry point or rollup deciding to split would put code where an inlined require is invisible, and chunking is the kind of incidental build-output change this gate exists to be robust against. Both checks now walk every ESM file in the package, and expected externals are matched across their union so an import from a chunk still counts. 5. `react` was matched as an exact string, and --accept took a tarball. Switching to the automatic JSX runtime would import react/jsx-runtime and double false positive: unexpected external, plus "react was inlined" when it was not. Now matched as a pattern, like firebase already was. Separately, `--accept ` would have recorded a baseline from an arbitrary old build, blessing whatever regression it contained; it is now rejected, as are unknown flags. --- release-gate/README.md | 22 ++++- scripts/release-gate.mjs | 160 +++++++++++++++++++++++++++------- test/release-gate.test.mjs | 174 ++++++++++++++++++++++++++++++++++++- 3 files changed, 318 insertions(+), 38 deletions(-) diff --git a/release-gate/README.md b/release-gate/README.md index ce8a1050..0f1a7781 100644 --- a/release-gate/README.md +++ b/release-gate/README.md @@ -30,11 +30,27 @@ are caught by this gate (verified by running it against the published 4.2.4 and | Check | What it catches | | --- | --- | -| `no-cjs-in-esm` | A CJS module inlined into the ESM entry, i.e. the #759 crash. Matches the bare `require` identifier, since the output is minified and the shipped 4.2.5 bundle never literally wrote `require(`. | +| `no-cjs-in-esm` | A CJS module inlined into **any** ESM chunk, i.e. the #759 crash. Matches the bare `require` identifier, since the output is minified and the shipped 4.2.5 bundle never literally wrote `require(`. | | `externals` | Externals getting inlined (duplicate React instance, the shim regression) or new dependencies leaking out as runtime imports. | | `exports-map` | Any path in `exports` / `main` / `module` / `typings` missing from the tarball. | -| `types` | Any change to the emitted `.d.ts` versus the accepted baseline, i.e. the #749 class. | -| `size` | Packed tarball and entry-point gzip size moving more than ±10%, a proxy for accidental inlining or for `files` sweeping something in. | +| `types` | Any change to the emitted `.d.ts` versus the accepted baseline, i.e. the #749 class. Covers added and removed declaration files as well as changed ones. | +| `size` | Packed tarball and entry-point gzip size moving more than ±2%. | + +### What `size` is and is not + +It is a coarse guard against gross packaging changes, for example `files` +sweeping in something it should not. **It is not a reliable inlining detector**, +and the table above should not be read as claiming otherwise. + +Measured against the real artifacts, the #759 shim inlining moved the ESM entry +only **+3.1%** gzip (16815 to 17330 B) and the packed tarball **+0.3%**. An +earlier draft of this gate used a ±10% band, which would have missed that +entirely. The tolerance is now ±2%: tight enough to see a #759-sized change, +loose enough to clear the version stamp CI writes into the bundle (measured at ++0.5% on the tarball, +0.2% on the entries). + +`no-cjs-in-esm` and `externals` are what actually catch inlining. Treat `size` +as a backstop. ## Tests diff --git a/scripts/release-gate.mjs b/scripts/release-gate.mjs index 14862be2..847c627b 100644 --- a/scripts/release-gate.mjs +++ b/scripts/release-gate.mjs @@ -42,12 +42,22 @@ const BASELINE_METRICS = path.join(BASELINE_DIR, 'metrics.json'); // Bare specifiers the ESM build is allowed to leave as runtime imports. // Anything else showing up here means a dependency got externalized by accident; // anything missing from MUST_BE_EXTERNAL means one got inlined by accident. -export const ALLOWED_EXTERNALS = ['react', 'react-dom', 'use-sync-external-store/shim', /^firebase(\/.*)?$/, /^@firebase\/.*$/]; +// Subpaths are matched too: switching to the automatic JSX runtime makes the +// build import `react/jsx-runtime`, which is legitimate and must not be +// reported as an unexpected external. +export const ALLOWED_EXTERNALS = [/^react(\/.*)?$/, /^react-dom(\/.*)?$/, 'use-sync-external-store/shim', /^firebase(\/.*)?$/, /^@firebase\/.*$/]; // Regressing either of these is what shipped as 4.2.4/4.2.5: the CJS // `use-sync-external-store/shim` got bundled into the ESM output and became a // dynamic `require()` that throws in any browser bundle. See #759 / #760. -export const MUST_BE_EXTERNAL = ['react', 'use-sync-external-store/shim']; +// +// Matched as patterns rather than exact strings so that `react/jsx-runtime` +// counts as react still being external. An exact match would report "react was +// inlined" the moment the build switched to the automatic JSX runtime. +export const MUST_BE_EXTERNAL = [ + { label: 'react', re: /^react(\/.*)?$/ }, + { label: 'use-sync-external-store/shim', re: /^use-sync-external-store\/shim$/ }, +]; // Patterns that mean a CJS module was inlined into the ESM output. // @@ -66,7 +76,18 @@ export const CJS_MARKERS = [ ]; // Size tolerance before the gate complains, as a fraction of the baseline. -export const SIZE_TOLERANCE = 0.1; +// +// Deliberately tight. Measured against the real artifacts, the #759 shim +// inlining moved the ESM entry only +3.1% gzip (16815 -> 17330 B) and the +// packed tarball +0.3%, so a 10% band would have missed it entirely. Version +// churn is the noise floor this has to clear: a CI build stamps the version +// into the bundle, which measured at +0.5% on the tarball and +0.2% on the +// entries, so 2% leaves room for that and little else. +// +// This is a coarse guard against gross packaging changes, not a reliable +// inlining detector. `no-cjs-in-esm` and `externals` are what actually catch +// inlining; see release-gate/README.md. +export const SIZE_TOLERANCE = 0.02; /** Collects failures and informational notes for one run. */ export function createReport() { @@ -237,45 +258,95 @@ function measure(file) { return { bytes: bytes.length, gzip: gzipSync(bytes, { level: 9 }).length }; } +/** Strip a leading "./" so package.json fields and baseline keys agree. */ +export function normalizeRelative(rel) { + return rel.replace(/^\.\//, ''); +} + +/** + * Every ESM file the package ships, relative to the package root. + * + * Not just the `module` entry. Nothing guarantees the build emits a single + * chunk, and a second entry point (src/nextjs, pending #739) or rollup deciding + * to split would put code outside the entry where a CJS inline would be + * invisible. Chunking is exactly the kind of incidental build-output change + * this gate exists to be robust against. + */ +export function listEsmFiles(pkgDir, pkg) { + const entry = normalizeRelative(pkg.module ?? pkg.exports?.['.']?.import ?? ''); + if (!entry) return []; + const dir = path.dirname(entry); + const cjs = pkg.main ? normalizeRelative(pkg.main) : null; + + const walk = (abs, rel) => { + if (!fs.existsSync(abs)) return []; + const out = []; + for (const item of fs.readdirSync(abs, { withFileTypes: true })) { + const childRel = rel ? `${rel}/${item.name}` : item.name; + if (item.isDirectory()) { + out.push(...walk(path.join(abs, item.name), childRel)); + } else if (/\.(js|mjs)$/.test(item.name) && childRel !== cjs) { + out.push(childRel); + } + } + return out; + }; + + // The entry sorts first so its findings are reported before other chunks'. + return walk(path.join(pkgDir, dir), dir).sort((a, b) => (a === entry ? -1 : b === entry ? 1 : a.localeCompare(b))); +} + // --------------------------------------------------------------------------- // Checks // --------------------------------------------------------------------------- export function checkNoCjsInEsm(pkgDir, pkg, report) { - const esmRelative = pkg.module ?? pkg.exports?.['.']?.import; - const esm = path.join(pkgDir, esmRelative); - if (!fs.existsSync(esm)) { - report.fail('no-cjs-in-esm', `ESM entry ${esmRelative} is missing from the package`); + const files = listEsmFiles(pkgDir, pkg); + if (files.length === 0) { + const entry = pkg.module ?? pkg.exports?.['.']?.import; + report.fail('no-cjs-in-esm', `ESM entry ${entry} is missing from the package`); return; } - const source = fs.readFileSync(esm, 'utf8'); - for (const { name, count, lines } of findCjsMarkers(source)) { - report.fail( - 'no-cjs-in-esm', - `found ${count} occurrence(s) of \`${name}\` in the ESM entry ${esmRelative}`, - [ - ...lines.map((line) => ` ${esmRelative}:${line}`), - '', - ' A CJS module was inlined into the ESM build. This throws', - ' "Calling `require` for ... in an environment that doesn\'t expose', - ' the require function" in any browser bundle. Externalize it in', - ' vite.config.ts. See #759 / #760.', - ].join('\n'), - ); + + for (const rel of files) { + const source = fs.readFileSync(path.join(pkgDir, rel), 'utf8'); + for (const { name, count, lines } of findCjsMarkers(source)) { + report.fail( + 'no-cjs-in-esm', + `found ${count} occurrence(s) of \`${name}\` in ${rel}`, + [ + ...lines.map((line) => ` ${rel}:${line}`), + '', + ' A CJS module was inlined into the ESM build. This throws', + ' "Calling `require` for ... in an environment that doesn\'t expose', + ' the require function" in any browser bundle. Externalize it in', + ' vite.config.ts. See #759 / #760.', + ].join('\n'), + ); + } } } export function checkExternals(pkgDir, pkg, report) { - const esmRelative = pkg.module ?? pkg.exports?.['.']?.import; - const esm = path.join(pkgDir, esmRelative); - if (!fs.existsSync(esm)) return; // already reported - const imports = collectImports(fs.readFileSync(esm, 'utf8')); + const files = listEsmFiles(pkgDir, pkg); + if (files.length === 0) return; // already reported + + // Union across every chunk: a split build can import react from a chunk + // rather than from the entry, and that is still react staying external. + const byFile = new Map(); + const all = new Set(); + for (const rel of files) { + const imports = collectImports(fs.readFileSync(path.join(pkgDir, rel), 'utf8')); + byFile.set(rel, imports); + imports.forEach((spec) => all.add(spec)); + } - const unexpected = imports.filter((spec) => !isAllowedExternal(spec)); - if (unexpected.length > 0) { + for (const [rel, imports] of byFile) { + const unexpected = imports.filter((spec) => !isAllowedExternal(spec)); + if (unexpected.length === 0) continue; report.fail( 'externals', - `unexpected external import(s) in ${esmRelative}: ${unexpected.join(', ')}`, + `unexpected external import(s) in ${rel}: ${unexpected.join(', ')}`, [ ' These are imported at runtime but are not declared externals.', ' Either bundle them, or add them to ALLOWED_EXTERNALS and make sure', @@ -284,11 +355,11 @@ export function checkExternals(pkgDir, pkg, report) { ); } - const inlined = MUST_BE_EXTERNAL.filter((spec) => !imports.includes(spec)); + const inlined = MUST_BE_EXTERNAL.filter(({ re }) => ![...all].some((spec) => re.test(spec))); if (inlined.length > 0) { report.fail( 'externals', - `expected external(s) no longer imported by ${esmRelative}: ${inlined.join(', ')}`, + `expected external(s) no longer imported by the ESM output: ${inlined.map((m) => m.label).join(', ')}`, [ ' These must stay external. Losing the import means the module was', ' inlined, which risks a duplicate React instance or a dynamic', @@ -297,7 +368,8 @@ export function checkExternals(pkgDir, pkg, report) { ); } - report.note(`external imports in ${esmRelative}: ${imports.join(', ') || '(none)'}`); + report.note(`ESM files checked: ${files.join(', ')}`); + report.note(`external imports: ${[...all].sort().join(', ') || '(none)'}`); } export function checkExportsMap(pkgDir, pkg, report) { @@ -314,7 +386,15 @@ export function checkExportsMap(pkgDir, pkg, report) { if (typeof pkg[field] === 'string') referenced.add(pkg[field]); } - const missing = [...referenced].filter((rel) => rel.startsWith('.')).filter((rel) => !fs.existsSync(path.join(pkgDir, rel))); + // `module` is written "./dist/index.js" but `main` and `typings` are written + // "dist/index.umd.cjs" and "dist/index.d.ts". Filtering on a leading "." (as + // this once did) dropped both before they were ever checked, so the two + // fields the failure message names were the two it did not look at. + const isFileRef = (rel) => rel.startsWith('.') || rel.startsWith('/') || /\.(js|cjs|mjs|json|ts)$/.test(rel); + const missing = [...referenced] + .filter(isFileRef) + .map(normalizeRelative) + .filter((rel) => !fs.existsSync(path.join(pkgDir, rel))); if (missing.length > 0) { report.fail( @@ -380,7 +460,7 @@ export function checkTypes(pkgDir, report, { accept = false, baselineTypes = BAS export function checkSize(pkgDir, pkg, tarball, report, { accept = false, baselineMetrics = BASELINE_METRICS } = {}) { // `module` is written "./dist/index.js" and `main` "dist/index.umd.cjs"; // normalize so the baseline keys do not churn if package.json is tidied. - const entries = [pkg.module, pkg.main].filter((rel) => typeof rel === 'string').map((rel) => rel.replace(/^\.\//, '')); + const entries = [pkg.module, pkg.main].filter((rel) => typeof rel === 'string').map(normalizeRelative); const current = {}; for (const rel of entries) { const file = path.join(pkgDir, rel); @@ -464,6 +544,20 @@ function main() { const accept = args.includes('--accept'); const tarballArg = args.find((a) => !a.startsWith('--')); + const unknown = args.filter((a) => a.startsWith('--') && a !== '--accept'); + if (unknown.length > 0) { + console.error(`unknown option(s): ${unknown.join(', ')}`); + console.error('usage: node scripts/release-gate.mjs [tarball] | --accept'); + return 2; + } + // Recording a baseline from an arbitrary tarball would silently bless + // whatever that build contained, including a regression. + if (accept && tarballArg) { + console.error('--accept records the baseline from the current build and takes no tarball argument.'); + console.error('Build first (`npx tsc && npx vite build`), then run `npm run gate:accept`.'); + return 2; + } + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'reactfire-gate-')); try { const tarball = tarballArg ? path.resolve(tarballArg) : pack(tmp); diff --git a/test/release-gate.test.mjs b/test/release-gate.test.mjs index e4eb6a97..8f9a26e9 100644 --- a/test/release-gate.test.mjs +++ b/test/release-gate.test.mjs @@ -2,7 +2,7 @@ import { gzipSync } from 'node:zlib'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { collectImports, findCjsMarkers, listTypeFiles, diffLines, isAllowedExternal, runChecks } from '../scripts/release-gate.mjs'; +import { collectImports, findCjsMarkers, listTypeFiles, listEsmFiles, diffLines, isAllowedExternal, runChecks } from '../scripts/release-gate.mjs'; /** * The release gate exists to catch dist-level regressions that shipped as patch @@ -72,6 +72,13 @@ describe('isAllowedExternal', () => { expect(isAllowedExternal(spec)).toBe(false); } }); + + // The automatic JSX runtime imports react/jsx-runtime, which is legitimate. + it('allows react and react-dom subpaths', () => { + for (const spec of ['react/jsx-runtime', 'react/jsx-dev-runtime', 'react-dom/client']) { + expect(isAllowedExternal(spec)).toBe(true); + } + }); }); describe('findCjsMarkers', () => { @@ -143,11 +150,16 @@ describe('diffLines', () => { describe('runChecks', () => { let dir; - const build = ({ esm, types = { 'index.d.ts': 'export declare const a: string;\n' } }) => { + const build = ({ esm, types = { 'index.d.ts': 'export declare const a: string;\n' }, extraDist = {} }) => { const pkgDir = path.join(dir, 'package'); fs.mkdirSync(path.join(pkgDir, 'dist'), { recursive: true }); fs.writeFileSync(path.join(pkgDir, 'dist', 'index.js'), esm); fs.writeFileSync(path.join(pkgDir, 'dist', 'index.umd.cjs'), 'module.exports = {};'); + for (const [file, contents] of Object.entries(extraDist)) { + const dest = path.join(pkgDir, 'dist', file); + fs.mkdirSync(path.dirname(dest), { recursive: true }); + fs.writeFileSync(dest, contents); + } for (const [file, contents] of Object.entries(types)) { fs.writeFileSync(path.join(pkgDir, 'dist', file), contents); } @@ -237,4 +249,162 @@ describe('runChecks', () => { const { failures } = runChecks(pkgDir, pkg, tarball, options); expect(failures.find((f) => f.check === 'exports-map')?.message).toContain('dist/index.umd.cjs'); }); + + // Regression: `main` and `typings` are written without a "./" prefix, and the + // check used to filter on a leading ".", so the two fields its own failure + // message named were the two it never looked at. + it('fails when typings is missing, even though it has no "./" prefix', () => { + const { pkgDir, pkg, tarball, options } = build({ esm: ESM_CLEAN }); + fs.rmSync(path.join(pkgDir, 'dist', 'index.d.ts')); + fs.rmSync(path.join(options.baselineTypes, 'index.d.ts')); + const { failures } = runChecks(pkgDir, pkg, tarball, options); + expect(failures.find((f) => f.check === 'exports-map')?.message).toContain('dist/index.d.ts'); + }); + + it('fails when main is missing and no exports map duplicates it', () => { + const { pkgDir, pkg, tarball, options } = build({ esm: ESM_CLEAN }); + delete pkg.exports; + fs.rmSync(path.join(pkgDir, 'dist', 'index.umd.cjs')); + const { failures } = runChecks(pkgDir, pkg, tarball, options); + expect(failures.find((f) => f.check === 'exports-map')?.message).toContain('dist/index.umd.cjs'); + }); + + it('does not treat bare package specifiers as missing files', () => { + const { pkgDir, pkg, tarball, options } = build({ esm: ESM_CLEAN }); + pkg.exports['./polyfill'] = { import: 'react' }; + const { failures } = runChecks(pkgDir, pkg, tarball, options); + expect(failures.find((f) => f.check === 'exports-map')).toBeUndefined(); + }); + + // The #739 case: a new entry point emits a new declaration file. The + // recursion that finds it is only useful if a new file actually fails. + it('fails when a declaration file is added', () => { + const { pkgDir, pkg, tarball, options } = build({ esm: ESM_CLEAN }); + fs.mkdirSync(path.join(pkgDir, 'dist', 'nextjs'), { recursive: true }); + fs.writeFileSync(path.join(pkgDir, 'dist', 'nextjs', 'middleware.d.ts'), 'export declare const mw: string;\n'); + const { failures } = runChecks(pkgDir, pkg, tarball, options); + const types = failures.find((f) => f.check === 'types'); + expect(types?.message).toContain('1 added'); + expect(types?.detail).toContain('nextjs/middleware.d.ts'); + }); + + it('fails when a declaration file is removed', () => { + const { pkgDir, pkg, tarball, options } = build({ esm: ESM_CLEAN }); + fs.writeFileSync(path.join(options.baselineTypes, 'storage.d.ts'), 'export declare const s: string;\n'); + const { failures } = runChecks(pkgDir, pkg, tarball, options); + const types = failures.find((f) => f.check === 'types'); + expect(types?.message).toContain('1 removed'); + expect(types?.detail).toContain('storage.d.ts'); + }); + + // A CJS inline in a non-entry chunk was previously invisible: both bundle + // checks only ever opened pkg.module. + it('detects inlined CJS in a non-entry chunk', () => { + const { pkgDir, pkg, tarball, options } = build({ + esm: `${ESM_CLEAN}\nimport "./chunk-abc.js";`, + extraDist: { 'chunk-abc.js': ESM_WITH_INLINED_CJS }, + }); + const { failures } = runChecks(pkgDir, pkg, tarball, options); + const cjs = failures.find((f) => f.check === 'no-cjs-in-esm'); + expect(cjs?.message).toContain('chunk-abc.js'); + }); + + it('accepts an expected external imported from a chunk rather than the entry', () => { + const { pkgDir, pkg, tarball, options } = build({ + esm: 'import * as e from "react";\nimport "./chunk-abc.js";\nexport { e };', + extraDist: { 'chunk-abc.js': 'import { useSyncExternalStore as m } from "use-sync-external-store/shim";\nexport { m };' }, + }); + const { failures } = runChecks(pkgDir, pkg, tarball, options); + expect(failures.find((f) => f.check === 'externals')).toBeUndefined(); + }); + + it('does not scan the UMD build for CJS markers', () => { + const { pkgDir, pkg, tarball, options } = build({ esm: ESM_CLEAN }); + fs.writeFileSync(path.join(pkgDir, 'dist', 'index.umd.cjs'), 'var x = require("react");'); + const { failures } = runChecks(pkgDir, pkg, tarball, options); + expect(failures.find((f) => f.check === 'no-cjs-in-esm')).toBeUndefined(); + }); + + it('treats react/jsx-runtime as react staying external', () => { + const { pkgDir, pkg, tarball, options } = build({ + esm: 'import { jsx } from "react/jsx-runtime";\nimport { useSyncExternalStore as m } from "use-sync-external-store/shim";\nexport { jsx, m };', + }); + const { failures } = runChecks(pkgDir, pkg, tarball, options); + expect(failures.find((f) => f.check === 'externals')).toBeUndefined(); + }); + + // The size check had no coverage at all: deleting it, or setting the + // tolerance to 100, left every test green. + it('fails when an entry point grows beyond the tolerance', () => { + const { pkgDir, pkg, tarball, options } = build({ esm: ESM_CLEAN }); + const metrics = JSON.parse(fs.readFileSync(options.baselineMetrics, 'utf8')); + metrics.sizes['dist/index.js'].gzip = Math.round(metrics.sizes['dist/index.js'].gzip / 1.5); + fs.writeFileSync(options.baselineMetrics, JSON.stringify(metrics)); + const { failures } = runChecks(pkgDir, pkg, tarball, options); + expect(failures.find((f) => f.check === 'size')?.message).toContain('dist/index.js'); + }); + + it('fails when the packed tarball grows beyond the tolerance', () => { + const { pkgDir, pkg, tarball, options } = build({ esm: ESM_CLEAN }); + const metrics = JSON.parse(fs.readFileSync(options.baselineMetrics, 'utf8')); + metrics.packed = 10; + fs.writeFileSync(options.baselineMetrics, JSON.stringify(metrics)); + const { failures } = runChecks(pkgDir, pkg, tarball, options); + expect(failures.find((f) => f.check === 'size')?.message).toContain('packed tarball'); + }); + + // The band has to sit below the movement the #759 inlining actually produced + // (+3.1% on the ESM entry) and above CI's version-stamp churn (~+0.5%). + // A 10% tolerance, which is what this shipped with first, misses this. + it('fails on a delta the size of the #759 inlining', () => { + const { pkgDir, pkg, tarball, options } = build({ esm: ESM_CLEAN }); + const metrics = JSON.parse(fs.readFileSync(options.baselineMetrics, 'utf8')); + metrics.sizes['dist/index.js'].gzip = Math.round(metrics.sizes['dist/index.js'].gzip / 1.031); + fs.writeFileSync(options.baselineMetrics, JSON.stringify(metrics)); + const { failures } = runChecks(pkgDir, pkg, tarball, options); + expect(failures.find((f) => f.check === 'size')?.message).toContain('dist/index.js'); + }); + + it('tolerates size movement within the tolerance', () => { + const { pkgDir, pkg, tarball, options } = build({ esm: ESM_CLEAN }); + const metrics = JSON.parse(fs.readFileSync(options.baselineMetrics, 'utf8')); + metrics.sizes['dist/index.js'].gzip = Math.round(metrics.sizes['dist/index.js'].gzip / 1.01); + fs.writeFileSync(options.baselineMetrics, JSON.stringify(metrics)); + const { failures } = runChecks(pkgDir, pkg, tarball, options); + expect(failures.find((f) => f.check === 'size')).toBeUndefined(); + }); +}); + +describe('listEsmFiles', () => { + let dir; + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gate-esm-')); + fs.mkdirSync(path.join(dir, 'dist'), { recursive: true }); + }); + + afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + + const pkg = { module: './dist/index.js', main: 'dist/index.umd.cjs' }; + + it('lists every ESM chunk with the entry first, excluding the UMD build', () => { + for (const f of ['index.js', 'chunk-b.js', 'chunk-a.mjs', 'index.umd.cjs', 'index.js.map']) { + fs.writeFileSync(path.join(dir, 'dist', f), ''); + } + expect(listEsmFiles(dir, pkg)).toEqual(['dist/index.js', 'dist/chunk-a.mjs', 'dist/chunk-b.js']); + }); + + it('recurses into nested chunk directories', () => { + fs.mkdirSync(path.join(dir, 'dist', 'nextjs'), { recursive: true }); + fs.writeFileSync(path.join(dir, 'dist', 'index.js'), ''); + fs.writeFileSync(path.join(dir, 'dist', 'nextjs', 'middleware.js'), ''); + expect(listEsmFiles(dir, pkg)).toEqual(['dist/index.js', 'dist/nextjs/middleware.js']); + }); + + it('returns nothing when the dist directory is absent', () => { + fs.rmSync(path.join(dir, 'dist'), { recursive: true }); + expect(listEsmFiles(dir, pkg)).toEqual([]); + }); }); From 36087a783f31b0593e33c2865267ba694f696ef0 Mon Sep 17 00:00:00 2001 From: Tyler Dixon Date: Mon, 27 Jul 2026 16:56:31 -0700 Subject: [PATCH 4/5] ci: pin actions to commit SHAs in test.yaml The zizmor security scan reports 15 `unpinned-uses` findings against this file: actions referenced by floating tag (`actions/checkout@v4`) rather than pinned to a commit hash, which the blanket policy requires. A floating tag can be repointed at new code by whoever controls it, so an unpinned action is an unreviewed dependency with access to the job. Only 3 of the 15 are lines this branch added; the other 12 predate it. The scan runs only when a workflow file changes, and this is the first PR to touch .github/workflows/, so the condition was pre-existing and simply never surfaced before. Fixing only the 3 new ones would leave the check red, since it fails on any Medium/High finding. Each SHA is the current tip of that action's v4 tag, with the resolved release in a trailing comment so the version stays legible and Dependabot can still bump them. docs.yaml has the same two unpinned refs and is not touched here: the scan did not flag it, and this PR is already carrying a release gate. Worth a separate pass. --- .github/workflows/test.yaml | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 209b4842..8e43afa8 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -19,9 +19,9 @@ jobs: name: Build steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - name: Setup node - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: '24' cache: 'npm' @@ -31,7 +31,7 @@ jobs: id: pack-dir run: ./build.sh - name: 'Upload Artifact' - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: name: reactfire-${{ github.run_id }} path: | @@ -45,13 +45,13 @@ jobs: name: Verify built artifact steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - name: Setup node - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: '24' - name: 'Download Artifacts' - uses: actions/download-artifact@v4 + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 # Checks the packed tarball the publish job uploads verbatim, so this # verifies the exact bytes that ship. No deps needed; the gate is # dependency-free by design. @@ -67,9 +67,9 @@ jobs: name: Test Node.js ${{ matrix.node }} (Ubuntu) steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - name: Setup node - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: ${{ matrix.node }} check-latest: true @@ -77,7 +77,7 @@ jobs: - name: Install deps run: npm ci - name: Setup Java - uses: actions/setup-java@v4 + uses: actions/setup-java@c1e323688fd81a25caa38c78aa6df2d33d3e20d9 # v4.8.0 with: distribution: 'temurin' java-version: '21' @@ -85,12 +85,12 @@ jobs: run: npm install working-directory: ./functions - name: Firebase emulator cache - uses: actions/cache@v4 + uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 with: path: ~/.cache/firebase/emulators key: firebase_emulators - name: 'Download Artifacts' - uses: actions/download-artifact@v4 + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 - name: Expand Artifact run: | chmod +x reactfire-${{ github.run_id }}/unpack.sh @@ -106,9 +106,9 @@ jobs: name: Type check (React ${{ matrix.react }}) steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - name: Setup node - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: '24' cache: 'npm' @@ -127,12 +127,12 @@ jobs: if: ${{ github.ref == 'refs/heads/main' || github.event_name == 'release' }} steps: - name: Setup node - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: node-version: '24' registry-url: 'https://registry.npmjs.org' - name: 'Download Artifacts' - uses: actions/download-artifact@v4 + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 - name: Publish run: | cd ./reactfire-${{ github.run_id }}/ From f19b59b5b561834c626090bdc4d1a150f4b5161e Mon Sep 17 00:00:00 2001 From: Tyler Dixon Date: Mon, 27 Jul 2026 17:43:19 -0700 Subject: [PATCH 5/5] ci: least-privilege permissions and non-persisted checkout credentials Clears the two remaining zizmor medium findings on test.yaml. excessive-permissions (6): the workflow declared no `permissions:` block, so every job ran with the default token scope. Added a top-level `permissions: contents: read`. No job writes to the repo through GITHUB_TOKEN; the publish job authenticates to npm with NODE_AUTH_TOKEN, so this does not touch the publish path's credentials. artipacked (4): actions/checkout persists the token into .git/config by default, leaving it readable by every later step in the job. Nothing here re-uses git credentials after checkout, so `persist-credentials: false` is safe. The publish job has no checkout step and is unchanged. Both classes predate this branch. Verified with zizmor 1.25.2 locally: test.yaml goes from 10 medium to 0. The 6 remaining cache-poisoning findings are pre-existing, are already suppressed by the CI config (the failing run exited 13/Medium, not 14/High), and fixing them would mean restructuring how the workflow caches around the publish path, which does not belong in this PR. docs.yaml has findings of its own and is untouched here; the scan only runs on changed workflow files. Worth a separate pass. --- .github/workflows/test.yaml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 8e43afa8..13e158fe 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -13,6 +13,11 @@ on: types: - published +# Least privilege by default. No job writes to the repo through GITHUB_TOKEN; +# the publish job authenticates to npm with NODE_AUTH_TOKEN. +permissions: + contents: read + jobs: build: runs-on: ubuntu-latest @@ -20,6 +25,8 @@ jobs: steps: - name: Checkout uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + with: + persist-credentials: false - name: Setup node uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: @@ -46,6 +53,8 @@ jobs: steps: - name: Checkout uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + with: + persist-credentials: false - name: Setup node uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: @@ -68,6 +77,8 @@ jobs: steps: - name: Checkout uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + with: + persist-credentials: false - name: Setup node uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: @@ -107,6 +118,8 @@ jobs: steps: - name: Checkout uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + with: + persist-credentials: false - name: Setup node uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 with: