Skip to content

fix(build): preserve ESM modules so bundlers can tree-shake unused Firebase SDKs - #838

Open
jhuleatt wants to merge 9 commits into
mainfrom
fix/esm-tree-shaking
Open

jhuleatt wants to merge 9 commits into
mainfrom
fix/esm-tree-shaking

Conversation

@jhuleatt

@jhuleatt jhuleatt commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

dist/index.js was a single file that imported every Firebase product, so an app using only useUser still bundled Firestore, RTDB, Storage, Functions, Remote Config and Performance. This PR:

  1. Emits one ES module per source file (preserveModules: true, preserveModulesRoot: 'src', entryFileNames: '[name].js') alongside dist/index.umd.cjs. With the "sideEffects": false already on main (ci: add built-artifact release gate (#765) #766), bundlers can now skip unused product modules and their firebase/* imports.
  2. Moves ReactFireError, checkIdField, checkOptions and checkinitialData from src/index.ts to src/utils.ts, still exported from index.ts. auth, database and firestore imported them back from the barrel, and Next 16's default bundler (Turbopack) then kept every product.
  3. Sets tsconfig target to es2021 (from es2015), so the split output stays at ES2021, the level main already ships.

rxjs and rxfire stay bundled, as on main; under preserveModules they are emitted to dist/vendor/. Not dist/node_modules/: Node's package scope lookup stops at that folder name, so the files would load as CommonJS wherever Node does not detect module syntax (before 20.19 / 22.7, and Yarn PnP). The rxjs range is unchanged (^6.6.3 || ^7.0.1).

Bundle size

Consumer apps installed from a packed tarball, firebase 13.1.0, React 19.3.1. gzip -9 of all emitted JS, kB.

Case main This PR
Vite, auth only 271.0, all 6 products 104.8, auth only
Vite, Firestore only 280.4, all 6 215.7, Firestore only
Vite, all hooks (import * as RF) 288.7 289.3
Next 16 Turbopack, auth only 384.2, all 6 213.4, auth only
Next 16 --webpack, auth only 500.7, all 6 325.9, auth only

webpack 5, esbuild and Rollup also drop unused products. Without the change in (2), Turbopack keeps all six products (384.8 kB).

Compatibility

  • Same 74 named exports from import and require; main, exports and typings unchanged.
  • attw green on node10, node16 CJS/ESM and bundler.
  • rxjs 6 app passing its own Observable to useObservable: tsc passes, no nested rxjs.
  • Output stays ES2021 (es-check).
  • Test suite on React 18 and 19, the Jest/CJS load, and the CONTRIBUTING release smoke test (Vite, Next 16, Node ESM/CJS) match main.

Risks

  • dist/vendor/ ships in the package (rxjs, rxfire, tslib; 87 files). Node ESM imports it on 20.12.2 and 22.15 (the dist/node_modules/ layout failed on 20.12.2). Yarn PnP: Armando's run imports it; not run by me.
  • Import-time code of unused products no longer runs when a bundler honors sideEffects, e.g. firebase/database registering its component in an auth-only app. An app that calls getDatabase imports firebase/database itself, so this is expected to be harmless (inferred, not tested beyond the smoke test).
  • The Turbopack result is measured on Next 16.4.5; later versions are not tested.

Closes #489
Refs #488 (lazy-loading behind React.lazy was not measured)
Refs #480

@jhuleatt
jhuleatt requested a review from tyler-reitz October 27, 2026 18:57

@tyler-reitz tyler-reitz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for one thing, which only shows up together with #839.

Tree-shaking works as described: on Vite 8.3.5 with firebase 12, an auth-only app goes from 240 KB to 82 KB gzipped with no Firestore, Database, Storage or Functions code. The 74 ESM and CJS exports match main, and #835's import-time registration survives "sideEffects": false.

Blocking: rxjs 6 apps. Externalizing rxjs and rxfire splits them: in an rxjs 6.6.7 app, reactfire's code loads a nested rxjs 7.8.2 while rxfire loads the app's 6.6.7, and teardown doesn't cross that boundary. On its own that's invisible, since shareReplay never tears down, but with #839 merged too, useUser and useFirestoreDocData each leak one listener per remount (1,2,3,4,5 after five cycles; 0 on rxjs 7). Making rxjs a ^7.1.0 peer dependency turns this into an install error. Whichever of this and #839 lands first should carry it, plus a !, and the body's "runtime behavior and npm install are unchanged" needs updating for these apps.

Non-blocking: target: es2021 also avoids the tslib chunk and keeps dist at ES2020; es2024 raises it to ES2022 (native class fields), one level above main.

Externalizing rxjs and rxfire adds no size win (the split ESM output alone
gives it) and installs a second rxjs for rxjs 6 apps. Restore main's rxjs
range and target es2021 so the output stays at ES2021.
auth, database and firestore imported ReactFireError and checkIdField from
index.ts, which re-exports every product, so Turbopack kept all of them.
Move the four values to src/utils.ts and re-export them from index.ts.
@tyler-reitz

Copy link
Copy Markdown
Contributor

I'm withdrawing my rxjs peer-dependency and ! ask. I've pushed a version that keeps the size win and can ship as a patch: a merge of main, 23079e0 (rxjs and rxfire stay bundled, main's rxjs range, target: es2021), and 7bb549a (moves ReactFireError and the three option helpers from index.ts to src/utils.ts, still exported from index.ts).

Externalizing rxjs saved nothing: bundling it is 0.1 to 0.5 kB smaller in every bundler, and it removes the nested rxjs 7 and the TS2345 error in rxjs 6 apps. Without 7bb549a, Next 16's default bundler (Turbopack) kept every Firebase product, because auth, database and firestore imported those values back from index.ts.

gzip kB, firebase 13 main before now
Vite, auth only 271.0 104.9 104.8
Vite, Firestore only 280.4 215.8 215.7
Next 16 Turbopack, auth only 384.2 385.1 213.4

webpack, esbuild and Rollup drop unused products as well. Exports, attw, the release smoke test and the suite on React 18 and 19 match main. "sideEffects": false was already on main (#766), so the merge keeps one key. I've updated the body to match. The rxjs 6 leak from my review no longer applies either: rxjs stays bundled, and #839's from() wrap fixes it on its own (0 leaked listeners over 5 remounts).

@armando-navarro could you review the rework? Jeff, I'll loop you in after.

@tyler-reitz
tyler-reitz dismissed their stale review November 6, 2026 19:29

Superseded by the rework in 2d644f7, see comment.

…e_modules

Node resolves a file's module format from the nearest package.json and
stops that search at a folder named node_modules. The vendored rxjs,
rxfire and tslib files therefore had no "type": "module" scope and were
read as CommonJS on Node without default module syntax detection (before
20.19 and 22.7) and under Yarn Plug'n'Play, so importing reactfire there
failed with "is a CommonJS module". Renaming the output folder keeps the
files inside this package's scope. Bundle sizes, attw and publint are
unchanged.

@armando-navarro armando-navarro left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the rework, Tyler, and for the original build change, Jeff. The size win is real: I rebuilt both tarballs, measured auth-only and Firestore-only consumer apps under Vite 7 and an auth-only app under Next 16 Turbopack, and got your table within a few kB, with only the matching @firebase packages left in each bundle.

One thing does regress against main, and it is the only reason I am not approving yet.

1. Blocking: the package no longer imports through Node's own ESM loader on Node before 20.19 / 22.7, nor under Yarn Plug'n'Play

Cause:

  • The emitted modules import rxjs, rxfire and tslib from dist/node_modules/..., and those folders ship without a package.json.
  • Node's module-format lookup stops at any folder named node_modules, so those files lose this package's "type": "module" and load as CommonJS.
  • Node 22.7.0 and 20.19.0 turned on module syntax detection by default, which is why it loads on current Node and in the release smoke test. Older Node, and Yarn PnP's own loader, do not detect.

What I ran with the PR tarball:

  • npm install, Node 20.12.2, import('reactfire'): fails with "Named export 'Subject' not found. The requested module './node_modules/rxjs/dist/esm5/internal/Subject.js' is a CommonJS module".
  • Yarn 4.12 PnP project (loose mode, since firebase's own undeclared peers already fail strict PnP) on Node 22.23.2: same error.
  • The main tarball loads in both.

Who is affected: any app that loads reactfire through Node itself rather than a bundler (server rendering, test runners), on Node 20 before 20.19 (end of life, but engines still says >=14) or under Yarn PnP, on current Node too.

Fix I verified, one line in vite.config.ts:

  • Replace the ES output's entryFileNames string with a function that maps the node_modules/ prefix to vendor/, so the vendored files inherit this package's module scope.
  • Node 20.12.2 and the PnP project both import the result.
  • The auth-only Vite bundle stays at 106 kB with the same five @firebase packages.
  • I pushed it as branch armando/838-vendor-folder on this repo, one commit (1fa546c) on top of 2d644f7, in case cherry-picking is easier than retyping it.

A { "type": "module" } package.json placed in each of the three vendored folders also fixes Node 20.12.2 (not tested under PnP), but I would take the rename.

Everything below is optional and does not block the approve.

2. src/index.ts must define no runtime values, and nothing says so

  • I rebuilt with a runtime export added to index.ts and imported by auth.tsx. When rolldown cannot inline it (a function, a class, or a value that is re-exported), dist/auth.js imports ./index.js by name again.
  • That is the exact path that made Turbopack keep every product before the barrel move.
  • Nothing in CI would notice that: size-limit measures the whole package, and the tests import from src.
  • A one-line comment at the top of index.ts saying "types and re-exports only, define no runtime values here" would stop the next helper from undoing this silently.

3. The moved checkOptions keeps a cast the compiler does not need

  • npx tsc passes without the as ReactFireOptions['idField'] | ... | ReactFireOptions['suspense'] cast.
  • The emitted declarations are byte-identical with or without it, because checkOptions, checkIdField and checkinitialData are declared any either way (initialData is T | any).
  • Moved code, so whether to touch it in this PR is up to you.

4. build.lib.fileName: 'index' is now dead configuration

  • Both outputs set entryFileNames explicitly, so the fileName entry influences nothing and a reader will expect it to. Worth removing while in the file.

If I have misread the Node resolution rules or there is a reason to keep the node_modules folder name, say so and I will look again.

Comment thread vite.config.ts Outdated
format: 'es',
preserveModules: true,
preserveModulesRoot: 'src',
entryFileNames: '[name].js',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the line behind item 1. With preserveModules, the bundled rxjs, rxfire and tslib land under dist/node_modules/, and Node stops its package.json scope lookup at that folder name, so the files lose this package's "type": "module" and fail to import on Node before 20.19 / 22.7 and under Yarn PnP. A function here that maps the node_modules/ prefix to vendor/ fixes it, with the auth-only Vite bundle unchanged at 106 kB.

Comment thread src/index.ts
return checkOptions(options, 'idField');
}

export { ReactFireError, checkOptions, checkinitialData, checkIdField } from './utils.js';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Item 2. Moving these definitions out is what lets Turbopack drop unused products. A runtime function or class defined in this file and imported by any product module brings the regression back silently. Worth a one-line comment at the top of the file stating that this file holds types and re-exports only.

…leName

Removing lib.fileName leaves dist/ byte-identical, since rollupOptions.output sets entryFileNames for every format.
@tyler-reitz

Copy link
Copy Markdown
Contributor

Thanks, Armando. I took item 1 as your commit (1fa546c) and items 2 and 4 in d3c9779, and left 3 since it's moved code. This corrects the PR body, which said Node ESM loads dist/node_modules/: that was only true on Node with syntax detection, and the body now says dist/vendor/.

I reproduced item 1 before taking the fix. On Node 20.12.2, import('reactfire') fails on the 2d644f7 tarball with the same Named export 'Subject' not found and loads with dist/vendor/; both load on 22.15. I didn't run Yarn PnP, so that case rests on your run.

The packed file set is identical apart from the rename. Vite auth-only and all-hooks bundles are unchanged (104.8 and 289.3 kB). Firestore-only is 26 B smaller because rxjs's EmptyError now gets dropped; I haven't checked why.

@armando-navarro armando-navarro left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Tyler. I re-ran the checks on 225fb26 and everything holds:

  • import('reactfire') loads on Node 20.12.2, on Node 22 with syntax detection off, and under Yarn PnP.
  • The auth-only bundles are unchanged: 106 kB under Vite 7 and 210 kB under Next 16 Turbopack, with only the five @firebase packages the app uses.
  • A full build leaves the reference docs unchanged.

Leaving item 3 as is works for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@firebase/database included in the final build while I only use firestore

3 participants