FirebaseExtended / FirebaseExtended/reactfire

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

Abierto
#765 2 comentarios 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
TypeScript
Estrellas
3.6k
Forks
403
Merge medio
14 h 53 min
PR fusionados (30 d)
5

Descripción

### 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)

Guía de contribución

Abrir la guía de contribución

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
firebase, node.js, react, typescript
Área
build-system, ci-cd, release, testing
Tipo de issue
Nueva funcionalidad
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Tranquilo
Claridad
Bastante claro
Aptitud para principiantes
48/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.