Add link to documentation in tsconfig.json template generated by tsc --init #34686
Conversation
|
A PR adding the link to the top is OK, but again, we do not want literally every compiler flag listed in the init tsconfig. For example, some of the flags listed here do nothing, are explicitly discouraged, or have dangerous effects. |
|
Reopening in case you want to just make the PR about providing the link to other options. In that case, I would also suggest we remove the emitted comment descriptions because they tend to be a major pain when using |
|
I removed the Advanced Options again... however I would suggest to move various options then into better category names like "Deprecated", "Use with Caution" etc. Me not being really familiar with the codebase just assumed these are all "Advanced options" that you should be able to use as an advanced user. In addition I removed the comment descriptions as you suggested. |
|
@borgfriend Can you update the description of this PR? I'm triaging our old PRs that we forgot about and I can't tell what this evolved into, but it seems different than the initial description. |
borgfriend
changed the title
|
I think it is quite misleading to only expose a subset of flags available in tsconfig.json when generating it via tsc --init. The current flags are "handpicked" because - there are dangerous, deprecated flags that should not be exposed. The issue evolved into:
Thus users would be encouraged to take a look at the documentation to learn more about available flags (also those that are not available in the 'handpicked selection') |
|
@borgfriend can you please update this to resolve merge conflicts.. Thanks |
|
There's an example of what we'd like it to look like in #37361 (comment) too |
|
@sheetalkamat I fixed the merge conflicts, @orta adjusted the link to be like in the example |
src/compiler/commandLineParser.ts
Outdated
| @@ -150,6 +150,14 @@ namespace ts { | |||
| category: Diagnostics.Command_line_Options, | |||
| description: Diagnostics.Whether_to_keep_outdated_console_output_in_watch_mode_instead_of_clearing_the_screen, | |||
| }, | |||
| { | |||
There was a problem hiding this comment.
There is already incremental included in this list why is there another entry?
There was a problem hiding this comment.
guess that was automerged and I missed it - pushed a fix for it
There was a problem hiding this comment.
Can you please existing entry instead? Is there reason this needs to move ?
| /* Basic Options */ | ||
| // "incremental": true, /* Enable incremental compilation */ |
There was a problem hiding this comment.
Example shows the comments as not removed #37361 (comment) @RyanCavanaugh @DanielRosenwasser Whats the final call on that
src/compiler/commandLineParser.ts
Outdated
| @@ -2108,10 +2108,9 @@ namespace ts { | |||
|
|
|||
| function isAllowedOption({ category, name }: CommandLineOption): boolean { | |||
| // Skip options which do not have a category or have category `Command_line_Options` | |||
| // Exclude all possible `Advanced_Options` in tsconfig.json which were NOT defined in command line | |||
There was a problem hiding this comment.
Return the comment, remove the extra ;
There was a problem hiding this comment.
The comment is still gone. Can you please fix that so that its ready to merge
| // "strictPropertyInitialization": true, /* Enable strict checking of property initialization in classes. */ | ||
| // "noImplicitThis": true, /* Raise error on 'this' expressions with an implied 'any' type. */ | ||
| // "alwaysStrict": true, /* Parse in strict mode and emit "use strict" for each source file. */ | ||
| /* Basic Options */ /* Enable incremental compilation */ |
There was a problem hiding this comment.
The comments are off by a line ?
There was a problem hiding this comment.
This is rather odd, i just reverted my change of removing them.
I adjusted the code so an offset is not possible anymore
|
I just noticed that for the flag "target" the description states the default is 'es3' - however it is set to 'es5'? Maybe the target and description should be updated to ES2017 (version supported by node 8, and all current Browsers without IE)? Should I change the flag? |
|
@borgfriend default for target is es3 (that means if target is not specified it is es3). The comment is correct |
Fixes #34538