FirebaseExtended / FirebaseExtended/reactfire
Release process: add a built-artifact (dist/bundle) check to catch runtime regressions before publish
- Dominant language
- TypeScript
- Stars
- 3.6k
- Forks
- 403
- Avg merge
- 14h 53m
- Merged PRs (30d)
- 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)
Contributor guide
Research direction
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.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- firebase, node.js, react, typescript
- Domain
- build-system, ci-cd, release, testing
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100