From ef48a7495fb6ba3e145f52c02a98ac01384d624e Mon Sep 17 00:00:00 2001 From: Armando Navarro Date: Tue, 21 Jul 2026 16:41:15 -0700 Subject: [PATCH] fix(schematics): align the workspace firebase dependency with the supported 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; #3684, #3681, and #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 #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 #3684 Refs #3681 Refs #3682 --- src/schematics/add/index.ts | 3 +- src/schematics/common.jasmine.ts | 111 ++++++++++++++++++++- src/schematics/common.ts | 69 +++++++++++++ src/schematics/migration.json | 5 + src/schematics/tsconfig.json | 1 + src/schematics/update/v21/index.jasmine.ts | 43 ++++++++ src/schematics/update/v21/index.ts | 17 ++++ tools/build.ts | 1 + 8 files changed, 248 insertions(+), 2 deletions(-) create mode 100644 src/schematics/update/v21/index.jasmine.ts create mode 100644 src/schematics/update/v21/index.ts diff --git a/src/schematics/add/index.ts b/src/schematics/add/index.ts index 2a9c926ec..a2636952a 100644 --- a/src/schematics/add/index.ts +++ b/src/schematics/add/index.ts @@ -1,6 +1,6 @@ import { SchematicContext, Tree } from '@angular-devkit/schematics'; import { NodePackageInstallTask, RunSchematicTask } from '@angular-devkit/schematics/tasks'; -import { addDependencies, pinInstalledPrereleaseVersion } from '../common'; +import { addDependencies, alignFirebaseVersion, pinInstalledPrereleaseVersion } from '../common'; import { DeployOptions } from '../interfaces'; import { peerDependencies } from '../versions.json'; @@ -10,6 +10,7 @@ export const ngAdd = (options: DeployOptions) => (tree: Tree, context: Schematic peerDependencies, context ); + alignFirebaseVersion(tree, context); pinInstalledPrereleaseVersion(tree, context); const npmInstallTaskId = context.addTask(new NodePackageInstallTask()); context.addTask(new RunSchematicTask('ng-add-setup-project', options), [npmInstallTaskId]); diff --git a/src/schematics/common.jasmine.ts b/src/schematics/common.jasmine.ts index b40612d04..0322ae89d 100644 --- a/src/schematics/common.jasmine.ts +++ b/src/schematics/common.jasmine.ts @@ -1,6 +1,7 @@ +import { readFileSync } from 'fs'; import { logging } from '@angular-devkit/core'; import { HostTree, SchematicContext } from '@angular-devkit/schematics'; -import { pinInstalledPrereleaseVersion } from './common.js'; +import { alignFirebaseVersion, firebaseVersionRange, pinInstalledPrereleaseVersion } from './common.js'; import 'jasmine'; const context = { logger: new logging.Logger('test') } as unknown as SchematicContext; @@ -115,3 +116,111 @@ describe('pinInstalledPrereleaseVersion', () => { }); }); + +const treeWithFirebase = (firebaseVersion?: string, section = 'dependencies') => { + const tree = new HostTree(); + const packageContents: Record = { name: 'test-app' }; + if (firebaseVersion !== undefined) { + packageContents[section] = { firebase: firebaseVersion }; + } + tree.create('package.json', JSON.stringify(packageContents, null, 2)); + return tree; +}; + +describe('alignFirebaseVersion', () => { + + it('rewrites a previous-major range', () => { + const tree = treeWithFirebase('^11.0.0'); + expect(alignFirebaseVersion(tree, context)).toBeTrue(); + expect(dependenciesIn(tree).firebase).toBe(firebaseVersionRange); + }); + + it('rewrites a same-major range that still allows versions below the required floor', () => { + const tree = treeWithFirebase('^12.0.0'); + expect(alignFirebaseVersion(tree, context)).toBeTrue(); + expect(dependenciesIn(tree).firebase).toBe(firebaseVersionRange); + }); + + it('rewrites a wide range that a stale lockfile can hold below the floor', () => { + const tree = treeWithFirebase('>=11'); + expect(alignFirebaseVersion(tree, context)).toBeTrue(); + expect(dependenciesIn(tree).firebase).toBe(firebaseVersionRange); + }); + + it('leaves a compatible range untouched', () => { + const tree = treeWithFirebase('^12.6.0'); + expect(alignFirebaseVersion(tree, context)).toBeFalse(); + expect(dependenciesIn(tree).firebase).toBe('^12.6.0'); + }); + + it('adds firebase when the workspace has none', () => { + const tree = treeWithFirebase(undefined); + expect(alignFirebaseVersion(tree, context)).toBeTrue(); + expect(dependenciesIn(tree).firebase).toBe(firebaseVersionRange); + }); + + it('aligns firebase in devDependencies without duplicating it into dependencies', () => { + const tree = treeWithFirebase('^11.0.0', 'devDependencies'); + expect(alignFirebaseVersion(tree, context)).toBeTrue(); + expect(sectionIn(tree, 'devDependencies').firebase).toBe(firebaseVersionRange); + expect(sectionIn(tree, 'dependencies')).toBeUndefined(); + }); + + it('warns about a non-semver specifier and leaves it as-is', () => { + const warnSpy = spyOn(context.logger, 'warn'); + const tree = treeWithFirebase('latest'); + expect(alignFirebaseVersion(tree, context)).toBeFalse(); + expect(dependenciesIn(tree).firebase).toBe('latest'); + expect(warnSpy).toHaveBeenCalled(); + }); + + it('warns about a workspace specifier and leaves it as-is', () => { + const warnSpy = spyOn(context.logger, 'warn'); + const tree = treeWithFirebase('workspace:*'); + expect(alignFirebaseVersion(tree, context)).toBeFalse(); + expect(dependenciesIn(tree).firebase).toBe('workspace:*'); + expect(warnSpy).toHaveBeenCalled(); + }); + + it('rewrites stale entries in both sections when firebase appears twice', () => { + const tree = new HostTree(); + tree.create('package.json', JSON.stringify({ + name: 'test-app', + dependencies: { firebase: '^11.0.0' }, + devDependencies: { firebase: '~11.9.0' }, + }, null, 2)); + expect(alignFirebaseVersion(tree, context)).toBeTrue(); + expect(dependenciesIn(tree).firebase).toBe(firebaseVersionRange); + expect(sectionIn(tree, 'devDependencies').firebase).toBe(firebaseVersionRange); + }); + + it('rewrites only the stale section when the other is already compatible', () => { + const tree = new HostTree(); + tree.create('package.json', JSON.stringify({ + name: 'test-app', + dependencies: { firebase: '^12.6.0' }, + devDependencies: { firebase: '^11.0.0' }, + }, null, 2)); + expect(alignFirebaseVersion(tree, context)).toBeTrue(); + expect(dependenciesIn(tree).firebase).toBe('^12.6.0'); + expect(sectionIn(tree, 'devDependencies').firebase).toBe(firebaseVersionRange); + }); + + it('matches the firebase range the library actually ships with', () => { + const libraryManifest = JSON.parse(readFileSync('src/package.json', 'utf8')); + expect(libraryManifest.dependencies.firebase).toBe(firebaseVersionRange); + }); + + it('is a no-op when run a second time', () => { + const tree = treeWithFirebase('^11.0.0'); + expect(alignFirebaseVersion(tree, context)).toBeTrue(); + expect(alignFirebaseVersion(tree, context)).toBeFalse(); + expect(dependenciesIn(tree).firebase).toBe(firebaseVersionRange); + }); + + it('throws when package.json is absent', () => { + const tree = new HostTree(); + expect(() => alignFirebaseVersion(tree, context)).toThrow(); + }); + +}); diff --git a/src/schematics/common.ts b/src/schematics/common.ts index 525440321..d34d3c643 100644 --- a/src/schematics/common.ts +++ b/src/schematics/common.ts @@ -3,6 +3,7 @@ import { intersects as semverIntersects, prerelease as semverPrerelease, satisfies as semverSatisfies, + subset as semverSubset, valid as semverValid, } from 'semver'; import { FirebaseHostingSite } from './interfaces'; @@ -71,6 +72,74 @@ export const addDependencies = ( overwriteIfExists(host, 'package.json', stringifyFormatted(packageJson)); }; +// Must stay identical to `dependencies.firebase` in `src/package.json`: if the two drift, the +// alignment below can pin workspaces outside the range the library actually installs against, +// re-creating the duplicate-SDK trees it exists to prevent. +export const firebaseVersionRange = '^12.4.0'; + +/** + * Aligns the workspace's `firebase` entry with the range `@angular/fire` requires. + * + * `@angular/fire` bundles its own `firebase` dependency, so a workspace pinned to an older + * major never conflicts at install time; npm silently nests a second SDK copy under + * `@angular/fire`, and the two copies reject each other's objects at runtime (#3684, #3681, + * #3682). Rewriting the workspace's range before the install runs is the only point where + * that class of breakage can be headed off. + * + * Returns whether `package.json` was modified, so callers can skip scheduling an install + * when nothing changed (`ng update` re-runs the v21 migration on rc-to-stable updates). + */ +export const alignFirebaseVersion = ( + host: Tree, + context: SchematicContext, +): boolean => { + if (!host.exists('package.json')) { + throw new SchematicsException('Could not locate package.json'); + } + const packageJson = safeReadJSON('package.json', host); + + const sectionsWithFirebase = ['dependencies', 'devDependencies'].filter( + section => typeof packageJson[section]?.firebase === 'string', + ); + + if (sectionsWithFirebase.length === 0) { + packageJson.dependencies ??= {}; + packageJson.dependencies.firebase = firebaseVersionRange; + context.logger.info(`Added firebase ${firebaseVersionRange} to your package.json.`); + overwriteIfExists(host, 'package.json', stringifyFormatted(packageJson)); + return true; + } + + let changed = false; + for (const section of sectionsWithFirebase) { + const declaredFirebaseVersion = packageJson[section].firebase; + let alreadyCompatible: boolean; + try { + alreadyCompatible = semverSubset(declaredFirebaseVersion, firebaseVersionRange); + } catch (_) { + context.logger.warn( + `⚠️ The firebase version in your package.json (${declaredFirebaseVersion}) is not a semver ` + + `range, so it was left as-is; make sure it resolves inside ${firebaseVersionRange}, the ` + + 'range @angular/fire requires; a version outside it can leave the install with two copies of the firebase SDK.' + ); + continue; + } + if (alreadyCompatible) { continue; } + packageJson[section].firebase = firebaseVersionRange; + context.logger.info( + `Updated the firebase version in your package.json from ${declaredFirebaseVersion} to ` + + `${firebaseVersionRange}, the range @angular/fire requires; a workspace range outside it ` + + 'can leave the install with a second copy of the firebase SDK, which fails at runtime.' + ); + changed = true; + } + + if (changed) { + overwriteIfExists(host, 'package.json', stringifyFormatted(packageJson)); + } + return changed; +}; + // The build writes the published version over this placeholder in the compiled schematics // (tools/build.ts, replaceSchematicVersions); running from source leaves the placeholder. const angularFireVersion = 'ANGULARFIRE2_VERSION'; diff --git a/src/schematics/migration.json b/src/schematics/migration.json index 453d5a9e3..9dfd04e02 100644 --- a/src/schematics/migration.json +++ b/src/schematics/migration.json @@ -6,6 +6,11 @@ "description": "Update @angular/fire to v7", "factory": "./update/v7#ngUpdate" }, + "migration-v21": { + "version": "21.0.0", + "description": "Align the workspace's firebase dependency with the range @angular/fire 21 requires, so the install cannot contain two copies of the firebase SDK", + "factory": "./update/v21#ngUpdate" + }, "ng-post-upgate": { "description": "Print out results after ng-update", "factory": "./update#ngPostUpdate", diff --git a/src/schematics/tsconfig.json b/src/schematics/tsconfig.json index ea6375bea..bc0b86afd 100644 --- a/src/schematics/tsconfig.json +++ b/src/schematics/tsconfig.json @@ -28,5 +28,6 @@ "add/index.ts", "setup/index.ts", "update/v7/index.ts", + "update/v21/index.ts", ] } \ No newline at end of file diff --git a/src/schematics/update/v21/index.jasmine.ts b/src/schematics/update/v21/index.jasmine.ts new file mode 100644 index 000000000..87ebe1353 --- /dev/null +++ b/src/schematics/update/v21/index.jasmine.ts @@ -0,0 +1,43 @@ +import { logging } from '@angular-devkit/core'; +import { HostTree, SchematicContext } from '@angular-devkit/schematics'; +import { firebaseVersionRange } from '../../common.js'; +import { ngUpdate } from './index.js'; +import 'jasmine'; + +const contextWithTaskSpy = () => { + const addTask = jasmine.createSpy('addTask'); + const context = { + logger: new logging.Logger('test'), + addTask, + } as unknown as SchematicContext; + return { context, addTask }; +}; + +const treeWithFirebase = (firebaseVersion?: string) => { + const tree = new HostTree(); + tree.create('package.json', JSON.stringify({ + name: 'test-app', + dependencies: firebaseVersion === undefined ? {} : { firebase: firebaseVersion }, + }, null, 2)); + return tree; +}; + +describe('migration-v21 ngUpdate', () => { + + it('aligns a stale firebase range and schedules an install', () => { + const { context, addTask } = contextWithTaskSpy(); + const tree = treeWithFirebase('^11.0.0'); + ngUpdate()(tree, context); + const written = JSON.parse(tree.readText('package.json')); + expect(written.dependencies.firebase).toBe(firebaseVersionRange); + expect(addTask).toHaveBeenCalledTimes(1); + }); + + it('schedules nothing when the workspace is already aligned', () => { + const { context, addTask } = contextWithTaskSpy(); + const tree = treeWithFirebase(firebaseVersionRange); + ngUpdate()(tree, context); + expect(addTask).not.toHaveBeenCalled(); + }); + +}); diff --git a/src/schematics/update/v21/index.ts b/src/schematics/update/v21/index.ts new file mode 100644 index 000000000..3920d9b90 --- /dev/null +++ b/src/schematics/update/v21/index.ts @@ -0,0 +1,17 @@ +import { Rule, SchematicContext, Tree } from '@angular-devkit/schematics'; +// The explicit index.js subpath keeps this importable from the ESM jasmine run; the bare +// /tasks directory specifier only resolves under CommonJS. +import { NodePackageInstallTask } from '@angular-devkit/schematics/tasks/index.js'; +import { alignFirebaseVersion } from '../../common.js'; + +// ng update re-runs this migration on rc-to-stable transitions (the CLI clamps the migration +// range's upper bound to the release version), so it must stay a no-op when nothing changes. +export const ngUpdate = (): Rule => ( + host: Tree, + context: SchematicContext +) => { + if (alignFirebaseVersion(host, context)) { + context.addTask(new NodePackageInstallTask()); + } + return host; +}; diff --git a/tools/build.ts b/tools/build.ts index 0e66812e1..5b3397360 100644 --- a/tools/build.ts +++ b/tools/build.ts @@ -322,6 +322,7 @@ async function compileSchematics() { src('schematics', "add", "index.ts"), src('schematics', "setup", "index.ts"), src('schematics', "update", "v7", "index.ts"), + src('schematics', "update", "v21", "index.ts"), ], format: "cjs", // turns out schematics don't support ESM, need to use webpack or shim these