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.

Sign up for GitHub

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
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0995e45
Add explicit info logger to avoid writing info logs to stderr
Oct 13, 2016
11d9455
Merge branch 'master' of https://github.com/TypeStrong/ts-loader
Oct 14, 2016
5165114
Add a loaderOption for the logLevel and logging to different logLevels
Oct 14, 2016
c045ec5
Fix tsc compile error
Oct 14, 2016
51f8c68
Add a new loaderOption to specify the info logging output.
Oct 14, 2016
e546515
Merge branch 'master' of https://github.com/TypeStrong/ts-loader
Oct 17, 2016
79ae1ed
Change documentation text according to review advise
Oct 17, 2016
5ccd4fc
Doing uppercase on loaderOptions logLevel just once
Oct 17, 2016
6c337d5
Bugfix the default value of logLevel must be string
Oct 17, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ 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.

##### 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 stdout.
Copy link
Member

Choose a reason for hiding this comment

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

Could we specify details about logInfoToStdOut before logLevel in the documentation? Also, instead of:

errors are written to stderr and everything else is written to stdout

could we say:

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
41 changes: 34 additions & 7 deletions index.ts
View file
Open in desktop
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,15 @@ 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;
instance: string;
compiler: string;
configFileName: string;
Expand Down Expand Up @@ -148,12 +156,30 @@ function findConfigFile(compiler: typeof typescript, searchPath: string, configF
// `instance` property.
function ensureTypeScriptInstance(loaderOptions: LoaderOptions, loader: any): { instance?: TSInstance, error?: WebpackError } {

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

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

@johnnyreilly johnnyreilly Oct 14, 2016 •
edited
Loading

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.toUpperCase()] <= LogLevel.INFO) {
Copy link
Member

Choose a reason for hiding this comment

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

Rather than doing .toUpperCase() repeatedly here could we initialise the value of logLevel to the uppercase value when it is first read in? That way the code will read a little cleaner and there will be a (super minor) perf gain.

log(stdoutConsole, messages);
}
}

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

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

if (hasOwnProperty(instances, loaderOptions.instance)) {
return { instance: instances[loaderOptions.instance] };
}
Expand All @@ -180,11 +206,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 @@ -209,8 +235,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 @@ -225,7 +251,7 @@ function ensureTypeScriptInstance(loaderOptions: LoaderOptions, loader: any): {
}
}
else {
if (compilerCompatible) log(motd.green)
if (compilerCompatible) logInfo(motd.green)

configFile = {
config: {
Expand Down Expand Up @@ -538,6 +564,7 @@ function loader(contents) {

var options = objectAssign<LoaderOptions>({}, {
silent: false,
logLevel: 'info',
instance: 'default',
compiler: 'typescript',
configFileName: 'tsconfig.json',
Expand Down