From fba22bc9d8bed95b97e58b77709e1170a38607f9 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 10 Jan 2020 20:09:45 +0100 Subject: [PATCH] fix(material): ng-add should always install matching CDK version (#18076) Currently ng-add for Angular Material determines the version name of the CDK and Angular Material by looking for `@angular/cdk/package.json` and `@angular/material/package.json` in the node modules. This is incorrect because other installed packages could rely on older CDK versions being installed. In those cases, we don't want to add that transitively installed CDK version to the `package.json` file. Instead, we always want to add the CDK version that matches Angular Material. We can do this by just relying on the version placeholder we use in other locations too. Also in the issue ticket (#18020) it was mentioned that we might want to warn if a lower CDK version is transitively installed. This seems unnecessary since we explicitly add a project-scoped dependency on the CDK and transitive CDK dependencies are nothing we should bother about. In general, if someone runs `ng add @angular/material` it's _only_ right to always install a working/matching version of the CDK. Fixes #18020 --- src/cdk/schematics/ng-add/index.spec.ts | 4 ++- src/cdk/schematics/ng-add/index.ts | 28 ++++++++----------- src/material/schematics/ng-add/index.spec.ts | 9 ++++-- src/material/schematics/ng-add/index.ts | 5 ++-- .../schematics/ng-add/version-names.ts | 13 +-------- 5 files changed, 24 insertions(+), 35 deletions(-) diff --git a/src/cdk/schematics/ng-add/index.spec.ts b/src/cdk/schematics/ng-add/index.spec.ts index 7b426389ce85..30e837f97a7b 100644 --- a/src/cdk/schematics/ng-add/index.spec.ts +++ b/src/cdk/schematics/ng-add/index.spec.ts @@ -16,10 +16,12 @@ describe('CDK ng-add', () => { const packageJson = JSON.parse(getFileContent(tree, '/package.json')); const dependencies = packageJson.dependencies; - expect(dependencies['@angular/cdk']).toBeDefined(); + expect(dependencies['@angular/cdk']).toBe('~0.0.0-PLACEHOLDER'); expect(Object.keys(dependencies)) .toEqual( Object.keys(dependencies).sort(), 'Expected the modified "dependencies" to be sorted alphabetically.'); + expect(runner.tasks.some(task => task.name === 'node-package')).toBe(true, + 'Expected the package manager to be scheduled in order to update lock files.'); }); }); diff --git a/src/cdk/schematics/ng-add/index.ts b/src/cdk/schematics/ng-add/index.ts index 5abdfaed9fd3..3165c6837ca7 100644 --- a/src/cdk/schematics/ng-add/index.ts +++ b/src/cdk/schematics/ng-add/index.ts @@ -6,12 +6,10 @@ * found in the LICENSE file at https://angular.io/license */ -import {Rule, Tree} from '@angular-devkit/schematics'; +import {Rule, SchematicContext, Tree} from '@angular-devkit/schematics'; +import {NodePackageInstallTask} from '@angular-devkit/schematics/tasks'; import {addPackageToPackageJson} from './package-config'; -/** Name of the Angular CDK version that is shipped together with the schematics. */ -export const cdkVersion = loadPackageVersionGracefully('@angular/cdk'); - /** * Schematic factory entry-point for the `ng-add` schematic. The ng-add schematic will be * automatically executed if developers run `ng add @angular/cdk`. @@ -21,18 +19,14 @@ export const cdkVersion = loadPackageVersionGracefully('@angular/cdk'); * this ensures that there will be no error that says that the CDK does not support `ng add`. */ export default function(): Rule { - return (host: Tree) => { - // In order to align the CDK version with the other Angular dependencies, we use tilde - // instead of caret. This is default for Angular dependencies in new CLI projects. - addPackageToPackageJson(host, '@angular/cdk', `~${cdkVersion}`); - }; -} + return (host: Tree, context: SchematicContext) => { + // In order to align the CDK version with other Angular dependencies that are setup + // by "@schematics/angular", we use tilde instead of caret. This is default for Angular + // dependencies in new CLI projects. + addPackageToPackageJson(host, '@angular/cdk', `~0.0.0-PLACEHOLDER`); -/** Loads the full version from the given Angular package gracefully. */ -function loadPackageVersionGracefully(packageName: string): string|null { - try { - return require(`${packageName}/package.json`).version; - } catch { - return null; - } + // Add a task to run the package manager. This is necessary because we updated the + // workspace "package.json" file and we want lock files to reflect the new version range. + context.addTask(new NodePackageInstallTask()); + }; } diff --git a/src/material/schematics/ng-add/index.spec.ts b/src/material/schematics/ng-add/index.spec.ts index aaafcf89be33..f54a62b1ea09 100644 --- a/src/material/schematics/ng-add/index.spec.ts +++ b/src/material/schematics/ng-add/index.spec.ts @@ -57,8 +57,8 @@ describe('ng-add schematic', () => { const dependencies = packageJson.dependencies; const angularCoreVersion = dependencies['@angular/core']; - expect(dependencies['@angular/material']).toBeDefined(); - expect(dependencies['@angular/cdk']).toBeDefined(); + expect(dependencies['@angular/material']).toBe('~0.0.0-PLACEHOLDER'); + expect(dependencies['@angular/cdk']).toBe('~0.0.0-PLACEHOLDER'); expect(dependencies['@angular/forms']) .toBe( angularCoreVersion, @@ -73,7 +73,10 @@ describe('ng-add schematic', () => { Object.keys(dependencies).sort(), 'Expected the modified "dependencies" to be sorted alphabetically.'); - expect(runner.tasks.some(task => task.name === 'run-schematic')).toBe(true); + expect(runner.tasks.some(task => task.name === 'node-package')).toBe(true, + 'Expected the package manager to be scheduled in order to update lock files.'); + expect(runner.tasks.some(task => task.name === 'run-schematic')).toBe(true, + 'Expected the setup-project schematic to be scheduled.'); }); it('should add default theme', async () => { diff --git a/src/material/schematics/ng-add/index.ts b/src/material/schematics/ng-add/index.ts index 8c033839fae4..076cb39841a9 100644 --- a/src/material/schematics/ng-add/index.ts +++ b/src/material/schematics/ng-add/index.ts @@ -27,8 +27,9 @@ export default function(options: Schema): Rule { const ngCoreVersionTag = getPackageVersionFromPackageJson(host, '@angular/core'); const angularDependencyVersion = ngCoreVersionTag || requiredAngularVersionRange; - // In order to align the Material and CDK version with the other Angular dependencies, - // we use tilde instead of caret. This is default for Angular dependencies in new CLI projects. + // In order to align the Material and CDK version with other Angular dependencies that + // are setup by "@schematics/angular", we use tilde instead of caret. This is default for + // Angular dependencies in new CLI projects. addPackageToPackageJson(host, '@angular/cdk', `~${materialVersion}`); addPackageToPackageJson(host, '@angular/material', `~${materialVersion}`); addPackageToPackageJson(host, '@angular/forms', angularDependencyVersion); diff --git a/src/material/schematics/ng-add/version-names.ts b/src/material/schematics/ng-add/version-names.ts index 4c34cf83c1f7..db02cc916ea0 100644 --- a/src/material/schematics/ng-add/version-names.ts +++ b/src/material/schematics/ng-add/version-names.ts @@ -7,21 +7,10 @@ */ /** Name of the Material version that is shipped together with the schematics. */ -export const materialVersion = - loadPackageVersionGracefully('@angular/cdk') || - loadPackageVersionGracefully('@angular/material'); +export const materialVersion = '0.0.0-PLACEHOLDER'; /** * Range of Angular versions that can be used together with the Angular Material version * that provides these schematics. */ export const requiredAngularVersionRange = '0.0.0-NG'; - -/** Loads the full version from the given Angular package gracefully. */ -function loadPackageVersionGracefully(packageName: string): string | null { - try { - return require(`${packageName}/package.json`).version; - } catch { - return null; - } -}