prettier / tslint-config-prettier

Use TSLint with Prettier without any conflict
MIT License
1.23k stars 42 forks source link

tslint-config-prettier-check relative path on Windows #341

Open PMExtra opened 5 years ago

PMExtra commented 5 years ago

I just test it on Windows, but I think it may also occurs on other platform (Edited: It's because windows path separator \, so it won't occurs on other platform).

>.\node_modules\.bin\tslint-config-prettier-check tslint.json
Invalid "extends" configuration value - could not require "tslint.json". Review the Node lookup algorithm (https://nodejs.org/api/modules.html#modules_all_together) for the approximate method TSLint uses to find the referenced configuration file.        

>.\node_modules\.bin\tslint-config-prettier-check .\tslint.json
Invalid "extends" configuration value - could not require ".\tslint.json". Review the Node lookup algorithm (https://nodejs.org/api/modules.html#modules_all_together) for the approximate method TSLint uses to find the referenced configuration file.      

>.\node_modules\.bin\tslint-config-prettier-check %cd%\tslint.json
(no output, it works)

>.\node_modules\.bin\tslint-config-prettier-check ..\..\..\tslint.json
(no output, it works)

%cd% is equivalent to UNIX $(pwd) for Windows.

It seems because the check command calculate the relative path based node_modules/... instead the current work directory of shell.

ikatyang commented 5 years ago

Ref: https://github.com/palantir/tslint/blob/5.18.0/src/configuration.ts#L236-L245

We simply call loadConfigurationFromPath from TSLint to load configs, which uses the Node module resolution to find it. Since it uses Node resolution, you should pass ./tslint.json instead of .\tslint.json.

This does not look like a bug but something we could improve, PRs welcome.

PMExtra commented 5 years ago

@ikatyang Great! So it's only a trouble for Windows users. It's awesome if we can simply replace the '\' with '/' on Windows. I'm not familiar with this project. I think maybe you could add below code in place.

if (process.platform === "win32") {
  path = path.replace(/\\/g, '/')
}
PMExtra commented 5 years ago

I have checked.

> tslint-config-prettier-check ./tslint.json

> tslint-config-prettier-check C:/path/to/project/tslint.json

They are all works. So we can replace all the \ with / on Windows at ease.