Skip to content

Commit 5ba43e1

Browse files
iclantoniclanton
committed
Clean up some issues with Node.js version comatability warnings.
1 parent b0cfe9a commit 5ba43e1

4 files changed

Lines changed: 86 additions & 32 deletions

File tree

‎apps/rush-lib/src/api/Rush.ts‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
import { EOL } from 'os';
55
import * as colors from 'colors';
6-
import * as semver from 'semver';
76
import { PackageJsonLookup } from '@microsoft/node-core-library';
87

98
import { RushCommandLineParser } from '../cli/RushCommandLineParser';
@@ -56,17 +55,15 @@ export class Rush {
5655

5756
Rush._printStartupBanner(options.isManaged);
5857

59-
if (!options.alreadyReportedNodeTooNewError) {
60-
NodeJsCompatibility.warnAboutVersionTooNew(true);
61-
}
62-
6358
if (!CommandLineMigrationAdvisor.checkArgv(process.argv)) {
6459
// The migration advisor recognized an obsolete command-line
6560
process.exitCode = 1;
6661
return;
6762
}
6863

69-
const parser: RushCommandLineParser = new RushCommandLineParser();
64+
const parser: RushCommandLineParser = new RushCommandLineParser({
65+
alreadyReportedNodeTooNewError: arg.alreadyReportedNodeTooNewError
66+
});
7067
parser.execute().catch(console.error); // CommandLineParser.execute() should never reject the promise
7168
}
7269

@@ -82,11 +79,13 @@ export class Rush {
8279

8380
Rush._printStartupBanner(options.isManaged);
8481

85-
if (!options.alreadyReportedNodeTooNewError) {
86-
NodeJsCompatibility.warnAboutVersionTooNew(true);
87-
}
88-
89-
RushXCommandLine.launchRushX(launcherVersion, options.isManaged);
82+
RushXCommandLine._launchRushXInternal(
83+
launcherVersion,
84+
{
85+
isManaged: options.isManaged,
86+
alreadyReportedNodeTooNewError: options.alreadyReportedNodeTooNewError
87+
}
88+
);
9089
}
9190

9291
/**
@@ -108,10 +107,9 @@ export class Rush {
108107

109108
private static _printStartupBanner(isManaged: boolean): void {
110109
const nodeVersion: string = process.versions.node;
111-
const nodeMajorVersion: number = semver.major(nodeVersion);
112-
const nodeReleaseLabel: string = (nodeMajorVersion % 2 === 0)
113-
? (NodeJsCompatibility.isLtsVersion ? 'LTS' : 'pre-LTS')
114-
: 'unstable';
110+
const nodeReleaseLabel: string = (NodeJsCompatibility.isOddNumberedVersion)
111+
? 'unstable'
112+
: (NodeJsCompatibility.isLtsVersion ? 'LTS' : 'pre-LTS');
115113

116114
console.log(
117115
EOL +

‎apps/rush-lib/src/cli/RushCommandLineParser.ts‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,12 @@ import { NodeJsCompatibility } from '../logic/NodeJsCompatibility';
4141
* Options for `RushCommandLineParser`.
4242
*/
4343
export interface IRushCommandLineParserOptions {
44-
cwd?: string; // Defaults to `cwd`
44+
cwd: string; // Defaults to `cwd`
45+
46+
/**
47+
* @internal
48+
*/
49+
alreadyReportedNodeTooNewError: boolean;
4550
}
4651

4752
export class RushCommandLineParser extends CommandLineParser {
@@ -52,7 +57,7 @@ export class RushCommandLineParser extends CommandLineParser {
5257
private _debugParameter: CommandLineFlagParameter;
5358
private _rushOptions: IRushCommandLineParserOptions;
5459

55-
constructor(options?: IRushCommandLineParserOptions) {
60+
constructor(options?: Partial<IRushCommandLineParserOptions>) {
5661
super({
5762
toolFilename: 'rush',
5863
toolDescription: 'Rush makes life easier for JavaScript developers who develop, build, and publish'
@@ -80,6 +85,11 @@ export class RushCommandLineParser extends CommandLineParser {
8085
}
8186

8287
this._populateActions();
88+
89+
NodeJsCompatibility.warnAboutCompatibilityIssues(
90+
this._rushOptions.alreadyReportedNodeTooNewError,
91+
this.rushConfiguration
92+
);
8393
}
8494

8595
public get isDebug(): boolean {
@@ -112,8 +122,6 @@ export class RushCommandLineParser extends CommandLineParser {
112122
InternalError.breakInDebugger = true;
113123
}
114124

115-
NodeJsCompatibility.warnAboutNonLtsVersion(this.rushConfiguration);
116-
117125
return this._wrapOnExecute().catch((error: Error) => {
118126
this._reportErrorAndSetExitCode(error);
119127
}).then(() => {
@@ -123,8 +131,10 @@ export class RushCommandLineParser extends CommandLineParser {
123131
}
124132

125133
private _normalizeOptions(options: Partial<IRushCommandLineParserOptions>): IRushCommandLineParserOptions {
126-
const cwd: string = options.cwd || process.cwd();
127-
return { cwd };
134+
return {
135+
cwd: options.cwd || process.cwd(),
136+
alreadyReportedNodeTooNewError: options.alreadyReportedNodeTooNewError || false
137+
};
128138
}
129139

130140
private _wrapOnExecute(): Promise<void> {

‎apps/rush-lib/src/cli/RushXCommandLine.ts‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,37 @@ import { ProjectCommandSet } from '../logic/ProjectCommandSet';
1515
import { RushConfiguration } from '../api/RushConfiguration';
1616
import { NodeJsCompatibility } from '../logic/NodeJsCompatibility';
1717

18+
/**
19+
* @internal
20+
*/
21+
export interface ILaunchRushXInternalOptions {
22+
isManaged: boolean;
23+
alreadyReportedNodeTooNewError?: boolean;
24+
}
25+
1826
export class RushXCommandLine {
1927
public static launchRushX(launcherVersion: string, isManaged: boolean): void {
28+
RushXCommandLine._launchRushXInternal(launcherVersion, { isManaged });
29+
}
30+
31+
/**
32+
* @internal
33+
*/
34+
public static _launchRushXInternal(launcherVersion: string, options: ILaunchRushXInternalOptions): void {
2035
// Node.js can sometimes accidentally terminate with a zero exit code (e.g. for an uncaught
2136
// promise exception), so we start with the assumption that the exit code is 1
2237
// and set it to 0 only on success.
2338
process.exitCode = 1;
2439

2540
try {
41+
// Are we in a Rush repo?
42+
let rushConfiguration: RushConfiguration | undefined = undefined;
43+
if (RushConfiguration.tryFindRushJsonLocation()) {
44+
rushConfiguration = RushConfiguration.loadFromDefaultLocation({ showVerbose: true });
45+
}
46+
47+
NodeJsCompatibility.warnAboutCompatibilityIssues(!!options.alreadyReportedNodeTooNewError, rushConfiguration);
48+
2649
// Find the governing package.json for this folder:
2750
const packageJsonLookup: PackageJsonLookup = new PackageJsonLookup();
2851

@@ -68,14 +91,6 @@ export class RushXCommandLine {
6891
return;
6992
}
7093

71-
// Are we in a Rush repo?
72-
let rushConfiguration: RushConfiguration | undefined = undefined;
73-
if (RushConfiguration.tryFindRushJsonLocation()) {
74-
rushConfiguration = RushConfiguration.loadFromDefaultLocation({ showVerbose: true });
75-
}
76-
77-
NodeJsCompatibility.warnAboutNonLtsVersion(rushConfiguration);
78-
7994
console.log('Executing: ' + JSON.stringify(scriptBody) + os.EOL);
8095

8196
const packageFolder: string = path.dirname(packageJsonFilePath);

‎apps/rush-lib/src/logic/NodeJsCompatibility.ts‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,27 @@ import { RushConfiguration } from '../api/RushConfiguration';
1212
*/
1313
const UPCOMING_NODE_LTS_VERSION: number = 12;
1414
const nodeVersion: string = process.versions.node;
15+
const nodeMajorVersion: number = semver.major(nodeVersion);
1516

1617
/**
1718
* This class provides useful functions for warning if the current Node.js runtime isn't supported.
1819
*
1920
* @internal
2021
*/
2122
export class NodeJsCompatibility {
23+
public static warnAboutCompatibilityIssues(
24+
alreadyReportedNodeTooNewError: boolean,
25+
rushConfiguration: RushConfiguration | undefined
26+
): boolean {
27+
// Only show the first warning
28+
return (
29+
NodeJsCompatibility.warnAboutVersionTooOld() ||
30+
NodeJsCompatibility.warnAboutVersionTooNew(true, alreadyReportedNodeTooNewError) ||
31+
NodeJsCompatibility.warnAboutOddNumberedVersion() ||
32+
NodeJsCompatibility.warnAboutNonLtsVersion(rushConfiguration)
33+
);
34+
}
35+
2236
public static warnAboutVersionTooOld(): boolean {
2337
if (semver.satisfies(nodeVersion, '< 8.9.0')) {
2438
// We are on an ancient version of Node.js that is known not to work with Rush
@@ -33,12 +47,12 @@ export class NodeJsCompatibility {
3347
}
3448
}
3549

36-
public static warnAboutVersionTooNew(isRushLib: boolean): boolean {
37-
if (semver.satisfies(nodeVersion, `>= ${UPCOMING_NODE_LTS_VERSION + 1}.0.0`)) {
50+
public static warnAboutVersionTooNew(isRushLib: boolean, alreadyReportedNodeTooNewError: boolean): boolean {
51+
if (!alreadyReportedNodeTooNewError && nodeMajorVersion >= (UPCOMING_NODE_LTS_VERSION + 1)) {
3852
// We are on a much newer release than we have tested and support
3953
if (isRushLib) {
4054
console.warn(colors.yellow(
41-
`Your version of Node.js (${nodeVersion}) has not been tested with this release` +
55+
`Your version of Node.js (${nodeVersion}) has not been tested with this release ` +
4256
`of the Rush engine. Please consider upgrading the "rushVersion" setting in rush.json, ` +
4357
`or downgrading Node.js.`
4458
));
@@ -62,6 +76,19 @@ export class NodeJsCompatibility {
6276
!rushConfiguration.suppressNodeLtsWarning &&
6377
!NodeJsCompatibility.isLtsVersion
6478
) {
79+
console.warn(colors.yellow(
80+
`Your version of Node.js (${nodeVersion}) is not a Long-Term Support (LTS) release. ` +
81+
'These versions frequently have bugs. Please consider installing a stable release.'
82+
));
83+
84+
return true;
85+
} else {
86+
return false;
87+
}
88+
}
89+
90+
public static warnAboutOddNumberedVersion(): boolean {
91+
if (NodeJsCompatibility.isOddNumberedVersion) {
6592
console.warn(colors.yellow(
6693
`Your version of Node.js (${nodeVersion}) is an odd-numbered release. ` +
6794
`These releases frequently have bugs. Please consider installing a Long Term Support (LTS) ` +
@@ -83,4 +110,8 @@ export class NodeJsCompatibility {
83110

84111
return !!(process as IExtendedNodeProcess).release.lts;
85112
}
113+
114+
public static get isOddNumberedVersion(): boolean {
115+
return (nodeMajorVersion % 2) !== 0;
116+
}
86117
}

0 commit comments

Comments
 (0)