Skip to content

Commit

Permalink
refactor(ng-update): run package manager upon migration completion (#…
Browse files Browse the repository at this point in the history
…17455)

We cannot run the package-manager to refresh the lock file
synchronously. This is because the changes made to the
schematic tree are not applied until the actual task
completes. Hence we need to run the task asynchronously
upon migration completion. This ensures that the lock file
is properly updated if something changed the `package.json` file.
  • Loading branch information
devversion authored and jelbourn committed Oct 21, 2019
1 parent 2f197cd commit 1ab4b77
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 8 deletions.
20 changes: 16 additions & 4 deletions src/cdk/schematics/ng-update/upgrade-rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import {Rule, SchematicContext, Tree} from '@angular-devkit/schematics';
import {NodePackageInstallTask} from '@angular-devkit/schematics/tasks';

import {MigrationRuleType, runMigrationRules} from '../../update-tool';
import {TargetVersion} from '../../update-tool/target-version';
Expand Down Expand Up @@ -79,12 +80,23 @@ export function createUpgradeRule(
buildPaths.forEach(p => runMigration(p, false));
testPaths.forEach(p => runMigration(p, true));

let runPackageManager = false;

// Run the global post migration static members for all migration rules.
rules.forEach(r => r.globalPostMigration(tree, context));
rules.forEach(rule => {
const actionResult = rule.globalPostMigration(tree, context);
if (actionResult) {
runPackageManager = runPackageManager || actionResult.runPackageManager;
}
});

// Execute all asynchronous tasks and await them here. We want to run
// the "onMigrationCompleteFn" after all work is done.
await context.engine.executePostTasks().toPromise();
// If a rule requested the package manager to run, we run it as an
// asynchronous post migration task. We cannot run it synchronously,
// as file changes from the current migration task are not applied to
// the file system yet.
if (runPackageManager) {
context.addTask(new NodePackageInstallTask({quiet: false}));
}

if (onMigrationCompleteFn) {
onMigrationCompleteFn(targetVersion, hasRuleFailures);
Expand Down
7 changes: 6 additions & 1 deletion src/cdk/schematics/update-tool/migration-rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ export interface MigrationFailure {
position?: LineAndCharacter;
}

export type PostMigrationAction = void | {
/** Whether the package manager should run upon migration completion. */
runPackageManager: boolean;
};

export class MigrationRule<T> {
/** List of migration failures that need to be reported. */
failures: MigrationFailure[] = [];
Expand Down Expand Up @@ -92,5 +97,5 @@ export class MigrationRule<T> {
* migration result of all individual targets. e.g. removing HammerJS if it
* is not needed in any project target.
*/
static globalPostMigration(tree: Tree, context: SchematicContext) {}
static globalPostMigration(tree: Tree, context: SchematicContext): PostMigrationAction {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ import {
Path as DevkitPath
} from '@angular-devkit/core';
import {SchematicContext, SchematicsException, Tree} from '@angular-devkit/schematics';
import {NodePackageInstallTask} from '@angular-devkit/schematics/tasks';
import {
getProjectIndexFiles,
getProjectMainFile,
MigrationFailure,
MigrationRule,
PostMigrationAction,
ResolvedResource,
TargetVersion
} from '@angular/cdk/schematics';
Expand Down Expand Up @@ -762,11 +762,11 @@ export class HammerGesturesRule extends MigrationRule<null> {
* on the analysis of the individual targets. For example: we only remove Hammer
* from the "package.json" if it is not used in *any* project target.
*/
static globalPostMigration(tree: Tree, context: SchematicContext) {
static globalPostMigration(tree: Tree, context: SchematicContext): PostMigrationAction {
if (!this.globalUsesHammer && this._removeHammerFromPackageJson(tree)) {
// Since Hammer has been removed from the workspace "package.json" file,
// we schedule a node package install task to refresh the lock file.
context.addTask(new NodePackageInstallTask({quiet: false}));
return {runPackageManager: true};
}

context.logger.info(chalk.yellow(
Expand Down

0 comments on commit 1ab4b77

Please sign in to comment.