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

refactor: use createConsoleLogger and remove duplicate code #12787

Merged
merged 1 commit into from
Nov 8, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
66 changes: 2 additions & 64 deletions packages/angular/cli/lib/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,14 @@
* found in the LICENSE file at https://angular.io/license
*/

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


export default async function(options: { testing?: boolean, cliArgs: string[] }) {
const logger = new logging.IndentLogger('cling');
let loggingSubscription;
if (!options.testing) {
loggingSubscription = initializeLogging(logger);
}
const logger = createConsoleLogger();

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

if (loggingSubscription) {
loggingSubscription.unsubscribe();
}

return 1;
}
}

// Initialize logging.
function initializeLogging(logger: logging.Logger) {
return logger
.subscribe(entry => {
let color = (x: string) => terminal.dim(terminal.white(x));
let output = process.stdout;
switch (entry.level) {
case 'debug':
return;
case 'info':
color = terminal.white;
break;
case 'warn':
color = (x: string) => terminal.bold(terminal.yellow(x));
output = process.stderr;
break;
case 'fatal':
case 'error':
color = (x: string) => terminal.bold(terminal.red(x));
output = process.stderr;
break;
}


// If we do console.log(message) or process.stdout.write(message + '\n'), the process might
// stop before the whole message is written and the stream is flushed. This happens when
// streams are asynchronous.
//
// NodeJS IO streams are different depending on platform and usage. In POSIX environment,
// for example, they're asynchronous when writing to a pipe, but synchronous when writing
// to a TTY. In windows, it's the other way around. You can verify which is which with
// stream.isTTY and platform, but this is not good enough.
// In the async case, one should wait for the callback before sending more data or
// continuing the process. In our case it would be rather hard to do (but not impossible).
//
// Instead we take the easy way out and simply chunk the message and call the write
// function while the buffer drain itself asynchronously. With a smaller chunk size than
// the buffer, we are mostly certain that it works. In this case, the chunk has been picked
// as half a page size (4096/2 = 2048), minus some bytes for the color formatting.
// On POSIX it seems the buffer is 2 pages (8192), but just to be sure (could be different
// by platform).
//
// For more details, see https://nodejs.org/api/process.html#process_a_note_on_process_i_o
const chunkSize = 2000; // Small chunk.
let message = entry.message;
while (message) {
const chunk = message.slice(0, chunkSize);
message = message.slice(chunkSize);
output.write(color(chunk));
}
output.write('\n');
});
}
4 changes: 1 addition & 3 deletions packages/angular/cli/models/architect-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ export abstract class ArchitectCommand<
private _host = new NodeJsSyncHost();
protected _architect: Architect;
protected _workspace: experimental.workspace.Workspace;
protected _logger = createConsoleLogger();

protected _registry: json.schema.SchemaRegistry;

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

const result = await this._architect.run(realBuilderConf, { logger: this._logger }).toPromise();
const result = await this._architect.run(realBuilderConf, { logger: this.logger }).toPromise();

return result.success ? 0 : 1;
}
Expand Down
29 changes: 28 additions & 1 deletion packages/angular_devkit/core/node/cli-logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export function createConsoleLogger(
break;
case 'warn':
color = (x: string) => terminal.bold(terminal.yellow(x));
output = stderr;
break;
case 'fatal':
case 'error':
Expand All @@ -41,7 +42,33 @@ export function createConsoleLogger(
break;
}

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

return logger;
Expand Down
8 changes: 4 additions & 4 deletions tests/legacy-cli/e2e/tests/build/bundle-budgets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ export default function () {
json.projects['test-project'].architect.build.options.budgets = [cfg.budget];
})
.then(() => ng('build', '--optimization'))
.then(({ stdout }) => {
if (!/WARNING in budgets/.test(stdout)) {
.then(({ stderr }) => {
if (!/WARNING in budgets/.test(stderr)) {
throw new Error(cfg.message);
}
});
Expand All @@ -62,8 +62,8 @@ export default function () {
json.projects['test-project'].architect.build.options.budgets = [cfg.budget];
})
.then(() => ng('build', '--optimization'))
.then(({ stdout }) => {
if (/(WARNING|ERROR)/.test(stdout)) {
.then(({ stderr }) => {
if (/(WARNING|ERROR)/.test(stderr)) {
throw new Error(cfg.message);
}
});
Expand Down
4 changes: 2 additions & 2 deletions tests/legacy-cli/e2e/tests/misc/circular-dependency.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ export default async function () {

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