Skip to content

Commit b8bf3b4

Browse files
clydindgp1130
authored andcommitted
fix(@schematics/angular): only issue a warning for addDependency existing specifier
When using the `addDependency` helper rule, a package specifier mismatch will now result in a warning instead of an error. This prevents the potential for breaking changes in schematics that previously allowed the behavior. The warning will also display the existing and new specifiers to better inform the schematic user of the outcome of the schematic.
1 parent 5708ac3 commit b8bf3b4

File tree

2 files changed

+53
-24
lines changed

2 files changed

+53
-24
lines changed

packages/schematics/angular/utility/dependency.ts

+16-6
Original file line numberDiff line numberDiff line change
@@ -103,13 +103,23 @@ export function addDependency(
103103
if (!dependencySection) {
104104
// Section is not present. The dependency can be added to a new object literal for the section.
105105
manifest[type] = { [name]: specifier };
106-
} else if (dependencySection[name] === specifier) {
107-
// Already present with same specifier
108-
return;
109-
} else if (dependencySection[name]) {
110-
// Already present but different specifier
111-
throw new Error(`Package dependency "${name}" already exists with a different specifier.`);
112106
} else {
107+
const existingSpecifier = dependencySection[name];
108+
109+
if (existingSpecifier === specifier) {
110+
// Already present with same specifier
111+
return;
112+
}
113+
114+
if (existingSpecifier) {
115+
// Already present but different specifier
116+
// This warning may become an error in the future
117+
context.logger.warn(
118+
`Package dependency "${name}" already exists with a different specifier. ` +
119+
`"${existingSpecifier}" will be replaced with "${specifier}".`,
120+
);
121+
}
122+
113123
// Add new dependency in alphabetical order
114124
const entries = Object.entries(dependencySection);
115125
entries.push([name, specifier]);

packages/schematics/angular/utility/dependency_spec.ts

+37-18
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,31 @@ import {
1717
} from '@angular-devkit/schematics';
1818
import { DependencyType, InstallBehavior, addDependency } from './dependency';
1919

20-
async function testRule(rule: Rule, tree: Tree): Promise<TaskConfigurationGenerator[]> {
20+
interface LogEntry {
21+
type: 'warn';
22+
message: string;
23+
}
24+
25+
async function testRule(
26+
rule: Rule,
27+
tree: Tree,
28+
): Promise<{ tasks: TaskConfigurationGenerator[]; logs: LogEntry[] }> {
2129
const tasks: TaskConfigurationGenerator[] = [];
30+
const logs: LogEntry[] = [];
2231
const context = {
2332
addTask(task: TaskConfigurationGenerator) {
2433
tasks.push(task);
2534
},
35+
logger: {
36+
warn(message: string): void {
37+
logs.push({ type: 'warn', message });
38+
},
39+
},
2640
};
2741

2842
await callRule(rule, tree, context as unknown as SchematicContext).toPromise();
2943

30-
return tasks;
44+
return { tasks, logs };
3145
}
3246

3347
describe('addDependency', () => {
@@ -49,7 +63,7 @@ describe('addDependency', () => {
4963
});
5064
});
5165

52-
it('throws if a package is already present with a different specifier', async () => {
66+
it('warns if a package is already present with a different specifier', async () => {
5367
const tree = new EmptyTree();
5468
tree.create(
5569
'/package.json',
@@ -60,9 +74,14 @@ describe('addDependency', () => {
6074

6175
const rule = addDependency('@angular/core', '^14.0.0');
6276

63-
await expectAsync(testRule(rule, tree)).toBeRejectedWithError(
64-
undefined,
65-
'Package dependency "@angular/core" already exists with a different specifier.',
77+
const { logs } = await testRule(rule, tree);
78+
expect(logs).toContain(
79+
jasmine.objectContaining({
80+
type: 'warn',
81+
message:
82+
'Package dependency "@angular/core" already exists with a different specifier. ' +
83+
'"^13.0.0" will be replaced with "^14.0.0".',
84+
}),
6685
);
6786
});
6887

@@ -164,7 +183,7 @@ describe('addDependency', () => {
164183

165184
const rule = addDependency('@angular/core', '^14.0.0');
166185

167-
const tasks = await testRule(rule, tree);
186+
const { tasks } = await testRule(rule, tree);
168187

169188
expect(tasks.map((task) => task.toConfiguration())).toEqual([
170189
{
@@ -182,7 +201,7 @@ describe('addDependency', () => {
182201
packageJsonPath: '/abc/package.json',
183202
});
184203

185-
const tasks = await testRule(rule, tree);
204+
const { tasks } = await testRule(rule, tree);
186205

187206
expect(tasks.map((task) => task.toConfiguration())).toEqual([
188207
{
@@ -201,7 +220,7 @@ describe('addDependency', () => {
201220
install: InstallBehavior.Auto,
202221
});
203222

204-
const tasks = await testRule(rule, tree);
223+
const { tasks } = await testRule(rule, tree);
205224

206225
expect(tasks.map((task) => task.toConfiguration())).toEqual([
207226
{
@@ -220,7 +239,7 @@ describe('addDependency', () => {
220239
install: InstallBehavior.Always,
221240
});
222241

223-
const tasks = await testRule(rule, tree);
242+
const { tasks } = await testRule(rule, tree);
224243

225244
expect(tasks.map((task) => task.toConfiguration())).toEqual([
226245
{
@@ -239,7 +258,7 @@ describe('addDependency', () => {
239258
install: InstallBehavior.None,
240259
});
241260

242-
const tasks = await testRule(rule, tree);
261+
const { tasks } = await testRule(rule, tree);
243262

244263
expect(tasks).toEqual([]);
245264
});
@@ -255,7 +274,7 @@ describe('addDependency', () => {
255274

256275
const rule = addDependency('@angular/core', '^14.0.0');
257276

258-
const tasks = await testRule(rule, tree);
277+
const { tasks } = await testRule(rule, tree);
259278

260279
expect(tasks).toEqual([]);
261280
});
@@ -269,7 +288,7 @@ describe('addDependency', () => {
269288
addDependency('@angular/common', '^14.0.0'),
270289
]);
271290

272-
const tasks = await testRule(rule, tree);
291+
const { tasks } = await testRule(rule, tree);
273292

274293
expect(tasks.map((task) => task.toConfiguration())).toEqual([
275294
{
@@ -288,7 +307,7 @@ describe('addDependency', () => {
288307
addDependency('@angular/common', '^14.0.0', { install: InstallBehavior.Auto }),
289308
]);
290309

291-
const tasks = await testRule(rule, tree);
310+
const { tasks } = await testRule(rule, tree);
292311

293312
expect(tasks.map((task) => task.toConfiguration())).toEqual([
294313
{
@@ -307,7 +326,7 @@ describe('addDependency', () => {
307326
addDependency('@angular/common', '^14.0.0', { install: InstallBehavior.None }),
308327
]);
309328

310-
const tasks = await testRule(rule, tree);
329+
const { tasks } = await testRule(rule, tree);
311330

312331
expect(tasks.map((task) => task.toConfiguration())).toEqual([
313332
{
@@ -326,7 +345,7 @@ describe('addDependency', () => {
326345
addDependency('@angular/common', '^14.0.0', { install: InstallBehavior.Auto }),
327346
]);
328347

329-
const tasks = await testRule(rule, tree);
348+
const { tasks } = await testRule(rule, tree);
330349

331350
expect(tasks.map((task) => task.toConfiguration())).toEqual([
332351
{
@@ -345,7 +364,7 @@ describe('addDependency', () => {
345364
addDependency('@angular/common', '^14.0.0', { install: InstallBehavior.Always }),
346365
]);
347366

348-
const tasks = await testRule(rule, tree);
367+
const { tasks } = await testRule(rule, tree);
349368

350369
expect(tasks.map((task) => task.toConfiguration())).toEqual([
351370
{
@@ -369,7 +388,7 @@ describe('addDependency', () => {
369388
addDependency('@angular/common', '^14.0.0', { packageJsonPath: '/abc/package.json' }),
370389
]);
371390

372-
const tasks = await testRule(rule, tree);
391+
const { tasks } = await testRule(rule, tree);
373392

374393
expect(tasks.map((task) => task.toConfiguration())).toEqual([
375394
{

0 commit comments

Comments
 (0)