Change project option resolution #3102
Conversation
|
Hey @vvakame, thanks for the PR!
We sort of do - we have project tests. However, I don't think any of them test tsconfig.json and I'm not sure exactly how we could. I think it would be helpful to have a discussion with @JsonFreeman and @vladima offline to figure things out.
Agreed, though I'd make it
Not sure what you mean; what regression are you referring to? |
| export function readConfigFile(fileName: string): { config?: any; error?: Diagnostic } { | ||
| export function readConfigFile(fileName: string): { config?: any; error?: Diagnostic } { | ||
| if (!sys.fileExists(fileName)) { | ||
| return { error: createCompilerDiagnostic(Diagnostics.Unable_to_open_file_0, fileName) }; |
There was a problem hiding this comment.
what regression are you referring to?
this line.
$ tsc -v
message TS6029: Version 1.5.0-beta
$ tsc --project ./not-exists/
error TS6050: Unable to open file 'not-exists/tsconfig.json'.
$ node ~/work/TypeScript/bin/tsc.js --project ./not-exists/
error TS5014: Failed to parse file 'not-exists/tsconfig.json': Unexpected token u.
# I think this is regression. error message is not easy to understanding.
$ node ~/work/TypeScript/built/local/tsc.js --project ./not-exists/
error TS6050: Unable to open file 'not-exists'.
# fixed by this change
|
thank you for comments!
😺 👍
I made a commit! |
|
tests\cases\projects are indeed where we normally do 'command line' sort of tests. There's a thin implementation of a batch compiler that handles the work. It's not ideal but historically we've had very few bugs in that layer which aren't caught by our other test suites which are easier to author/validate and faster to run. |
|
In my understanding, src/harness/projectsRunner.ts can not test parsing of command line option, it only testing |
|
Correct, the options in the project runner are the result of JSON.parse, and not the command line parser. |
|
So I have a few concerns here after thinking about this some more. I know the original issue was marked as accepting PRs but I'm not sure how this will actually work. The main concept to start with here is that tsconfig.json is not a project file, it is not a build system, it is a means for editors to acquire project context so that loose files can be opened and edited with fidelity (completion lists, type checking, etc). That is the context under which it was created, not because we found .csproj lacking for setting project options, build targets, etc. That is also why it is missing relatively simple features of a build system like file globbing, configuration targets (like this fix is aiming for), etc. The problem this is trying to solve is, roughly: "how do I specify different build configurations for my TypeScript project?" This could be something like Release (minify) vs Debug (sourcemaps), or ES5 vs ES6, client (many asynchronously loaded modules) vs server (a single bundled module), etc. The core question is whether these configurations should exist within the same build specification (ie file) or each in separate build specifications. This issue/fix chooses the latter. Almost all other systems I can think of (Visual Studio projects, makefiles, grunt, etc) choose the former. Why would we want to go this different route? There are good reasons to do it within the same file. For example, consider that right now tsconfig.json maps conceptually to a .csproj file. Well .csproj files have .sln files to group them together, a concept we currently lack for tsconfig. What does a future .sln equivalent look like if it has to understand n different config files next to one another (with arbitrary names and file extensions) are actually the same project but different configurations? What makes this approach even more dangerous for us is that unlike some of those other systems (ex makefile, grunt) we have tightly coupled the editor experience to tsconfig.json and no other files. This is precisely because of the original design aim noted for tsconfig.json: to give the editor context on your project. If I open a loose file in a project how does the editor know which config file to use, or even which files are my config files (don't use package.config, but do use myConfig.config and myOtherConfig.txt)? These config files effect language service errors (target ES3 vs ES5), JavaScript emit (--target, --module, etc), and other build artifacts (sourcemaps) so it's important that editors are aware of the full context. Basically if we want to bite the bullet and admit that tsconfig.json is now a project file/build system format then we need to sit down and holistically look at the requirements and design rather than patch it together one bug fix at a time and risk causing ourselves avoidable future pain. |
|
sorry, I lack of confidence read english correctly... first, Why I created this PR? because issue is labeled by I want become tsconfig.json to motivations. but, Each editors implement scope that is a very hard work! |
implement #2869
does this repository not have tests for command line options?
I was create a sample file on gist. https://gist.github.com/vvakame/1d10fbef9a19fc8c05f7
I think
-p DIRECTORY, --project DIRECTORY Compile the project in the given directory.should be change.-p TARGET, --project TARGET Compile the project in the given directory or file of tsconfig.json format.?BTW, This pull request includes a patch for regression. should I separate a PR? It broken maybe between 1.5.0-beta and master/HEAD.