Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/schematics/add/index.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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]);
Expand Down
111 changes: 110 additions & 1 deletion src/schematics/common.jasmine.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -115,3 +116,111 @@ describe('pinInstalledPrereleaseVersion', () => {
});

});

const treeWithFirebase = (firebaseVersion?: string, section = 'dependencies') => {
const tree = new HostTree();
const packageContents: Record<string, unknown> = { 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();
});

});
69 changes: 69 additions & 0 deletions src/schematics/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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';
Expand Down
5 changes: 5 additions & 0 deletions src/schematics/migration.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions src/schematics/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,6 @@
"add/index.ts",
"setup/index.ts",
"update/v7/index.ts",
"update/v21/index.ts",
]
}
43 changes: 43 additions & 0 deletions src/schematics/update/v21/index.jasmine.ts
Original file line number Diff line number Diff line change
@@ -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();
});

});
17 changes: 17 additions & 0 deletions src/schematics/update/v21/index.ts
Original file line number Diff line number Diff line change
@@ -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;
};
1 change: 1 addition & 0 deletions tools/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading