Skip to content

Commit bd4dc38

Browse files
alan-agius4vikerman
authored andcommitted
refactor: use createConsoleLogger and remove duplicate code (#12787)
1 parent 8d69589 commit bd4dc38

File tree

5 files changed

+37
-74
lines changed

5 files changed

+37
-74
lines changed

packages/angular/cli/lib/cli/index.ts

+2-64
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,14 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import { logging, terminal } from '@angular-devkit/core';
9+
import { createConsoleLogger } from '@angular-devkit/core/node';
1010
import { runCommand } from '../../models/command-runner';
1111
import { getWorkspaceRaw } from '../../utilities/config';
1212
import { getWorkspaceDetails } from '../../utilities/project';
1313

1414

1515
export default async function(options: { testing?: boolean, cliArgs: string[] }) {
16-
const logger = new logging.IndentLogger('cling');
17-
let loggingSubscription;
18-
if (!options.testing) {
19-
loggingSubscription = initializeLogging(logger);
20-
}
16+
const logger = createConsoleLogger();
2117

2218
let projectDetails = getWorkspaceDetails();
2319
if (projectDetails === null) {
@@ -60,64 +56,6 @@ export default async function(options: { testing?: boolean, cliArgs: string[] })
6056
throw err;
6157
}
6258

63-
if (loggingSubscription) {
64-
loggingSubscription.unsubscribe();
65-
}
66-
6759
return 1;
6860
}
6961
}
70-
71-
// Initialize logging.
72-
function initializeLogging(logger: logging.Logger) {
73-
return logger
74-
.subscribe(entry => {
75-
let color = (x: string) => terminal.dim(terminal.white(x));
76-
let output = process.stdout;
77-
switch (entry.level) {
78-
case 'debug':
79-
return;
80-
case 'info':
81-
color = terminal.white;
82-
break;
83-
case 'warn':
84-
color = (x: string) => terminal.bold(terminal.yellow(x));
85-
output = process.stderr;
86-
break;
87-
case 'fatal':
88-
case 'error':
89-
color = (x: string) => terminal.bold(terminal.red(x));
90-
output = process.stderr;
91-
break;
92-
}
93-
94-
95-
// If we do console.log(message) or process.stdout.write(message + '\n'), the process might
96-
// stop before the whole message is written and the stream is flushed. This happens when
97-
// streams are asynchronous.
98-
//
99-
// NodeJS IO streams are different depending on platform and usage. In POSIX environment,
100-
// for example, they're asynchronous when writing to a pipe, but synchronous when writing
101-
// to a TTY. In windows, it's the other way around. You can verify which is which with
102-
// stream.isTTY and platform, but this is not good enough.
103-
// In the async case, one should wait for the callback before sending more data or
104-
// continuing the process. In our case it would be rather hard to do (but not impossible).
105-
//
106-
// Instead we take the easy way out and simply chunk the message and call the write
107-
// function while the buffer drain itself asynchronously. With a smaller chunk size than
108-
// the buffer, we are mostly certain that it works. In this case, the chunk has been picked
109-
// as half a page size (4096/2 = 2048), minus some bytes for the color formatting.
110-
// On POSIX it seems the buffer is 2 pages (8192), but just to be sure (could be different
111-
// by platform).
112-
//
113-
// For more details, see https://nodejs.org/api/process.html#process_a_note_on_process_i_o
114-
const chunkSize = 2000; // Small chunk.
115-
let message = entry.message;
116-
while (message) {
117-
const chunk = message.slice(0, chunkSize);
118-
message = message.slice(chunkSize);
119-
output.write(color(chunk));
120-
}
121-
output.write('\n');
122-
});
123-
}

packages/angular/cli/models/architect-command.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,6 @@ export abstract class ArchitectCommand<
3131
private _host = new NodeJsSyncHost();
3232
protected _architect: Architect;
3333
protected _workspace: experimental.workspace.Workspace;
34-
protected _logger = createConsoleLogger();
35-
3634
protected _registry: json.schema.SchemaRegistry;
3735

3836
// If this command supports running multiple targets.
@@ -150,7 +148,7 @@ export abstract class ArchitectCommand<
150148
}
151149
const realBuilderConf = this._architect.getBuilderConfiguration({ ...targetSpec, overrides });
152150

153-
const result = await this._architect.run(realBuilderConf, { logger: this._logger }).toPromise();
151+
const result = await this._architect.run(realBuilderConf, { logger: this.logger }).toPromise();
154152

155153
return result.success ? 0 : 1;
156154
}

packages/angular_devkit/core/node/cli-logger.ts

+28-1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ export function createConsoleLogger(
3333
break;
3434
case 'warn':
3535
color = (x: string) => terminal.bold(terminal.yellow(x));
36+
output = stderr;
3637
break;
3738
case 'fatal':
3839
case 'error':
@@ -41,7 +42,33 @@ export function createConsoleLogger(
4142
break;
4243
}
4344

44-
output.write(color(entry.message) + '\n');
45+
// If we do console.log(message) or process.stdout.write(message + '\n'), the process might
46+
// stop before the whole message is written and the stream is flushed. This happens when
47+
// streams are asynchronous.
48+
//
49+
// NodeJS IO streams are different depending on platform and usage. In POSIX environment,
50+
// for example, they're asynchronous when writing to a pipe, but synchronous when writing
51+
// to a TTY. In windows, it's the other way around. You can verify which is which with
52+
// stream.isTTY and platform, but this is not good enough.
53+
// In the async case, one should wait for the callback before sending more data or
54+
// continuing the process. In our case it would be rather hard to do (but not impossible).
55+
//
56+
// Instead we take the easy way out and simply chunk the message and call the write
57+
// function while the buffer drain itself asynchronously. With a smaller chunk size than
58+
// the buffer, we are mostly certain that it works. In this case, the chunk has been picked
59+
// as half a page size (4096/2 = 2048), minus some bytes for the color formatting.
60+
// On POSIX it seems the buffer is 2 pages (8192), but just to be sure (could be different
61+
// by platform).
62+
//
63+
// For more details, see https://nodejs.org/api/process.html#process_a_note_on_process_i_o
64+
const chunkSize = 2000; // Small chunk.
65+
let message = entry.message;
66+
while (message) {
67+
const chunk = message.slice(0, chunkSize);
68+
message = message.slice(chunkSize);
69+
output.write(color(chunk));
70+
}
71+
output.write('\n');
4572
});
4673

4774
return logger;

tests/legacy-cli/e2e/tests/build/bundle-budgets.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ export default function () {
5050
json.projects['test-project'].architect.build.options.budgets = [cfg.budget];
5151
})
5252
.then(() => ng('build', '--optimization'))
53-
.then(({ stdout }) => {
54-
if (!/WARNING in budgets/.test(stdout)) {
53+
.then(({ stderr }) => {
54+
if (!/WARNING in budgets/.test(stderr)) {
5555
throw new Error(cfg.message);
5656
}
5757
});
@@ -62,8 +62,8 @@ export default function () {
6262
json.projects['test-project'].architect.build.options.budgets = [cfg.budget];
6363
})
6464
.then(() => ng('build', '--optimization'))
65-
.then(({ stdout }) => {
66-
if (/(WARNING|ERROR)/.test(stdout)) {
65+
.then(({ stderr }) => {
66+
if (/(WARNING|ERROR)/.test(stderr)) {
6767
throw new Error(cfg.message);
6868
}
6969
});

tests/legacy-cli/e2e/tests/misc/circular-dependency.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ export default async function () {
77

88
await prependToFile('src/app/app.component.ts',
99
`import { AppModule } from './app.module'; console.log(AppModule);`);
10-
let output = await ng('build', '--show-circular-dependencies');
11-
if (!output.stdout.match(/WARNING in Circular dependency detected/)) {
10+
const { stderr } = await ng('build', '--show-circular-dependencies');
11+
if (!stderr.match(/WARNING in Circular dependency detected/)) {
1212
throw new Error('Expected to have circular dependency warning in output.');
1313
}
1414
}

0 commit comments

Comments
 (0)