microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
99.06k stars 12.29k forks source link

Public APIs not normalizing paths #9486

Open cartant opened 8 years ago

cartant commented 8 years ago

As discussed in this issue, the parseJsonConfigFileContent public API can fail on Windows when passed non-normalized paths (that is, paths containing backslash delimiters).

It appears that this is not the only public API with which this occurs. The following two public API functions call getDirectoryPath with non-normalized paths:

The first of these has caused numerous issue with tsify:

Is there any guidance regarding how module authors are supposed to use public TypeScript APIs that accept paths? Are paths always expected to be normalized? If so, that puts a burden on module developers. Would it not be preferable for the parseJsonConfigFileContent, findConfigFile, resolveTripleslashReference functions to normalize any paths before calling functions - like getDirectoryPath - that expect/assume normalized paths?

cartant commented 8 years ago

@mhegazy would you welcome a pull request that sees findConfigFile and resolveTripleslashReference call normalizeSlashes before getDirectoryPath is called?

At the moment, the findConfigFile function is broken on Windows for tsconfig.json files located in parent directories, as backslash-delimited paths are not correctly handled. If getDirectoryPath is passed this:

C:\Users\Nicholas\git\personal\something\app\something

it will return this:

C:

And the tsconfig.json file will not be found.

As far as I can see, there is no guidance as to whether paths are expected to have slashes normalized before being passed to TypeScript's public API. In fact, elsewhere in the codebase a path is passed to normalizeSlashes before being passed to getDirectoryPath.

basarat commented 8 years ago

As an experiment I have gone with / all paths in alm and that has worked out beautifully both on windows and mac :rose:

cartant commented 8 years ago

Thanks, @basarat. However, replacing my tool chain and switching to another IDE is not my preferred solution to the problem. Specifically, the current implementation of findConfigFile causes problems with tsify, as, in some situations, browserify supplies a base directory that contains backslashes. I use Sublime Text with the TypeScript plugin and have no problems with that part of my tool chain.

basarat commented 8 years ago

However, replacing my tool chain and switching to another IDE is not my preferred solution to the problem

I wasn't recommending that. I was supporting your suggestion to let ts do all the normalization :rose: