FirebaseExtended / FirebaseExtended/reactfire

Release process: add a built-artifact (dist/bundle) check to catch runtime regressions before publish

Ouverte
#765 2 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
Langage dominant
TypeScript
Étoiles
3.6k
Forks
403
Merge moyen
14 h 53 min
PR mergées (30 j)
5

Description

### Motivation

#759 (the App Router / Vite client crash in 4.2.4 and 4.2.5) shipped because the build-tooling migration silently changed the **built ESM output**: `use-sync-external-store/shim` (CJS) started getting bundled into the ESM `dist`, producing a dynamic `require()` that throws in any browser bundle (`Calling \`require\` for "react" in an environment that doesn't expose the require function`). The source was unchanged; only the emitted artifact regressed, and nothing in the release flow compared the artifact before publish.

This is the **second dist-level regression to ship as a patch**: #749 already proposes a published `.d.ts` diff to catch the *type* side (the 4.2.4 `ObservableStatus` break). This issue covers the *runtime/bundle* side. Together they form a built-artifact diff gate.

### Proposed pre-publish checks

A concrete checklist for a pre-publish gate (each item derived from auditing the 4.2.6 release by hand). Items marked would have caught #759.

1. **No dynamic `require(` / CJS-interop shims in the ESM dist.** Grep `dist/index.js` for `\brequire\b`, `__require`, `createRequire`, `__commonJS`. *(Would have caught #759: `require` count went 0 at 4.2.3 to 3 at 4.2.4+.)*
2. **Externals are not inlined.** Only `rxfire` / `rxjs` / `tslib` should be bundled; `react`, `firebase/*`, `@firebase/*`, and `use-sync-external-store/shim` must stay external `import` specifiers. Catches accidental bundling that causes duplicate-instance bugs.
3. **Both entry points load.** `node --input-type=module -e "import('reactfire')"` and `node -e "require('reactfire')"` (in a fixture with `react` + `firebase` installed) must resolve without throwing. Catches missing/renamed files and broken imports.
4. **exports map integrity.** Every path referenced by `exports` / `main` / `module` exists in the packed tarball.
5. **Bundle-size delta vs previous `latest`.** `npm pack` the current `latest`, compare `dist` size; flag large jumps (a proxy for accidental inlining).
6. **Published `.d.ts` diff vs previous version.** Type-side counterpart, tracked in #749; catches the 4.2.4 `ObservableStatus` break class.
7. **Runtime smoke render in CI (strongest).** A minimal Next App Router (turbopack) and Vite app that renders a data hook against the packed build; fails on the #759 crash. This is the check that catches runtime regressions the static greps miss.

Items 1 to 5 are cheap and scriptable against `npm pack` output; 7 is the higher-value integration check.

### Related

- #749 (published `.d.ts` diff, the type-side counterpart)
- #759 / #760 (the regression this would have caught, and its fix)

Guide de contribution

Ouvrir le guide de contribution

Piste de recherche

Start with the proposed checks against npm pack output, especially dist/index.js, the package exports/main/module paths, and the Node ESM and CommonJS import commands. Define a pre-publish gate covering dynamic require and external checks, entry-point loading, tarball path integrity, and bundle-size comparison; the runtime smoke apps and the .d.ts diff are related follow-ups. Done means the release process detects the regressions described in #759 before publishing.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
firebase, node.js, react, typescript
Domaine
build-system, ci-cd, release, testing
Type d'issue
Fonctionnalité
Difficulté
4/5
Temps estimé
3-5 jours
Activité
Calme
Clarté
Plutôt claire
Accessibilité débutants
48/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.