From 83522616a1ca1e88dd0ddaaea225f6e2826d7787 Mon Sep 17 00:00:00 2001 From: Tyler Dixon Date: Wed, 11 Nov 2026 12:39:41 -0800 Subject: [PATCH 1/3] fix(types): emit CommonJS declarations for every module The require condition's index.d.cts was a copy of index.d.ts, so its relative imports resolved to .d.ts files that TypeScript reads as ESM in this "type": "module" package. CommonJS projects using module: node16 with skipLibCheck: false got TS1479 on each one. build:types-cjs now writes a .d.cts next to every .d.ts and points relative imports at the .cjs siblings. A new Verify package step type-checks a CommonJS fixture against the packed tarball, since attw passes on the broken output. Closes #850 --- .github/workflows/test.yaml | 4 ++++ package.json | 3 ++- scripts/check-cjs-types.mjs | 33 +++++++++++++++++++++++++++++++ scripts/cjs-fixture/index.ts | 4 ++++ scripts/cjs-fixture/package.json | 4 ++++ scripts/cjs-fixture/tsconfig.json | 11 +++++++++++ scripts/emit-cjs-types.mjs | 20 +++++++++++++++++++ 7 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 scripts/check-cjs-types.mjs create mode 100644 scripts/cjs-fixture/index.ts create mode 100644 scripts/cjs-fixture/package.json create mode 100644 scripts/cjs-fixture/tsconfig.json create mode 100644 scripts/emit-cjs-types.mjs diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 6296bfce..c57b4dd7 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -80,6 +80,10 @@ jobs: # their own since #770 gave the emitted .d.ts explicit .js extensions. - name: Lint types run: npm run lint:types -- reactfire-${{ github.run_id }}/reactfire.tgz + # attw resolves the `require` types but does not type-check them, so it + # passed while every relative import in index.d.cts failed with TS1479 (#850). + - name: Type-check from CommonJS + run: npm run lint:types-cjs -- reactfire-${{ github.run_id }}/reactfire.tgz - name: Bundle size run: npm run size test: diff --git a/package.json b/package.json index 098ca497..1d33e541 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "scripts": { "start": "firebase emulators:exec --project=rxfire-525a3 \"vitest --ui\"", "build": "tsc && vite build && npm run build:types-cjs && npm run docs", - "build:types-cjs": "node -e \"require('fs').copyFileSync('dist/index.d.ts','dist/index.d.cts')\"", + "build:types-cjs": "node scripts/emit-cjs-types.mjs", "test": "firebase emulators:exec --project=rxfire-525a3 \"vitest\"", "test:firestore": "firebase emulators:exec --only firestore --ui --project=rxfire-525a3 \"vitest firestore\"", "test:database": "firebase emulators:exec --only database --project=rxfire-525a3 \"vitest database\"", @@ -41,6 +41,7 @@ "analyze": "size-limit --why", "lint:package": "publint --strict", "lint:types": "attw", + "lint:types-cjs": "node scripts/check-cjs-types.mjs", "docs": "typedoc --options typedoc.json && markdown-toc -i docs/use.md", "docs:fork": "typedoc --options typedoc.json --gitRemote upstream && markdown-toc -i docs/use.md" }, diff --git a/scripts/check-cjs-types.mjs b/scripts/check-cjs-types.mjs new file mode 100644 index 00000000..99be2036 --- /dev/null +++ b/scripts/check-cjs-types.mjs @@ -0,0 +1,33 @@ +// Type-checks a CommonJS consumer (`module: node16`, `skipLibCheck: false`) against a packed reactfire tarball. +// attw passes even when this fails (#850). Only errors in reactfire's files or the fixture count: with +// `skipLibCheck: false` the check also reports errors inside firebase's and @types' own declarations. +import { execFileSync } from 'node:child_process'; +import { mkdirSync, rmSync } from 'node:fs'; +import { createRequire } from 'node:module'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const tarball = resolve(process.argv[2] ?? 'reactfire.tgz'); +const fixture = join(dirname(fileURLToPath(import.meta.url)), 'cjs-fixture'); +const installed = join(fixture, 'node_modules', 'reactfire'); + +// Unpack rather than `npm install`, so reactfire's dependencies and peers resolve from the repo's node_modules. +rmSync(join(fixture, 'node_modules'), { recursive: true, force: true }); +mkdirSync(installed, { recursive: true }); +execFileSync('tar', ['-xzf', tarball, '-C', installed, '--strip-components=1']); + +const tsc = createRequire(import.meta.url).resolve('typescript/bin/tsc'); +let output = ''; +try { + execFileSync(process.execPath, [tsc, '-p', '.'], { cwd: fixture, encoding: 'utf8' }); +} catch (error) { + output = error.stdout ?? ''; + // tsc exits non-zero on dependency errors too; anything without a diagnostic is a failure to run. + if (!output.includes('error TS')) throw error; +} +const ours = output.split('\n').filter((line) => /^(node_modules\/reactfire\/|index\.ts)/.test(line)); +if (ours.length > 0) { + console.error(ours.join('\n')); + process.exit(1); +} +console.log('CommonJS type check passed'); diff --git a/scripts/cjs-fixture/index.ts b/scripts/cjs-fixture/index.ts new file mode 100644 index 00000000..7f70cc1c --- /dev/null +++ b/scripts/cjs-fixture/index.ts @@ -0,0 +1,4 @@ +import { useUser } from 'reactfire'; + +// `true` only when `status` is the real union: an `any` from a failed resolution makes this `never`. +export const typed: 'not-a-status' extends ReturnType['status'] ? never : true = true; diff --git a/scripts/cjs-fixture/package.json b/scripts/cjs-fixture/package.json new file mode 100644 index 00000000..588fdeff --- /dev/null +++ b/scripts/cjs-fixture/package.json @@ -0,0 +1,4 @@ +{ + "name": "reactfire-cjs-fixture", + "private": true +} diff --git a/scripts/cjs-fixture/tsconfig.json b/scripts/cjs-fixture/tsconfig.json new file mode 100644 index 00000000..9976ef00 --- /dev/null +++ b/scripts/cjs-fixture/tsconfig.json @@ -0,0 +1,11 @@ +{ + "compilerOptions": { + "module": "node16", + "moduleResolution": "node16", + "skipLibCheck": false, + "strict": true, + "noEmit": true, + "jsx": "react-jsx" + }, + "files": ["index.ts"] +} diff --git a/scripts/emit-cjs-types.mjs b/scripts/emit-cjs-types.mjs new file mode 100644 index 00000000..cbfe1cc4 --- /dev/null +++ b/scripts/emit-cjs-types.mjs @@ -0,0 +1,20 @@ +// Writes a CommonJS declaration file (`.d.cts`) next to every `dist/*.d.ts`, for the `require` export condition. +// A copy of `index.d.ts` is not enough: its relative imports resolve to `.d.ts` files, which TypeScript reads +// as ESM in this `"type": "module"` package, so CommonJS consumers get TS1479 on each import (#850). +// Rewriting `./x.js` to `./x.cjs` points each import at the matching `.d.cts` instead. +import { readdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; + +const dir = 'dist'; +const relativeSpecifier = /(\bfrom\s*|\bimport\s*\(\s*)(['"])(\.{1,2}\/[^'"]*)\2/g; + +for (const file of readdirSync(dir).filter((f) => f.endsWith('.d.ts'))) { + const source = readFileSync(join(dir, file), 'utf8'); + const cjs = source.replace(relativeSpecifier, (_, keyword, quote, specifier) => { + if (!specifier.endsWith('.js')) { + throw new Error(`${file}: relative import without a .js extension: ${specifier}`); + } + return `${keyword}${quote}${specifier.slice(0, -'.js'.length)}.cjs${quote}`; + }); + writeFileSync(join(dir, file.replace(/\.d\.ts$/, '.d.cts')), cjs); +} From 7bde1e0ddc3fe6a4b49865b584a7b659590be06a Mon Sep 17 00:00:00 2001 From: Tyler Dixon Date: Wed, 11 Nov 2026 13:15:25 -0800 Subject: [PATCH 2/3] chore: trim check-cjs-types header comment --- scripts/check-cjs-types.mjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/check-cjs-types.mjs b/scripts/check-cjs-types.mjs index 99be2036..ae989849 100644 --- a/scripts/check-cjs-types.mjs +++ b/scripts/check-cjs-types.mjs @@ -1,6 +1,6 @@ // Type-checks a CommonJS consumer (`module: node16`, `skipLibCheck: false`) against a packed reactfire tarball. -// attw passes even when this fails (#850). Only errors in reactfire's files or the fixture count: with -// `skipLibCheck: false` the check also reports errors inside firebase's and @types' own declarations. +// Only errors in reactfire's files or the fixture count: `skipLibCheck: false` also reports errors inside +// dependencies' own declarations, which reactfire cannot fix. import { execFileSync } from 'node:child_process'; import { mkdirSync, rmSync } from 'node:fs'; import { createRequire } from 'node:module'; From c12ed6d5a3767e2592cef8125bfb8caf47fa66ad Mon Sep 17 00:00:00 2001 From: Tyler Dixon Date: Wed, 11 Nov 2026 14:02:34 -0800 Subject: [PATCH 3/3] chore: drop the CommonJS type-check step --- .github/workflows/test.yaml | 4 ---- package.json | 1 - scripts/check-cjs-types.mjs | 33 ------------------------------- scripts/cjs-fixture/index.ts | 4 ---- scripts/cjs-fixture/package.json | 4 ---- scripts/cjs-fixture/tsconfig.json | 11 ----------- 6 files changed, 57 deletions(-) delete mode 100644 scripts/check-cjs-types.mjs delete mode 100644 scripts/cjs-fixture/index.ts delete mode 100644 scripts/cjs-fixture/package.json delete mode 100644 scripts/cjs-fixture/tsconfig.json diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index c57b4dd7..6296bfce 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -80,10 +80,6 @@ jobs: # their own since #770 gave the emitted .d.ts explicit .js extensions. - name: Lint types run: npm run lint:types -- reactfire-${{ github.run_id }}/reactfire.tgz - # attw resolves the `require` types but does not type-check them, so it - # passed while every relative import in index.d.cts failed with TS1479 (#850). - - name: Type-check from CommonJS - run: npm run lint:types-cjs -- reactfire-${{ github.run_id }}/reactfire.tgz - name: Bundle size run: npm run size test: diff --git a/package.json b/package.json index 1d33e541..55f8e3e5 100644 --- a/package.json +++ b/package.json @@ -41,7 +41,6 @@ "analyze": "size-limit --why", "lint:package": "publint --strict", "lint:types": "attw", - "lint:types-cjs": "node scripts/check-cjs-types.mjs", "docs": "typedoc --options typedoc.json && markdown-toc -i docs/use.md", "docs:fork": "typedoc --options typedoc.json --gitRemote upstream && markdown-toc -i docs/use.md" }, diff --git a/scripts/check-cjs-types.mjs b/scripts/check-cjs-types.mjs deleted file mode 100644 index ae989849..00000000 --- a/scripts/check-cjs-types.mjs +++ /dev/null @@ -1,33 +0,0 @@ -// Type-checks a CommonJS consumer (`module: node16`, `skipLibCheck: false`) against a packed reactfire tarball. -// Only errors in reactfire's files or the fixture count: `skipLibCheck: false` also reports errors inside -// dependencies' own declarations, which reactfire cannot fix. -import { execFileSync } from 'node:child_process'; -import { mkdirSync, rmSync } from 'node:fs'; -import { createRequire } from 'node:module'; -import { dirname, join, resolve } from 'node:path'; -import { fileURLToPath } from 'node:url'; - -const tarball = resolve(process.argv[2] ?? 'reactfire.tgz'); -const fixture = join(dirname(fileURLToPath(import.meta.url)), 'cjs-fixture'); -const installed = join(fixture, 'node_modules', 'reactfire'); - -// Unpack rather than `npm install`, so reactfire's dependencies and peers resolve from the repo's node_modules. -rmSync(join(fixture, 'node_modules'), { recursive: true, force: true }); -mkdirSync(installed, { recursive: true }); -execFileSync('tar', ['-xzf', tarball, '-C', installed, '--strip-components=1']); - -const tsc = createRequire(import.meta.url).resolve('typescript/bin/tsc'); -let output = ''; -try { - execFileSync(process.execPath, [tsc, '-p', '.'], { cwd: fixture, encoding: 'utf8' }); -} catch (error) { - output = error.stdout ?? ''; - // tsc exits non-zero on dependency errors too; anything without a diagnostic is a failure to run. - if (!output.includes('error TS')) throw error; -} -const ours = output.split('\n').filter((line) => /^(node_modules\/reactfire\/|index\.ts)/.test(line)); -if (ours.length > 0) { - console.error(ours.join('\n')); - process.exit(1); -} -console.log('CommonJS type check passed'); diff --git a/scripts/cjs-fixture/index.ts b/scripts/cjs-fixture/index.ts deleted file mode 100644 index 7f70cc1c..00000000 --- a/scripts/cjs-fixture/index.ts +++ /dev/null @@ -1,4 +0,0 @@ -import { useUser } from 'reactfire'; - -// `true` only when `status` is the real union: an `any` from a failed resolution makes this `never`. -export const typed: 'not-a-status' extends ReturnType['status'] ? never : true = true; diff --git a/scripts/cjs-fixture/package.json b/scripts/cjs-fixture/package.json deleted file mode 100644 index 588fdeff..00000000 --- a/scripts/cjs-fixture/package.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "name": "reactfire-cjs-fixture", - "private": true -} diff --git a/scripts/cjs-fixture/tsconfig.json b/scripts/cjs-fixture/tsconfig.json deleted file mode 100644 index 9976ef00..00000000 --- a/scripts/cjs-fixture/tsconfig.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "compilerOptions": { - "module": "node16", - "moduleResolution": "node16", - "skipLibCheck": false, - "strict": true, - "noEmit": true, - "jsx": "react-jsx" - }, - "files": ["index.ts"] -}