Skip to content

Commit 11e29ff

Browse files
fix(schematics): align the workspace firebase dependency with the supported range (#3722)
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
1 parent 3521538 commit 11e29ff

8 files changed

Lines changed: 248 additions & 2 deletions

File tree

src/schematics/add/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { SchematicContext, Tree } from '@angular-devkit/schematics';
22
import { NodePackageInstallTask, RunSchematicTask } from '@angular-devkit/schematics/tasks';
3-
import { addDependencies, pinInstalledPrereleaseVersion } from '../common';
3+
import { addDependencies, alignFirebaseVersion, pinInstalledPrereleaseVersion } from '../common';
44
import { DeployOptions } from '../interfaces';
55
import { peerDependencies } from '../versions.json';
66

@@ -10,6 +10,7 @@ export const ngAdd = (options: DeployOptions) => (tree: Tree, context: Schematic
1010
peerDependencies,
1111
context
1212
);
13+
alignFirebaseVersion(tree, context);
1314
pinInstalledPrereleaseVersion(tree, context);
1415
const npmInstallTaskId = context.addTask(new NodePackageInstallTask());
1516
context.addTask(new RunSchematicTask('ng-add-setup-project', options), [npmInstallTaskId]);

src/schematics/common.jasmine.ts

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1+
import { readFileSync } from 'fs';
12
import { logging } from '@angular-devkit/core';
23
import { HostTree, SchematicContext } from '@angular-devkit/schematics';
3-
import { pinInstalledPrereleaseVersion } from './common.js';
4+
import { alignFirebaseVersion, firebaseVersionRange, pinInstalledPrereleaseVersion } from './common.js';
45
import 'jasmine';
56

67
const context = { logger: new logging.Logger('test') } as unknown as SchematicContext;
@@ -115,3 +116,111 @@ describe('pinInstalledPrereleaseVersion', () => {
115116
});
116117

117118
});
119+
120+
const treeWithFirebase = (firebaseVersion?: string, section = 'dependencies') => {
121+
const tree = new HostTree();
122+
const packageContents: Record<string, unknown> = { name: 'test-app' };
123+
if (firebaseVersion !== undefined) {
124+
packageContents[section] = { firebase: firebaseVersion };
125+
}
126+
tree.create('package.json', JSON.stringify(packageContents, null, 2));
127+
return tree;
128+
};
129+
130+
describe('alignFirebaseVersion', () => {
131+
132+
it('rewrites a previous-major range', () => {
133+
const tree = treeWithFirebase('^11.0.0');
134+
expect(alignFirebaseVersion(tree, context)).toBeTrue();
135+
expect(dependenciesIn(tree).firebase).toBe(firebaseVersionRange);
136+
});
137+
138+
it('rewrites a same-major range that still allows versions below the required floor', () => {
139+
const tree = treeWithFirebase('^12.0.0');
140+
expect(alignFirebaseVersion(tree, context)).toBeTrue();
141+
expect(dependenciesIn(tree).firebase).toBe(firebaseVersionRange);
142+
});
143+
144+
it('rewrites a wide range that a stale lockfile can hold below the floor', () => {
145+
const tree = treeWithFirebase('>=11');
146+
expect(alignFirebaseVersion(tree, context)).toBeTrue();
147+
expect(dependenciesIn(tree).firebase).toBe(firebaseVersionRange);
148+
});
149+
150+
it('leaves a compatible range untouched', () => {
151+
const tree = treeWithFirebase('^12.6.0');
152+
expect(alignFirebaseVersion(tree, context)).toBeFalse();
153+
expect(dependenciesIn(tree).firebase).toBe('^12.6.0');
154+
});
155+
156+
it('adds firebase when the workspace has none', () => {
157+
const tree = treeWithFirebase(undefined);
158+
expect(alignFirebaseVersion(tree, context)).toBeTrue();
159+
expect(dependenciesIn(tree).firebase).toBe(firebaseVersionRange);
160+
});
161+
162+
it('aligns firebase in devDependencies without duplicating it into dependencies', () => {
163+
const tree = treeWithFirebase('^11.0.0', 'devDependencies');
164+
expect(alignFirebaseVersion(tree, context)).toBeTrue();
165+
expect(sectionIn(tree, 'devDependencies').firebase).toBe(firebaseVersionRange);
166+
expect(sectionIn(tree, 'dependencies')).toBeUndefined();
167+
});
168+
169+
it('warns about a non-semver specifier and leaves it as-is', () => {
170+
const warnSpy = spyOn(context.logger, 'warn');
171+
const tree = treeWithFirebase('latest');
172+
expect(alignFirebaseVersion(tree, context)).toBeFalse();
173+
expect(dependenciesIn(tree).firebase).toBe('latest');
174+
expect(warnSpy).toHaveBeenCalled();
175+
});
176+
177+
it('warns about a workspace specifier and leaves it as-is', () => {
178+
const warnSpy = spyOn(context.logger, 'warn');
179+
const tree = treeWithFirebase('workspace:*');
180+
expect(alignFirebaseVersion(tree, context)).toBeFalse();
181+
expect(dependenciesIn(tree).firebase).toBe('workspace:*');
182+
expect(warnSpy).toHaveBeenCalled();
183+
});
184+
185+
it('rewrites stale entries in both sections when firebase appears twice', () => {
186+
const tree = new HostTree();
187+
tree.create('package.json', JSON.stringify({
188+
name: 'test-app',
189+
dependencies: { firebase: '^11.0.0' },
190+
devDependencies: { firebase: '~11.9.0' },
191+
}, null, 2));
192+
expect(alignFirebaseVersion(tree, context)).toBeTrue();
193+
expect(dependenciesIn(tree).firebase).toBe(firebaseVersionRange);
194+
expect(sectionIn(tree, 'devDependencies').firebase).toBe(firebaseVersionRange);
195+
});
196+
197+
it('rewrites only the stale section when the other is already compatible', () => {
198+
const tree = new HostTree();
199+
tree.create('package.json', JSON.stringify({
200+
name: 'test-app',
201+
dependencies: { firebase: '^12.6.0' },
202+
devDependencies: { firebase: '^11.0.0' },
203+
}, null, 2));
204+
expect(alignFirebaseVersion(tree, context)).toBeTrue();
205+
expect(dependenciesIn(tree).firebase).toBe('^12.6.0');
206+
expect(sectionIn(tree, 'devDependencies').firebase).toBe(firebaseVersionRange);
207+
});
208+
209+
it('matches the firebase range the library actually ships with', () => {
210+
const libraryManifest = JSON.parse(readFileSync('src/package.json', 'utf8'));
211+
expect(libraryManifest.dependencies.firebase).toBe(firebaseVersionRange);
212+
});
213+
214+
it('is a no-op when run a second time', () => {
215+
const tree = treeWithFirebase('^11.0.0');
216+
expect(alignFirebaseVersion(tree, context)).toBeTrue();
217+
expect(alignFirebaseVersion(tree, context)).toBeFalse();
218+
expect(dependenciesIn(tree).firebase).toBe(firebaseVersionRange);
219+
});
220+
221+
it('throws when package.json is absent', () => {
222+
const tree = new HostTree();
223+
expect(() => alignFirebaseVersion(tree, context)).toThrow();
224+
});
225+
226+
});

src/schematics/common.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
intersects as semverIntersects,
44
prerelease as semverPrerelease,
55
satisfies as semverSatisfies,
6+
subset as semverSubset,
67
valid as semverValid,
78
} from 'semver';
89
import { FirebaseHostingSite } from './interfaces';
@@ -71,6 +72,74 @@ export const addDependencies = (
7172
overwriteIfExists(host, 'package.json', stringifyFormatted(packageJson));
7273
};
7374

75+
// Must stay identical to `dependencies.firebase` in `src/package.json`: if the two drift, the
76+
// alignment below can pin workspaces outside the range the library actually installs against,
77+
// re-creating the duplicate-SDK trees it exists to prevent.
78+
export const firebaseVersionRange = '^12.4.0';
79+
80+
/**
81+
* Aligns the workspace's `firebase` entry with the range `@angular/fire` requires.
82+
*
83+
* `@angular/fire` bundles its own `firebase` dependency, so a workspace pinned to an older
84+
* major never conflicts at install time; npm silently nests a second SDK copy under
85+
* `@angular/fire`, and the two copies reject each other's objects at runtime (#3684, #3681,
86+
* #3682). Rewriting the workspace's range before the install runs is the only point where
87+
* that class of breakage can be headed off.
88+
*
89+
* Returns whether `package.json` was modified, so callers can skip scheduling an install
90+
* when nothing changed (`ng update` re-runs the v21 migration on rc-to-stable updates).
91+
*/
92+
export const alignFirebaseVersion = (
93+
host: Tree,
94+
context: SchematicContext,
95+
): boolean => {
96+
if (!host.exists('package.json')) {
97+
throw new SchematicsException('Could not locate package.json');
98+
}
99+
const packageJson = safeReadJSON('package.json', host);
100+
101+
const sectionsWithFirebase = ['dependencies', 'devDependencies'].filter(
102+
section => typeof packageJson[section]?.firebase === 'string',
103+
);
104+
105+
if (sectionsWithFirebase.length === 0) {
106+
packageJson.dependencies ??= {};
107+
packageJson.dependencies.firebase = firebaseVersionRange;
108+
context.logger.info(`Added firebase ${firebaseVersionRange} to your package.json.`);
109+
overwriteIfExists(host, 'package.json', stringifyFormatted(packageJson));
110+
return true;
111+
}
112+
113+
let changed = false;
114+
for (const section of sectionsWithFirebase) {
115+
const declaredFirebaseVersion = packageJson[section].firebase;
116+
let alreadyCompatible: boolean;
117+
try {
118+
alreadyCompatible = semverSubset(declaredFirebaseVersion, firebaseVersionRange);
119+
} catch (_) {
120+
context.logger.warn(
121+
`⚠️ The firebase version in your package.json (${declaredFirebaseVersion}) is not a semver ` +
122+
`range, so it was left as-is; make sure it resolves inside ${firebaseVersionRange}, the ` +
123+
'range @angular/fire requires; a version outside it can leave the install with two copies of the firebase SDK.'
124+
);
125+
continue;
126+
}
127+
if (alreadyCompatible) { continue; }
128+
packageJson[section].firebase = firebaseVersionRange;
129+
context.logger.info(
130+
`Updated the firebase version in your package.json from ${declaredFirebaseVersion} to ` +
131+
`${firebaseVersionRange}, the range @angular/fire requires; a workspace range outside it ` +
132+
'can leave the install with a second copy of the firebase SDK, which fails at runtime.'
133+
);
134+
changed = true;
135+
}
136+
137+
if (changed) {
138+
overwriteIfExists(host, 'package.json', stringifyFormatted(packageJson));
139+
}
140+
return changed;
141+
};
142+
74143
// The build writes the published version over this placeholder in the compiled schematics
75144
// (tools/build.ts, replaceSchematicVersions); running from source leaves the placeholder.
76145
const angularFireVersion = 'ANGULARFIRE2_VERSION';

src/schematics/migration.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@
66
"description": "Update @angular/fire to v7",
77
"factory": "./update/v7#ngUpdate"
88
},
9+
"migration-v21": {
10+
"version": "21.0.0",
11+
"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",
12+
"factory": "./update/v21#ngUpdate"
13+
},
914
"ng-post-upgate": {
1015
"description": "Print out results after ng-update",
1116
"factory": "./update#ngPostUpdate",

src/schematics/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,5 +28,6 @@
2828
"add/index.ts",
2929
"setup/index.ts",
3030
"update/v7/index.ts",
31+
"update/v21/index.ts",
3132
]
3233
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import { logging } from '@angular-devkit/core';
2+
import { HostTree, SchematicContext } from '@angular-devkit/schematics';
3+
import { firebaseVersionRange } from '../../common.js';
4+
import { ngUpdate } from './index.js';
5+
import 'jasmine';
6+
7+
const contextWithTaskSpy = () => {
8+
const addTask = jasmine.createSpy('addTask');
9+
const context = {
10+
logger: new logging.Logger('test'),
11+
addTask,
12+
} as unknown as SchematicContext;
13+
return { context, addTask };
14+
};
15+
16+
const treeWithFirebase = (firebaseVersion?: string) => {
17+
const tree = new HostTree();
18+
tree.create('package.json', JSON.stringify({
19+
name: 'test-app',
20+
dependencies: firebaseVersion === undefined ? {} : { firebase: firebaseVersion },
21+
}, null, 2));
22+
return tree;
23+
};
24+
25+
describe('migration-v21 ngUpdate', () => {
26+
27+
it('aligns a stale firebase range and schedules an install', () => {
28+
const { context, addTask } = contextWithTaskSpy();
29+
const tree = treeWithFirebase('^11.0.0');
30+
ngUpdate()(tree, context);
31+
const written = JSON.parse(tree.readText('package.json'));
32+
expect(written.dependencies.firebase).toBe(firebaseVersionRange);
33+
expect(addTask).toHaveBeenCalledTimes(1);
34+
});
35+
36+
it('schedules nothing when the workspace is already aligned', () => {
37+
const { context, addTask } = contextWithTaskSpy();
38+
const tree = treeWithFirebase(firebaseVersionRange);
39+
ngUpdate()(tree, context);
40+
expect(addTask).not.toHaveBeenCalled();
41+
});
42+
43+
});

src/schematics/update/v21/index.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { Rule, SchematicContext, Tree } from '@angular-devkit/schematics';
2+
// The explicit index.js subpath keeps this importable from the ESM jasmine run; the bare
3+
// /tasks directory specifier only resolves under CommonJS.
4+
import { NodePackageInstallTask } from '@angular-devkit/schematics/tasks/index.js';
5+
import { alignFirebaseVersion } from '../../common.js';
6+
7+
// ng update re-runs this migration on rc-to-stable transitions (the CLI clamps the migration
8+
// range's upper bound to the release version), so it must stay a no-op when nothing changes.
9+
export const ngUpdate = (): Rule => (
10+
host: Tree,
11+
context: SchematicContext
12+
) => {
13+
if (alignFirebaseVersion(host, context)) {
14+
context.addTask(new NodePackageInstallTask());
15+
}
16+
return host;
17+
};

tools/build.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,7 @@ async function compileSchematics() {
322322
src('schematics', "add", "index.ts"),
323323
src('schematics', "setup", "index.ts"),
324324
src('schematics', "update", "v7", "index.ts"),
325+
src('schematics', "update", "v21", "index.ts"),
325326
],
326327
format: "cjs",
327328
// turns out schematics don't support ESM, need to use webpack or shim these

0 commit comments

Comments
 (0)