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

#249 Add explicit info logger to avoid writing info logs to stderr #313

Merged
merged 9 commits into from
Oct 17, 2016
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,16 @@ different dependencies in your application will be lost. You should also
set the `isolatedModules` TypeScript option if you plan to ever make use
of this.

##### logInfoToStdOut *(boolean) (default=false)*

This is important if you read from stdout or stderr and for proper error handling.
The default value ensures that you can read from stdout e.g. via pipes or you use webpack -j to generate json output.

##### logLevel *(string) (default=info)*

Can be `info`, `warn` or `error` which limits the log output to the specified log level.
Beware of the fact that errors are written to stderr and everything else is written to stderr (or stdout if logInfoToStdOut is true).

##### silent *(boolean) (default=false)*

If true, no console.log messages will be emitted. Note that most error
Expand Down
52 changes: 45 additions & 7 deletions index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ var Console = require('console').Console;
var semver = require('semver')
require('colors');

const console = new Console(process.stderr);
const stderrConsole = new Console(process.stderr);
const stdoutConsole = new Console(process.stdout);

var pushArray = function(arr, toPush) {
Array.prototype.splice.apply(arr, [0, 0].concat(toPush));
Expand All @@ -25,8 +26,16 @@ function hasOwnProperty(obj, property) {
return Object.prototype.hasOwnProperty.call(obj, property)
}

enum LogLevel {
INFO = 1,
WARN = 2,
ERROR = 3
}

interface LoaderOptions {
silent: boolean;
logLevel: string;
logInfoToStdOut: boolean;
instance: string;
compiler: string;
configFileName: string;
Expand Down Expand Up @@ -149,8 +158,34 @@ function findConfigFile(compiler: typeof typescript, searchPath: string, configF
function ensureTypeScriptInstance(loaderOptions: LoaderOptions, loader: any): { instance?: TSInstance, error?: WebpackError } {

function log(...messages: string[]): void {
logToConsole(stdoutConsole, messages);
}

function logToConsole(logConsole:any, messages: string[]): void {
if (!loaderOptions.silent) {
console.log.apply(console, messages);
console.log.apply(logConsole, messages);
}
}

function logInfo(...messages: string[]): void {
Copy link
Member

@johnnyreilly johnnyreilly Oct 14, 2016

Choose a reason for hiding this comment

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

To avoid using .toUpperCase() repeatedly, could we uppercase the value when it's first read from the options in the loader function? That way we do it once rather than repeatedly (marginal perf gain) and the code reads a little cleaner (which is what I care about more).

if (LogLevel[loaderOptions.logLevel] <= LogLevel.INFO) {
if(loaderOptions.logInfoToStdOut) {
logToConsole(stdoutConsole, messages);
} else {
logToConsole(stderrConsole, messages);
}
}
}

function logError(...messages: string[]): void {
if (LogLevel[loaderOptions.logLevel] <= LogLevel.ERROR) {
logToConsole(stderrConsole, messages);
}
}

function logWarning(...messages: string[]): void {
if (LogLevel[loaderOptions.logLevel] <= LogLevel.WARN) {
logToConsole(stderrConsole, messages);
}
}

Expand Down Expand Up @@ -180,11 +215,11 @@ function ensureTypeScriptInstance(loaderOptions: LoaderOptions, loader: any): {
compilerCompatible = true;
}
else {
log(`${motd}. This version is incompatible with ts-loader. Please upgrade to the latest version of TypeScript.`.red);
logError(`${motd}. This version is incompatible with ts-loader. Please upgrade to the latest version of TypeScript.`.red);
}
}
else {
log(`${motd}. This version may or may not be compatible with ts-loader.`.yellow);
logWarning(`${motd}. This version may or may not be compatible with ts-loader.`.yellow);
}

var files = <TSFiles>{};
Expand All @@ -211,8 +246,8 @@ function ensureTypeScriptInstance(loaderOptions: LoaderOptions, loader: any): {
error?: typescript.Diagnostic;
};
if (configFilePath) {
if (compilerCompatible) log(`${motd} and ${configFilePath}`.green)
else log(`ts-loader: Using config file at ${configFilePath}`.green)
if (compilerCompatible) logInfo(`${motd} and ${configFilePath}`.green)
else logInfo(`ts-loader: Using config file at ${configFilePath}`.green)

// HACK: relies on the fact that passing an extra argument won't break
// the old API that has a single parameter
Expand All @@ -227,7 +262,7 @@ function ensureTypeScriptInstance(loaderOptions: LoaderOptions, loader: any): {
}
}
else {
if (compilerCompatible) log(motd.green)
if (compilerCompatible) logInfo(motd.green)

configFile = {
config: {
Expand Down Expand Up @@ -556,13 +591,16 @@ function loader(contents) {

var options = objectAssign<LoaderOptions>({}, {
silent: false,
logLevel: 'INFO',
logInfoToStdOut: false,
instance: 'default',
compiler: 'typescript',
configFileName: 'tsconfig.json',
transpileOnly: false,
compilerOptions: {}
}, configFileOptions, queryOptions);
options.ignoreDiagnostics = arrify(options.ignoreDiagnostics).map(Number);
options.logLevel = options.logLevel.toUpperCase();

// differentiate the TypeScript instance based on the webpack instance
var webpackIndex = webpackInstances.indexOf(this._compiler);
Expand Down