Improve configuration handling #1035
Conversation
This commit lets us simplify the codebase in a couple other ways: - helper `arrayify` and `objectify` functions make it easier to handle T | T[] types. - the CLI code can now use configurationPath to simplify its job
| @@ -45,7 +45,8 @@ export interface LintResult { | |||
| } | |||
|
|
|||
| export interface ILinterOptions { | |||
| configuration: any; | |||
| configuration?: any; | |||
| configurationPath?: string; | |||
There was a problem hiding this comment.
Most of the changes in this PR are good, but I'm wondering if we actually need this configurationPath option. If 3rd-party tools use findConfiguration/loadConfigurationFromPath, those functions can resolve relative paths which the programatic API should then be able to handle.
…e findConfiguration to easily handle loading configs, as the CLI now does
jkillian
force-pushed
the
configPath
branch
2 times, most recently
from
93e46c5
to
feef951
Compare
|
@adidahiya This needs review, and after this merges we can add the extends feature. I removed the part from this that dealt with |
|
Can't wait for this to go in so I can complete the extend feature. |
| @@ -19,6 +19,13 @@ import * as fs from "fs"; | |||
| import * as path from "path"; | |||
| import * as findup from "findup-sync"; | |||
|
|
|||
| import {arrayify, objectify} from "./utils"; | |||
|
|
|||
| export interface IConfigFile { | |||
There was a problem hiding this comment.
let's be consistent about the abbreviation -- I think it should be "Configuration" everywhere, not "Config"
edit: let's just try to use "Configuration" in all names in public APIs. private variable names can use "config"
There was a problem hiding this comment.
So just leave this as is, right? Exported things are a blurry line between private/public, but I assume we're only treating as public things in the top level package
There was a problem hiding this comment.
Sorry to chime in, but I think it should be renamed is IConfigurationFile as this interface is useful to extensions.
There was a problem hiding this comment.
anything exported is part of the public API; let's just assume people will use anything they can find in our generated typings
There was a problem hiding this comment.
Agreed, the tricky thing here is we already have public APIs like
'DEFAULT_CONFIG' before that don't obey this convention.
|
overall lgtm 👍 only blocking issue is the naming convention in public API surface |
|
[Updated] |
|
👍 |
This commit lets us simplify the codebase in a couple other ways:
arrayifyandobjectifyfunctions make it easier to handle T| T[] types.
loadConfigurationFromPathnow resolves rules directories to absolute paths. This means 3rd-party tools can doconfig: findConfiguration("possible/user/supplied/config/path", "file/ts/lint.ts")and everything should work correctly.Closes #1019