fix(schematics): align the workspace firebase dependency with the supported range#3722
Conversation
…ported range Upgrading an app whose own package.json still pins firebase 11 leaves npm building a tree with two firebase SDK copies: the stale one at the root (rxfire binds to it) and a second nested under @angular/fire. No install-time error fires, and the two copies reject each other's objects at runtime; angular#3684, angular#3681, and angular#3682 are all this one failure. A shared alignFirebaseVersion helper now runs in both places the schematics touch a project: ng add writes or realigns the firebase entry before its install task, and a new migration-v21 does the same during ng update, gated so a re-run on rc-to-stable updates is a no-op (the CLI strips the prerelease from the migration range's upper bound, so the migration re-executes on those transitions). Ranges not entirely inside ^12.4.0 are rewritten via semver.subset; intersects would keep ranges like ^12.0.0 or >=11 that a stale lockfile still resolves below the floor. Non-semver specifiers (dist-tags, workspace:, git URLs) are warned about, never rewritten. The ^12.4.0 constant deliberately duplicates dependencies.firebase from src/package.json rather than deriving it at build time: the build's version-injection machinery (versions.json) is under discussion in angular#3715, so this stays the minimal bridge that machinery can later supersede. A spec reads src/package.json and fails the suite if the two values ever drift. Refs angular#3684 Refs angular#3681 Refs angular#3682
tyler-reitz
left a comment
There was a problem hiding this comment.
Approve. Good semver handling; subset over intersects correctly catches ranges like >=11 that intersects would miss, and non-semver specifiers (workspace:*, git URLs) are left alone with a warning rather than mangled. The constant-drift test against src/package.json is a nice guard against the two values silently diverging later.
Minor: v21/index.ts uses explicit .js-suffixed imports where v7/index.ts doesn't, worth a one-line note in the description so it doesn't get "fixed" back to match v7 (it's needed for the dedicated jasmine spec to import it directly under ESM).
|
Thanks. I've added a reviewers note to the description explaining that the |
Checklist
Refsrather thanFixes: no published build carries the change until the next release is cut or a canary is published, so the issues should close when reporters can actually install the fix.)ng addprint a one-line explanation whenever they change anything, and the v21 upgrade documentation is tracked with the release notes.src/package.json, and the migration's install-task behavior).yarn install,yarn testrun successfully? Yes;npm run buildand the node test suites run green locally (55 specs), and the migration was verified end-to-end against a packed build (below).Description
Upgrading an app to
@angular/fire21 while its ownpackage.jsonstill requestsfirebase@11produces an install with two copies of the firebase SDK: npm keeps 11 at the root and nests 12 under@angular/fire. No install-time error fires, and the two copies reject each other's objects at runtime. #3684, #3681, and #3682 are all this one failure (analysis and reproductions in those threads).This PR makes the schematics keep the workspace's
firebaseentry aligned with the range the library requires:alignFirebaseVersionhelper andfirebaseVersionRangeconstant insrc/schematics/common.ts;ng addwrites the entry when missing and realigns an incompatible one before its install task runs;migration-v21runs the same alignment duringng update @angular/fire, scheduling an install only when it changed something (the CLI re-runs the migration on rc-to-stable updates, so the no-op path matters).Behavior details:
^12.4.0(semver.subset);intersectswould keep ranges like^12.0.0or>=11that a stale lockfile still resolves below the required floor.workspace:,file:, git URLs) are never rewritten; the user gets a warning explaining the risk instead.firebaseVersionRangeintentionally duplicatesdependencies.firebasefromsrc/package.jsoninstead of being injected at build time: the build's version-injection machinery (versions.json) is under discussion in TheANGULARFIRE2_VERSIONbuild placeholder ships unreplaced in the library, andversions.jsonnever reaches the compiled schematics #3715, so the constant is the minimal bridge that machinery can later supersede. A spec readssrc/package.jsonand fails the suite if the two values ever drift.Verified end-to-end against a packed build. A fixture app with
firebase@11.8.0installed next to this package (the exact broken tree from the linked issues), thenng update @angular/fire --migrate-only --from=20.0.1:npm ls firebaseafterwards shows a singlefirebase@12.16.0, and a second run reports "Migration completed (No changes made)".Note for reviewers
src/schematics/update/v21/index.tsuses explicit.js-suffixed relative imports wherev7/index.tsdoes not. This is intentional: the dedicated jasmine spec imports the migration directly under ESM, which requires the suffix. Please do not "fix" it back to matchv7.