Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rush] Introduce a "rush add" command #843

Merged
merged 32 commits into from
Sep 28, 2018
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
0f4ae68
introduce the new command - incomplete
nickpape-msft Sep 21, 2018
7260861
API Change
nickpape-msft Sep 24, 2018
60f2fa9
Fill out most of the rest of the logic for the add command
nickpape-msft Sep 24, 2018
0ae6ccf
Fix lint issues
nickpape-msft Sep 24, 2018
a66230d
Some small change4s
nickpape-msft Sep 25, 2018
9c6f76f
Small changes
nickpape-msft Sep 25, 2018
30df582
Move add code into a seperate class
nickpape-msft Sep 26, 2018
caf6983
Fix bugs
nickpape-msft Sep 26, 2018
0a8a922
Add logging
nickpape-msft Sep 26, 2018
f094bb8
Merge branch 'master' into nickpape/rush-add
nickpape-msft Sep 26, 2018
8add64f
Fix wording
nickpape-msft Sep 26, 2018
8177fec
Merge branch 'master' into nickpape/rush-add
nickpape-msft Sep 26, 2018
bc43560
PR Feedback
nickpape-msft Sep 26, 2018
56a6ab2
Refactor
nickpape-msft Sep 27, 2018
ee299b1
Random comments
nickpape-msft Sep 28, 2018
1c8e54a
Some PR feedback
nickpape-msft Sep 28, 2018
abfabf2
PR Feedback
nickpape-msft Sep 28, 2018
9d66f66
More feedback
nickpape-msft Sep 28, 2018
14e448f
More feedback
nickpape-msft Sep 28, 2018
6e62fdc
Snapshot
nickpape-msft Sep 28, 2018
836c9a2
Updating strings
pgonzal Sep 28, 2018
2909608
Merge branch 'nickpape/rush-add' of https://github.com/Microsoft/web-…
pgonzal Sep 28, 2018
e0793d0
Add change log
pgonzal Sep 28, 2018
ddf3e0b
Some fixes
nickpape-msft Sep 28, 2018
d48c7eb
Fix small bug
nickpape-msft Sep 28, 2018
2d81c46
Merge branch 'nickpape/rush-add' of https://github.com/Microsoft/web-…
nickpape-msft Sep 28, 2018
128da2f
Minor changes
nickpape-msft Sep 28, 2018
daf87a2
more bugs
nickpape-msft Sep 28, 2018
91b5592
small bug
nickpape-msft Sep 28, 2018
a115ad5
remove debugging:
nickpape-msft Sep 28, 2018
2ac1044
Missing changefile
nickpape-msft Sep 28, 2018
612e9bf
Merge branch 'master' into nickpape/rush-add
nickpape-msft Sep 28, 2018
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
10 changes: 10 additions & 0 deletions apps/rush-lib/src/api/RushConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,16 @@ export class RushConfiguration {
return this._versionPolicyConfiguration;
}

public getCurrentProjectFromPath(currentFolderPath: string): RushConfigurationProject | undefined {
Copy link
Member

@iclanton iclanton Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getCurrentProjectFromPath [](start = 9, length = 25)

getProjectFromPath? It doesn't necessarily need to be the current project or the current path. #Resolved

const resolvedPath: string = path.resolve(currentFolderPath);
for (const project of this.projects) {
if (path.relative(project.projectFolder, resolvedPath).indexOf('..') !== 0) {
Copy link
Member

@iclanton iclanton Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path.relative(project.projectFolder, resolvedPath).indexOf('..') !== 0 [](start = 10, length = 70)

use Path.isUnder from node-core-library. #Resolved

Copy link
Collaborator

@octogonz octogonz Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path.relative [](start = 10, length = 13)

Use Path.isUnder #Resolved

return project;
}
}
return undefined;
}

/**
* Use RushConfiguration.loadFromConfigurationFile() or Use RushConfiguration.loadFromDefaultLocation()
* instead.
Expand Down
45 changes: 25 additions & 20 deletions apps/rush-lib/src/api/VersionMismatchFinder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,31 +30,36 @@ export class VersionMismatchFinder {
VersionMismatchFinder._checkForInconsistentVersions(rushConfiguration, false);
}

private static _checkForInconsistentVersions(
rushConfiguration: RushConfiguration,
isRushCheckCommand: boolean): void {
public static getMismatches(rushConfiguration: RushConfiguration): VersionMismatchFinder {
Copy link
Collaborator

@octogonz octogonz Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getMismatches [](start = 16, length = 13)

Docs? #Resolved

// Collect all the preferred versions into a single table
const allPreferredVersions: { [dependency: string]: string } = {};

if (rushConfiguration.ensureConsistentVersions || isRushCheckCommand) {
// Collect all the preferred versions into a single table
const allPreferredVersions: { [dependency: string]: string } = {};
rushConfiguration.commonVersions.getAllPreferredVersions().forEach((version: string, dependency: string) => {
allPreferredVersions[dependency] = version;
});

rushConfiguration.commonVersions.getAllPreferredVersions().forEach((version: string, dependency: string) => {
allPreferredVersions[dependency] = version;
});
// Create a fake project for the purposes of reporting conflicts with preferredVersions
// or xstitchPreferredVersions from common-versions.json
const projects: RushConfigurationProject[] = [...rushConfiguration.projects];

// Create a fake project for the purposes of reporting conflicts with preferredVersions
// or xstitchPreferredVersions from common-versions.json
const projects: RushConfigurationProject[] = [...rushConfiguration.projects];
projects.push({
packageName: 'preferred versions from ' + RushConstants.commonVersionsFilename,
packageJson: { dependencies: allPreferredVersions }
} as RushConfigurationProject);

projects.push({
packageName: 'preferred versions from ' + RushConstants.commonVersionsFilename,
packageJson: { dependencies: allPreferredVersions }
} as RushConfigurationProject);
return new VersionMismatchFinder(
projects,
rushConfiguration.commonVersions.allowedAlternativeVersions
);
}

private static _checkForInconsistentVersions(
rushConfiguration: RushConfiguration,
isRushCheckCommand: boolean): void {

const mismatchFinder: VersionMismatchFinder = new VersionMismatchFinder(
projects,
rushConfiguration.commonVersions.allowedAlternativeVersions
);
if (rushConfiguration.ensureConsistentVersions || isRushCheckCommand) {
const mismatchFinder: VersionMismatchFinder
= VersionMismatchFinder.getMismatches(rushConfiguration);

// Iterate over the list. For any dependency with mismatching versions, print the projects
mismatchFinder.getMismatches().forEach((dependency: string) => {
Expand Down
2 changes: 2 additions & 0 deletions apps/rush-lib/src/cli/RushCommandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { CommandJson } from '../api/CommandLineJson';
import { Utilities } from '../utilities/Utilities';
import { BaseScriptAction } from '../cli/scriptActions/BaseScriptAction';

import { AddAction } from './actions/AddAction';
import { ChangeAction } from './actions/ChangeAction';
import { CheckAction } from './actions/CheckAction';
import { UpdateAction } from './actions/UpdateAction';
Expand Down Expand Up @@ -109,6 +110,7 @@ export class RushCommandLineParser extends CommandLineParser {
this.rushConfiguration = RushConfiguration.loadFromConfigurationFile(rushJsonFilename);
}

this.addAction(new AddAction(this));
this.addAction(new ChangeAction(this));
this.addAction(new CheckAction(this));
this.addAction(new InstallAction(this));
Expand Down
105 changes: 105 additions & 0 deletions apps/rush-lib/src/cli/actions/AddAction.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import * as os from 'os';

import {
CommandLineFlagParameter,
CommandLineStringParameter
} from '@microsoft/ts-command-line';

import { RushConfigurationProject } from '../../api/RushConfigurationProject';
import { BaseRushAction } from './BaseRushAction';
import { RushCommandLineParser } from '../RushCommandLineParser';
import { DependencyIntegrator, SemVerStyle } from '../../logic/DependencyIntegrator';

export class AddAction extends BaseRushAction {
private _exactFlag: CommandLineFlagParameter;
private _caretFlag: CommandLineFlagParameter;
private _devDependencyFlag: CommandLineFlagParameter;
private _makeConsistentFlag: CommandLineFlagParameter;
private _noInstallFlag: CommandLineFlagParameter;
private _packageName: CommandLineStringParameter;
private _versionSpecifier: CommandLineStringParameter;

constructor(parser: RushCommandLineParser) {
const documentation: string[] = [
'Blah.'
Copy link
Member

@iclanton iclanton Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blah [](start = 7, length = 4)

? #Resolved

];
super({
actionName: 'add',
summary: 'Adds a dependency to the package.json and runs rush upgrade.',
documentation: documentation.join(os.EOL),
safeForSimultaneousRushProcesses: false,
parser
});
}

public onDefineParameters(): void {
this._packageName = this.defineStringParameter({
parameterLongName: '--package',
parameterShortName: '-p',
required: true,
argumentName: 'PACKAGE_NAME',
description: '(Required) The name of the package which should be added as a dependency'
});
Copy link
Member

@iclanton iclanton Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is clunky. How hard would it be to add an "anonymous" argument to TS-command-line? #WontFix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pete and I discussed this, but it will be a pretty big change in ts-command-line. We decided to just go with the normal way for now, then potentially change it later.


In reply to: 220407271 [](ancestors = 220407271)

this._versionSpecifier = this.defineStringParameter({
parameterLongName: '--version',
parameterShortName: '-v',
argumentName: 'VERSION_RANGE',
description: ''
Copy link
Member

@iclanton iclanton Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'' [](start = 19, length = 2)

Include a description. #Resolved

});
Copy link
Member

@iclanton iclanton Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be specified as package@version? #WontFix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be, but requires extra parsing and whatnot. We will make it like that when we allow anonymous arguments.


In reply to: 220407345 [](ancestors = 220407345)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we already have parsing for that?


In reply to: 220743394 [](ancestors = 220743394,220407345)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that I am aware of...


In reply to: 220749976 [](ancestors = 220749976,220743394,220407345)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we just split by the "@" character?


In reply to: 220751648 [](ancestors = 220751648,220749976,220743394,220407345)

this._exactFlag = this.defineFlagParameter({
parameterLongName: '--exact',
description: 'If specified, the version specifier inserted into the'
+ ' package.json will be a locked, exact version.'
});
this._caretFlag = this.defineFlagParameter({
parameterLongName: '--caret',
description: 'If specified, the version specifier inserted into the'
+ ' package.json will be a prepended with a "caret" specifier ("^").'
});
this._devDependencyFlag = this.defineFlagParameter({
Copy link
Member

@iclanton iclanton Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can be just specified with the version. #ByDesign

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?


In reply to: 220407415 [](ancestors = 220407415)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can specify the version as "~1.2.3" or "^2.3.4"


In reply to: 220743430 [](ancestors = 220743430,220407415)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You highlighted devDependency flag.

And that isn't how --version works. It works how "npm install --save" works.


In reply to: 220750046 [](ancestors = 220750046,220743430,220407415)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So NPM has --carat and --tilde arguments? That seems really clunky.


In reply to: 220751772 [](ancestors = 220751772,220750046,220743430,220407415)

parameterLongName: '--dev',
description: 'If specified, the package will be added as a "devDependency"'
+ ' to the package.json'
});
this._makeConsistentFlag = this.defineFlagParameter({
parameterLongName: '--make-consistent',
parameterShortName: '-c',
Copy link
Member

@iclanton iclanton Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-c [](start = 27, length = 2)

-m? #Resolved

description: 'If specified, other packages with this dependency will have their package.json'
+ ' files updated to point at the specified depdendency'
});
this._noInstallFlag = this.defineFlagParameter({
parameterLongName: '--no-install',
parameterShortName: '-n',
Copy link
Member

@iclanton iclanton Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-n [](start = 27, length = 2)

Is this the same as npm? #WontFix

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wontfix?


In reply to: 220407520 [](ancestors = 220407520)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean. NPM doesn't have an option for --skip-install or --no-install or anything.


In reply to: 220750062 [](ancestors = 220750062,220407520)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I was asking.


In reply to: 220751842 [](ancestors = 220751842,220750062,220407520)

description: 'If specified, the "rush update" command will not be run after updating the'
+ ' package.json files.'
});
}

public run(): Promise<void> {
const project: RushConfigurationProject | undefined
= this.rushConfiguration.getCurrentProjectFromPath(process.cwd());

if (!project) {
return Promise.reject(new Error('Not currently in a project folder'));
Copy link
Collaborator

@octogonz octogonz Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not currently [](start = 39, length = 13)

The "rush add" command must be invoked under a project folder that is registered in rush.json #Resolved

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could relax this in the future


In reply to: 220754019 [](ancestors = 220754019)

}

if (this._caretFlag.value && this._exactFlag.value) {
return Promise.reject(new Error('Only one of --caret and --exact should be specified'));
}

return new DependencyIntegrator(this.rushConfiguration).run({
currentProject: project,
packageName: this._packageName.value!,
initialVersion: this._versionSpecifier.value,
devDependency: this._devDependencyFlag.value,
updateOtherPackages: this._makeConsistentFlag.value,
skipInstall: this._noInstallFlag.value,
debugInstall: this.parser.isDebug,
rangeStyle: this._caretFlag.value ? SemVerStyle.Caret
: (this._exactFlag.value ? SemVerStyle.Exact : SemVerStyle.Tilde)
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ proven turnkey solution for monorepo management, Rush is for you.

Positional arguments:
<command>
add Adds a dependency to the package.json and runs rush upgrade.
change Records changes made to projects, indicating how the
package version number should be bumped for the next
publish.
Expand Down Expand Up @@ -51,6 +52,35 @@ For detailed help about a specific command, use: rush <command> -h
"
`;

exports[`CommandLineHelp prints the help for "rush add" 1`] = `
"usage: rush add [-h] -p PACKAGE_NAME [-v VERSION_RANGE] [--exact] [--caret]
[--dev] [-c] [-n]


Blah.
Copy link
Collaborator

@octogonz octogonz Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

? #Resolved

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Document that it's a tilde by default.


In reply to: 220731693 [](ancestors = 220731693)


Optional arguments:
-h, --help Show this help message and exit.
-p PACKAGE_NAME, --package PACKAGE_NAME
(Required) The name of the package which should be
added as a dependency
-v VERSION_RANGE, --version VERSION_RANGE
--exact If specified, the version specifier inserted into the
Copy link
Collaborator

@octogonz octogonz Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inserted into [](start = 59, length = 14)

"SemVer specifier added to" #Resolved

package.json will be a locked, exact version.
Copy link
Collaborator

@octogonz octogonz Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a locked, exact version. [](start = 44, length = 25)

an exact version (e.g. without tilde or caret) #Resolved

--caret If specified, the version specifier inserted into the
package.json will be a prepended with a \\"caret\\"
specifier (\\"^\\").
--dev If specified, the package will be added as a
\\"devDependency\\" to the package.json
Copy link
Collaborator

@octogonz octogonz Sep 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\"devDependency\" to the package.json [](start = 23, length = 40)

to the package.json "devDependencies" section #Resolved

-c, --make-consistent
If specified, other packages with this dependency
will have their package.json files updated to point
at the specified depdendency
Copy link
Collaborator

@octogonz octogonz Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

specify the same SemVer range? #Resolved

-n, --no-install If specified, the \\"rush update\\" command will not be
Copy link
Collaborator

@octogonz octogonz Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no-install [](start = 8, length = 10)

--skip-update? #Resolved

run after updating the package.json files.
"
`;

exports[`CommandLineHelp prints the help for "rush build" 1`] = `
"usage: rush build [-h] [-p COUNT] [-t PROJECT1]
[--to-version-policy VERSION_POLICY_NAME] [-f PROJECT2] [-v]
Expand Down
Loading