Skip to content

Commit 02e1a32

Browse files
sheetalkamatsheetalkamat
authored
Merge pull request #26742 from Microsoft/addShortWatch
Fixes issue when --build --watch queue becomes incorrect because it isnt reset correctly
2 parents 838110a + 90abaa1 commit 02e1a32

File tree

5 files changed

+271
-144
lines changed

5 files changed

+271
-144
lines changed

‎src/compiler/commandLineParser.ts‎

Lines changed: 106 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,7 @@ namespace ts {
6262
/* @internal */
6363
export const libMap = createMapFromEntries(libEntries);
6464

65-
/* @internal */
66-
export const optionDeclarations: CommandLineOption[] = [
67-
// CommandLine only options
65+
const commonOptionsWithBuild: CommandLineOption[] = [
6866
{
6967
name: "help",
7068
shortName: "h",
@@ -78,6 +76,27 @@ namespace ts {
7876
shortName: "?",
7977
type: "boolean"
8078
},
79+
{
80+
name: "preserveWatchOutput",
81+
type: "boolean",
82+
showInSimplifiedHelpView: false,
83+
category: Diagnostics.Command_line_Options,
84+
description: Diagnostics.Whether_to_keep_outdated_console_output_in_watch_mode_instead_of_clearing_the_screen,
85+
},
86+
{
87+
name: "watch",
88+
shortName: "w",
89+
type: "boolean",
90+
showInSimplifiedHelpView: true,
91+
category: Diagnostics.Command_line_Options,
92+
description: Diagnostics.Watch_input_files,
93+
},
94+
];
95+
96+
/* @internal */
97+
export const optionDeclarations: CommandLineOption[] = [
98+
// CommandLine only options
99+
...commonOptionsWithBuild,
81100
{
82101
name: "all",
83102
type: "boolean",
@@ -125,21 +144,6 @@ namespace ts {
125144
category: Diagnostics.Command_line_Options,
126145
description: Diagnostics.Stylize_errors_and_messages_using_color_and_context_experimental
127146
},
128-
{
129-
name: "preserveWatchOutput",
130-
type: "boolean",
131-
showInSimplifiedHelpView: false,
132-
category: Diagnostics.Command_line_Options,
133-
description: Diagnostics.Whether_to_keep_outdated_console_output_in_watch_mode_instead_of_clearing_the_screen,
134-
},
135-
{
136-
name: "watch",
137-
shortName: "w",
138-
type: "boolean",
139-
showInSimplifiedHelpView: true,
140-
category: Diagnostics.Command_line_Options,
141-
description: Diagnostics.Watch_input_files,
142-
},
143147

144148
// Basic
145149
{
@@ -754,6 +758,38 @@ namespace ts {
754758
}
755759
];
756760

761+
/* @internal */
762+
export const buildOpts: CommandLineOption[] = [
763+
...commonOptionsWithBuild,
764+
{
765+
name: "verbose",
766+
shortName: "v",
767+
category: Diagnostics.Command_line_Options,
768+
description: Diagnostics.Enable_verbose_logging,
769+
type: "boolean"
770+
},
771+
{
772+
name: "dry",
773+
shortName: "d",
774+
category: Diagnostics.Command_line_Options,
775+
description: Diagnostics.Show_what_would_be_built_or_deleted_if_specified_with_clean,
776+
type: "boolean"
777+
},
778+
{
779+
name: "force",
780+
shortName: "f",
781+
category: Diagnostics.Command_line_Options,
782+
description: Diagnostics.Build_all_projects_including_those_that_appear_to_be_up_to_date,
783+
type: "boolean"
784+
},
785+
{
786+
name: "clean",
787+
category: Diagnostics.Command_line_Options,
788+
description: Diagnostics.Delete_the_outputs_of_all_projects,
789+
type: "boolean"
790+
}
791+
];
792+
757793
/* @internal */
758794
export const typeAcquisitionDeclarations: CommandLineOption[] = [
759795
{
@@ -997,6 +1033,58 @@ namespace ts {
9971033
return optionNameMap.get(optionName);
9981034
}
9991035

1036+
/*@internal*/
1037+
export interface ParsedBuildCommand {
1038+
buildOptions: BuildOptions;
1039+
projects: string[];
1040+
errors: ReadonlyArray<Diagnostic>;
1041+
}
1042+
1043+
/*@internal*/
1044+
export function parseBuildCommand(args: string[]): ParsedBuildCommand {
1045+
let buildOptionNameMap: OptionNameMap | undefined;
1046+
const returnBuildOptionNameMap = () => (buildOptionNameMap || (buildOptionNameMap = createOptionNameMap(buildOpts)));
1047+
1048+
const buildOptions: BuildOptions = {};
1049+
const projects: string[] = [];
1050+
let errors: Diagnostic[] | undefined;
1051+
for (const arg of args) {
1052+
if (arg.charCodeAt(0) === CharacterCodes.minus) {
1053+
const opt = getOptionDeclarationFromName(returnBuildOptionNameMap, arg.slice(arg.charCodeAt(1) === CharacterCodes.minus ? 2 : 1), /*allowShort*/ true);
1054+
if (opt) {
1055+
buildOptions[opt.name as keyof BuildOptions] = true;
1056+
}
1057+
else {
1058+
(errors || (errors = [])).push(createCompilerDiagnostic(Diagnostics.Unknown_build_option_0, arg));
1059+
}
1060+
}
1061+
else {
1062+
// Not a flag, parse as filename
1063+
projects.push(arg);
1064+
}
1065+
}
1066+
1067+
if (projects.length === 0) {
1068+
// tsc -b invoked with no extra arguments; act as if invoked with "tsc -b ."
1069+
projects.push(".");
1070+
}
1071+
1072+
// Nonsensical combinations
1073+
if (buildOptions.clean && buildOptions.force) {
1074+
(errors || (errors = [])).push(createCompilerDiagnostic(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "force"));
1075+
}
1076+
if (buildOptions.clean && buildOptions.verbose) {
1077+
(errors || (errors = [])).push(createCompilerDiagnostic(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "verbose"));
1078+
}
1079+
if (buildOptions.clean && buildOptions.watch) {
1080+
(errors || (errors = [])).push(createCompilerDiagnostic(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "watch"));
1081+
}
1082+
if (buildOptions.watch && buildOptions.dry) {
1083+
(errors || (errors = [])).push(createCompilerDiagnostic(Diagnostics.Options_0_and_1_cannot_be_combined, "watch", "dry"));
1084+
}
1085+
1086+
return { buildOptions, projects, errors: errors || emptyArray };
1087+
}
10001088

10011089
function getDiagnosticText(_message: DiagnosticMessage, ..._args: any[]): string {
10021090
const diagnostic = createCompilerDiagnostic.apply(undefined, arguments);

‎src/compiler/tsbuild.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,7 @@ namespace ts {
429429
projectPendingBuild.removeKey(proj);
430430
if (!projectPendingBuild.getSize()) {
431431
invalidatedProjectQueue.length = 0;
432+
nextIndex = 0;
432433
}
433434
return proj;
434435
}

‎src/testRunner/unittests/commandLineParsing.ts‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,4 +366,120 @@ namespace ts {
366366
});
367367
});
368368
});
369+
370+
describe("parseBuildOptions", () => {
371+
function assertParseResult(commandLine: string[], expectedParsedBuildCommand: ParsedBuildCommand) {
372+
const parsed = parseBuildCommand(commandLine);
373+
const parsedBuildOptions = JSON.stringify(parsed.buildOptions);
374+
const expectedBuildOptions = JSON.stringify(expectedParsedBuildCommand.buildOptions);
375+
assert.equal(parsedBuildOptions, expectedBuildOptions);
376+
377+
const parsedErrors = parsed.errors;
378+
const expectedErrors = expectedParsedBuildCommand.errors;
379+
assert.isTrue(parsedErrors.length === expectedErrors.length, `Expected error: ${JSON.stringify(expectedErrors)}. Actual error: ${JSON.stringify(parsedErrors)}.`);
380+
for (let i = 0; i < parsedErrors.length; i++) {
381+
const parsedError = parsedErrors[i];
382+
const expectedError = expectedErrors[i];
383+
assert.equal(parsedError.code, expectedError.code);
384+
assert.equal(parsedError.category, expectedError.category);
385+
assert.equal(parsedError.messageText, expectedError.messageText);
386+
}
387+
388+
const parsedProjects = parsed.projects;
389+
const expectedProjects = expectedParsedBuildCommand.projects;
390+
assert.deepEqual(parsedProjects, expectedProjects, `Expected projects: [${JSON.stringify(expectedProjects)}]. Actual projects: [${JSON.stringify(parsedProjects)}].`);
391+
}
392+
it("parse build without any options ", () => {
393+
// --lib es6 0.ts
394+
assertParseResult([],
395+
{
396+
errors: [],
397+
projects: ["."],
398+
buildOptions: {}
399+
});
400+
});
401+
402+
it("Parse multiple options", () => {
403+
// --lib es5,es2015.symbol.wellknown 0.ts
404+
assertParseResult(["--verbose", "--force", "tests"],
405+
{
406+
errors: [],
407+
projects: ["tests"],
408+
buildOptions: { verbose: true, force: true }
409+
});
410+
});
411+
412+
it("Parse option with invalid option ", () => {
413+
// --lib es5,invalidOption 0.ts
414+
assertParseResult(["--verbose", "--invalidOption"],
415+
{
416+
errors: [{
417+
messageText: "Unknown build option '--invalidOption'.",
418+
category: Diagnostics.Unknown_build_option_0.category,
419+
code: Diagnostics.Unknown_build_option_0.code,
420+
file: undefined,
421+
start: undefined,
422+
length: undefined,
423+
}],
424+
projects: ["."],
425+
buildOptions: { verbose: true }
426+
});
427+
});
428+
429+
it("Parse multiple flags with input projects at the end", () => {
430+
// --lib es5,es2015.symbol.wellknown --target es5 0.ts
431+
assertParseResult(["--force", "--verbose", "src", "tests"],
432+
{
433+
errors: [],
434+
projects: ["src", "tests"],
435+
buildOptions: { force: true, verbose: true }
436+
});
437+
});
438+
439+
it("Parse multiple flags with input projects in the middle", () => {
440+
// --module commonjs --target es5 0.ts --lib es5,es2015.symbol.wellknown
441+
assertParseResult(["--force", "src", "tests", "--verbose"],
442+
{
443+
errors: [],
444+
projects: ["src", "tests"],
445+
buildOptions: { force: true, verbose: true }
446+
});
447+
});
448+
449+
it("Parse multiple flags with input projects in the beginning", () => {
450+
// --module commonjs --target es5 0.ts --lib es5,es2015.symbol.wellknown
451+
assertParseResult(["src", "tests", "--force", "--verbose"],
452+
{
453+
errors: [],
454+
projects: ["src", "tests"],
455+
buildOptions: { force: true, verbose: true }
456+
});
457+
});
458+
459+
describe("Combining options that make no sense together", () => {
460+
function verifyInvalidCombination(flag1: keyof BuildOptions, flag2: keyof BuildOptions) {
461+
it(`--${flag1} and --${flag2} together is invalid`, () => {
462+
// --module commonjs --target es5 0.ts --lib es5,es2015.symbol.wellknown
463+
assertParseResult([`--${flag1}`, `--${flag2}`],
464+
{
465+
errors: [{
466+
messageText: `Options '${flag1}' and '${flag2}' cannot be combined.`,
467+
category: Diagnostics.Options_0_and_1_cannot_be_combined.category,
468+
code: Diagnostics.Options_0_and_1_cannot_be_combined.code,
469+
file: undefined,
470+
start: undefined,
471+
length: undefined,
472+
}],
473+
projects: ["."],
474+
buildOptions: { [flag1]: true, [flag2]: true }
475+
});
476+
});
477+
}
478+
479+
verifyInvalidCombination("clean", "force");
480+
verifyInvalidCombination("clean", "verbose");
481+
verifyInvalidCombination("clean", "watch");
482+
verifyInvalidCombination("watch", "dry");
483+
});
484+
});
369485
}

‎src/testRunner/unittests/tsbuildWatchMode.ts‎

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -98,34 +98,42 @@ namespace ts.tscWatch {
9898
for (const stamp of outputFileStamps) {
9999
assert.isDefined(stamp[1], `${stamp[0]} expected to be present`);
100100
}
101-
return { host, outputFileStamps };
101+
return host;
102102
}
103103
it("creates solution in watch mode", () => {
104104
createSolutionInWatchMode();
105105
});
106106

107107
it("change builds changes and reports found errors message", () => {
108-
const { host, outputFileStamps } = createSolutionInWatchMode();
109-
host.writeFile(core[1].path, `${core[1].content}
108+
const host = createSolutionInWatchMode();
109+
verifyChange(`${core[1].content}
110110
export class someClass { }`);
111-
host.checkTimeoutQueueLengthAndRun(1); // Builds core
112-
const changedCore = getOutputFileStamps(host);
113-
verifyChangedFiles(changedCore, outputFileStamps, [
114-
...getOutputFileNames(SubProject.core, "anotherModule"), // This should not be written really
115-
...getOutputFileNames(SubProject.core, "index")
116-
]);
117-
host.checkTimeoutQueueLengthAndRun(1); // Builds tests
118-
const changedTests = getOutputFileStamps(host);
119-
verifyChangedFiles(changedTests, changedCore, [
120-
...getOutputFileNames(SubProject.tests, "index") // Again these need not be written
121-
]);
122-
host.checkTimeoutQueueLengthAndRun(1); // Builds logic
123-
const changedLogic = getOutputFileStamps(host);
124-
verifyChangedFiles(changedLogic, changedTests, [
125-
...getOutputFileNames(SubProject.logic, "index") // Again these need not be written
126-
]);
127-
host.checkTimeoutQueueLength(0);
128-
checkOutputErrorsIncremental(host, emptyArray);
111+
112+
// Another change requeues and builds it
113+
verifyChange(core[1].content);
114+
115+
function verifyChange(coreContent: string) {
116+
const outputFileStamps = getOutputFileStamps(host);
117+
host.writeFile(core[1].path, coreContent);
118+
host.checkTimeoutQueueLengthAndRun(1); // Builds core
119+
const changedCore = getOutputFileStamps(host);
120+
verifyChangedFiles(changedCore, outputFileStamps, [
121+
...getOutputFileNames(SubProject.core, "anotherModule"), // This should not be written really
122+
...getOutputFileNames(SubProject.core, "index")
123+
]);
124+
host.checkTimeoutQueueLengthAndRun(1); // Builds tests
125+
const changedTests = getOutputFileStamps(host);
126+
verifyChangedFiles(changedTests, changedCore, [
127+
...getOutputFileNames(SubProject.tests, "index") // Again these need not be written
128+
]);
129+
host.checkTimeoutQueueLengthAndRun(1); // Builds logic
130+
const changedLogic = getOutputFileStamps(host);
131+
verifyChangedFiles(changedLogic, changedTests, [
132+
...getOutputFileNames(SubProject.logic, "index") // Again these need not be written
133+
]);
134+
host.checkTimeoutQueueLength(0);
135+
checkOutputErrorsIncremental(host, emptyArray);
136+
}
129137
});
130138

131139
// TODO: write tests reporting errors but that will have more involved work since file

0 commit comments

Comments
 (0)