Conversation
WalkthroughReplaces per-package Rollup builds with tsdown configs, updates package type exports to separate ESM/CJS declaration paths, removes Rollup deps/configs, adds publint and lint:publint scripts and CI publint step, adds tsdown config files and ignores them from published packages, and tweaks a Firestore import split. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant CI as CI
participant Repo as Repo scripts
participant PKG as Package build (tsdown)
participant Publint as publint
Dev->>CI: Open PR / push
CI->>Repo: run lint job
Repo->>CI: run prettier step
CI->>Repo: run publint step (pnpm lint:publint)
Repo->>PKG: pnpm build (tsdown) invoked by lint:publint
PKG-->>Repo: emit CJS/ESM + `.d.cts`/`.d.mts`
Repo->>Publint: run publint on outputs
Publint-->>CI: return results (pass/fail)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/react-kitchen-sink/tsdown.config.mjs (1)
5-7: Confirm intent:platform: 'browser'+format: ['esm','cjs']makes only ESM browser-targeted; CJS will target Node.In tsdown, CJS format is always built for Node regardless of
platform. Keep both if you need a Node CJS artifact; otherwise consider emitting ESM only for this package.Optional tweak:
- format: ['esm', 'cjs'], + format: ['esm'],Reference: CJS platform is forced to
node; formats list and usage. (tsdown.dev)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.github/workflows/pull-request.yml(1 hunks)package.json(2 hunks)packages/react-firestore/.npmignore(1 hunks)packages/react-firestore/package.json(2 hunks)packages/react-kitchen-sink/.npmignore(1 hunks)packages/react-kitchen-sink/package.json(1 hunks)packages/react-kitchen-sink/tsdown.config.mjs(1 hunks)packages/rxjs-firebase/.npmignore(1 hunks)packages/rxjs-firebase/package.json(2 hunks)packages/zustand-firestore/.npmignore(1 hunks)packages/zustand-firestore/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/react-kitchen-sink/.npmignore
- packages/rxjs-firebase/.npmignore
- packages/react-firestore/.npmignore
- package.json
- .github/workflows/pull-request.yml
- packages/zustand-firestore/package.json
- packages/zustand-firestore/.npmignore
- packages/react-kitchen-sink/package.json
- packages/rxjs-firebase/package.json
- packages/react-firestore/package.json
🧰 Additional context used
🪛 ESLint
packages/react-kitchen-sink/tsdown.config.mjs
[error] 1-1: Unable to resolve path to module 'tsdown'.
(import-x/no-unresolved)
🔇 Additional comments (1)
packages/react-kitchen-sink/tsdown.config.mjs (1)
4-9: Config options look valid;fixedExtension: trueis appropriate for .mjs/.cjs outputs.
fixedExtensionis a supported option and forces.mjs/.cjsextensions;target: 'es2020',outDir, andformatare standard config fields. LGTM.Refs: option definitions for
fixedExtension,outDir,target, andformat. (tsdown.dev)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/react-kitchen-sink/src/schema/firestoreDataConverter.ts (1)
15-16: Don’t treat all objects as plain objects; this breaks DocumentReference and other Firestore classes.
value instanceof Objectwill match Firestore classes (e.g.,DocumentReference,Bytes) and strip them viaObject.entries(...), returning an empty or mangled object. Use a plain-object guard.Apply:
- case value instanceof Object: - return Object.fromEntries(Object.entries(value).map(([key, v]) => [key, handleFirestoreDataTypes(v)])) as T + case value !== null && typeof value === 'object' && Object.getPrototypeOf(value) === Object.prototype: + return Object.fromEntries( + Object.entries(value).map(([key, v]) => [key, handleFirestoreDataTypes(v)]) + ) as T
🧹 Nitpick comments (1)
packages/react-kitchen-sink/src/schema/firestoreDataConverter.ts (1)
11-14: Type-safety of array mapping could be improved (optional).The casts to
T[keyof T]andas Tare broad. If you keep this utility generic, prefer widening tounknown[]internally and narrowing at the call site, or introduce a constrained generic for arrays.Example:
- case Array.isArray(value): - return value.map((v) => handleFirestoreDataTypes(v) as T[keyof T]) as T + case Array.isArray(value): + return (value as unknown[]).map((v) => handleFirestoreDataTypes(v)) as unknown as T
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/react-kitchen-sink/src/schema/firestoreDataConverter.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (GEMINI.md)
Ensure all new code conforms to the project's ESLint and Prettier rules
Files:
packages/react-kitchen-sink/src/schema/firestoreDataConverter.ts
🧠 Learnings (3)
📚 Learning: 2025-08-16T16:32:15.795Z
Learnt from: CR
PR: valian-ca/react-firebase#0
File: packages/react-firestore/GEMINI.md:0-0
Timestamp: 2025-08-16T16:32:15.795Z
Learning: Applies to packages/react-firestore/**/*.{ts,tsx} : Write all new code in TypeScript
Applied to files:
packages/react-kitchen-sink/src/schema/firestoreDataConverter.ts
📚 Learning: 2025-08-16T16:32:15.795Z
Learnt from: CR
PR: valian-ca/react-firebase#0
File: packages/react-firestore/GEMINI.md:0-0
Timestamp: 2025-08-16T16:32:15.795Z
Learning: Applies to packages/react-firestore/**/*.{ts,tsx} : Ensure all code conforms to the project’s ESLint rules
Applied to files:
packages/react-kitchen-sink/src/schema/firestoreDataConverter.ts
📚 Learning: 2025-08-16T16:32:27.169Z
Learnt from: CR
PR: valian-ca/react-firebase#0
File: packages/rxjs-firebase/GEMINI.md:0-0
Timestamp: 2025-08-16T16:32:27.169Z
Learning: Applies to packages/rxjs-firebase/**/*.ts : Implement library code in TypeScript
Applied to files:
packages/react-kitchen-sink/src/schema/firestoreDataConverter.ts
🪛 ESLint
packages/react-kitchen-sink/src/schema/firestoreDataConverter.ts
[error] 1-1: Filename is not in kebab case. Rename it to firestore-data-converter.ts.
(unicorn/filename-case)
[error] 1-1: Unable to resolve path to module '@firebase/firestore'.
(import-x/no-unresolved)
🔇 Additional comments (2)
packages/react-kitchen-sink/src/schema/firestoreDataConverter.ts (2)
1-1: Rename file to kebab-case to satisfy ESLint (unicorn/filename-case).Current:
firestoreDataConverter.ts. Expected:firestore-data-converter.ts.If you want, I can generate a small script to rename and update imports across the repo.
22-24: If dual App/Db model types are required, avoid internal typings.Should you need both App and Db model typing, model it without
@firebase/*by acceptingQueryDocumentSnapshot<App>and letting the converter be generic overApp, keepingDbtypes internal to yourwithConverterusage. This prevents tight coupling to Firebase internals.Do you actually consume the second generic elsewhere? If yes, I can propose a public-API-compliant typing for that flow.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
packages/react-kitchen-sink/src/schema/firestoreDataConverter.ts (3)
23-25: Align with publicQueryDocumentSnapshot<T>and return the converted type.Public API uses a single generic. Returning
FirestoreToJs<T>makes the conversion explicit and improves DX. Also resolves publint complaints.-export const firestoreDataConverter = <AppModelType = DocumentData, DbModelType extends DocumentData = DocumentData>( - snapshot: QueryDocumentSnapshot<AppModelType, DbModelType>, -) => handleFirestoreDataTypes(snapshot.data()) +export const firestoreDataConverter = <T extends DocumentData = DocumentData>( + snapshot: QueryDocumentSnapshot<T>, +) => handleFirestoreDataTypes(snapshot.data()) as FirestoreToJs<T>
1-2: Use public Firebase entrypoint; drop @firebase internals and unify imports.Importing from
@firebase/firestorereaches into private APIs and triggersimport-x/no-unresolved. Use the publicfirebase/firestorefor both types and values. This also aligns with publint and tsdown expectations.Apply:
-import { type DocumentData, type QueryDocumentSnapshot } from '@firebase/firestore' -import { GeoPoint, Timestamp } from 'firebase/firestore' +import type { DocumentData, QueryDocumentSnapshot } from 'firebase/firestore' +import { GeoPoint, Timestamp } from 'firebase/firestore'
2-2: Refactor all@firebase/firestoreimports and update ESLint config
- Replace every internal import from
@firebase/firestorewith the publicfirebase/firestoreentrypoint across the repo.- In your ESLint settings, add a TypeScript resolver under
settings.import/resolverand adjust theimport-x/extensionsrule to ignore package imports so thatfirebase/firestoreisn’t flagged for missing extensions.
🧹 Nitpick comments (1)
packages/react-kitchen-sink/src/schema/firestoreDataConverter.ts (1)
4-21: Type soundness: function returnsTbut convertsTimestamptoDate.The current signature lies to the type system and leaks
any-like casts. Model the deep conversion with a mapped conditional type and adjust the return type.Add above the function:
type FirestoreToJs<T> = T extends Timestamp ? Date : T extends GeoPoint ? GeoPoint : T extends Array<infer U> ? Array<FirestoreToJs<U>> : T extends object ? { [K in keyof T]: FirestoreToJs<T[K]> } : TThen change the function as follows:
-const handleFirestoreDataTypes = <T>(value: T): T => { +const handleFirestoreDataTypes = <T>(value: T): FirestoreToJs<T> => { switch (true) { case value instanceof Date: - return value + return value as unknown as FirestoreToJs<T> case value instanceof GeoPoint: - return value + return value as unknown as FirestoreToJs<T> case value instanceof Timestamp: - return value.toDate() as T + return value.toDate() as unknown as FirestoreToJs<T> case typeof value === 'string': - return value + return value as unknown as FirestoreToJs<T> case Array.isArray(value): - return value.map((v) => handleFirestoreDataTypes(v) as T[keyof T]) as T - case value instanceof Object: - return Object.fromEntries(Object.entries(value).map(([key, v]) => [key, handleFirestoreDataTypes(v)])) as T + return (value as unknown as Array<unknown>) + .map((v) => handleFirestoreDataTypes(v)) as unknown as FirestoreToJs<T> + case value !== null && typeof value === 'object': + return Object.fromEntries( + Object.entries(value as Record<string, unknown>) + .map(([key, v]) => [key, handleFirestoreDataTypes(v)]), + ) as unknown as FirestoreToJs<T> default: - return value + return value as unknown as FirestoreToJs<T> } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/react-kitchen-sink/src/schema/firestoreDataConverter.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (GEMINI.md)
Ensure all new code conforms to the project's ESLint and Prettier rules
Files:
packages/react-kitchen-sink/src/schema/firestoreDataConverter.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: valian-ca/react-firebase#0
File: packages/react-firestore/GEMINI.md:0-0
Timestamp: 2025-08-16T16:32:15.795Z
Learning: Applies to packages/react-firestore/**/*.{ts,tsx} : Ensure all code conforms to the project’s ESLint rules
📚 Learning: 2025-08-16T16:32:15.795Z
Learnt from: CR
PR: valian-ca/react-firebase#0
File: packages/react-firestore/GEMINI.md:0-0
Timestamp: 2025-08-16T16:32:15.795Z
Learning: Applies to packages/react-firestore/**/*.{ts,tsx} : Write all new code in TypeScript
Applied to files:
packages/react-kitchen-sink/src/schema/firestoreDataConverter.ts
📚 Learning: 2025-08-16T16:32:27.169Z
Learnt from: CR
PR: valian-ca/react-firebase#0
File: packages/rxjs-firebase/GEMINI.md:0-0
Timestamp: 2025-08-16T16:32:27.169Z
Learning: Applies to packages/rxjs-firebase/**/*.ts : Implement library code in TypeScript
Applied to files:
packages/react-kitchen-sink/src/schema/firestoreDataConverter.ts
📚 Learning: 2025-08-16T16:32:15.795Z
Learnt from: CR
PR: valian-ca/react-firebase#0
File: packages/react-firestore/GEMINI.md:0-0
Timestamp: 2025-08-16T16:32:15.795Z
Learning: Applies to packages/react-firestore/**/*.{ts,tsx} : Ensure all code conforms to the project’s ESLint rules
Applied to files:
packages/react-kitchen-sink/src/schema/firestoreDataConverter.ts
🪛 ESLint
packages/react-kitchen-sink/src/schema/firestoreDataConverter.ts
[error] 1-1: Filename is not in kebab case. Rename it to firestore-data-converter.ts.
(unicorn/filename-case)
[error] 1-1: Unable to resolve path to module '@firebase/firestore'.
(import-x/no-unresolved)
[error] 2-2: Unable to resolve path to module 'firebase/firestore'.
(import-x/no-unresolved)
[error] 2-2: Missing file extension for "firebase/firestore"
(import-x/extensions)
1 participant
Summary by CodeRabbit
Chores
Packaging