FirebaseExtended / FirebaseExtended/reactfire

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

Đang mở
#765 2 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
TypeScript
Star
3.6k
Fork
403
Merge trung bình
14 giờ 53 phút
Pull request đã merge (30 ngày)
5

Mô tả

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

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Hướng nghiên cứu

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.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
firebase, node.js, react, typescript
Lĩnh vực
build-system, ci-cd, release, testing
Loại issue
Tính năng
Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức độ hoạt động
Ít trao đổi
Độ rõ ràng
Khá rõ ràng
Mức phù hợp với người mới
48/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.