Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CLI DX] Improve the output for tsc --init #45714

Closed
orta opened this issue Sep 3, 2021 · 8 comments · Fixed by #45930
Closed

[CLI DX] Improve the output for tsc --init #45714

orta opened this issue Sep 3, 2021 · 8 comments · Fixed by #45930
Labels
Effort: Casual Good issue if you're already used to contributing to the codebase. Harder than "good first issue". Experience Enhancement Noncontroversial enhancements Good First Issue Well scoped, documented and has the green light Help Wanted You can do this

Comments

@orta
Copy link
Contributor

orta commented Sep 3, 2021

Suggestion

As of 4.4, a run of tsc looks like:

$ /Users/ortatherox/dev/typescript/resolve-ts/node_modules/.bin/tsc --init     
message TS6071: Successfully created a tsconfig.json file.                                                            

I propose instead we change this message to provide more context about a few key values which have been set:

Example

$ /Users/ortatherox/dev/typescript/resolve-ts/node_modules/.bin/tsc --init           
                                                                                     
Created a new tsconfig.json with:                                                 [    ]
                                                                                  [  TS]
  strict: true                                                                       
  target: es5                                                                        
  module: commonjs 
  esModuleInterop: true
                                                     
                                                                                     
You can learn more at https://aka.ms/tsconfig.json                                   

The qualifiers for whether something should be included in the list is that showInSimplifiedHelpView: true and the value is not the default. In the case about target is es5 but the default is es3 for example.

The cute little right aligned logo is used in tsc --help so you can re-use that code.

It's worth noting that a user can pass in defaults via tsc --init --target es2017, but I think that occurs before the code where you would write this, so it's just context which might be useful for testing.

Exceptions

None that I can think of.

@orta orta added Help Wanted You can do this Good First Issue Well scoped, documented and has the green light Experience Enhancement Noncontroversial enhancements Effort: Casual Good issue if you're already used to contributing to the codebase. Harder than "good first issue". labels Sep 3, 2021
@AlexMunoz
Copy link
Contributor

Hey @orta, i would like to try implementing it.

@orta
Copy link
Contributor Author

orta commented Sep 3, 2021

Sure, give it a shot - until there's a solid WIP PR it's open for anyone to give it a go.

@wizered67
Copy link

Hi @orta! I’ve made some progress on this, but I’d appreciate some guidance as to whether I’m on the right track and how to deal with a couple problems I’m running into:

Getting serialized option value

I believe we’d want the new logic for outputting the set values to live in executeCommandLine.ts. One thing I struggled with is getting executeCommandLine.ts to know the serialized representation of some of the enums, for example going from ScriptTarget.ES5 to "es5". It seems like that logic is currently done in commandLineParser.ts in functions like serializeCompilerOptions - however, that function is currently unexported.

My current approach is to export that function so that I can use it in executeCommandLine.ts, but I’m unsure how chill it is to add an export like this. Is this a reasonable approach? Other options I considered were making an exported new version of generateTSConfig that also returns the compilerOptionsMap it uses or simply duplicating code into executeCommandLine.ts.

Determining if default

The other thing I’m struggling with is how to determine whether a config value is different from the default. I saw there's a defaultValueDescription property for options in commandLineParser.ts, but it's a description and not the value itself. For example, it uses "ES3" as the defaultValueDescription for target. However, running tsc --init --target ES3 would use "es3" as the value. This is a simple case where just doing a case-insensitive comparison would work, but it seems like there are more complex cases where defaultValueDescription doesn’t directly correspond to a value. For example declaration uses defaultValueDescription: Diagnostics.false_unless_composite_is_set.

Do you have suggestions for figuring out if a value isn’t a default? Should I be hardcoding something for the specific options listed in this issue?

I appreciate any help you can offer! And also, @AlexMunoz, if you've attempted this too and have any advice, let me know! Thanks!

@orta
Copy link
Contributor Author

orta commented Sep 8, 2021

Ace!

Re: Getting serialized option value

Exporting that function and marking it internal is 👍🏻 ( that might even mean I can opt in to sneakily re-use it in the playground too) - it's the same pattern use for convertToTSConfig

Determining if default

Interesting, I wonder if we can generate a 'blank' tsconfig object and compare the one generated by that with the one we're about to make and see what the differences are there instead. Will look in the code now.

Maybe parseConfigFileTextToJson("blank.tsconfig.json", "{}")?

@wizered67
Copy link

From a quick test it seemed like parseConfigFileTextToJson("blank.tsconfig.json", "{}") just returned a config of {}.

From what I can tell, it seems like an approach like generating a blank tsconfig wouldn't work because the default settings aren't applied at the tsconfig level, but at a later point. For example, the default setting for target is es3, and, correct me if I'm wrong,
it seems like that gets applied as the default here.

I think based on that it would be hard to know what the default for any given setting is without having to hardcode something like "for target, call getEmitScriptTarget with {} and see what we get back" (or alternately just hardcode that the default for target is es3).

I think this gets particularly complex for settings like module where the default seems to depend on other settings.
For settings that depend on others like that perhaps we could do something like getEmitModuleKind({...compilerOptions, module: undefined}) to find what the default would normally be for the other settings that are being used 🤔

I'd appreciate any thoughts you have - I'm not sure if I'm overcomplicating something here or if this is actually complicated!

@wizered67
Copy link

Hi @orta, following up to see if you have any further suggestions! I'm still interested in taking this on, but was feeling blocked on how to figure if a setting is different from the default.

@orta
Copy link
Contributor Author

orta commented Sep 17, 2021

Ah sorry - this issue isn't assigned to me, so it falls through some of my email filters. Some great questions, after digging through the code I think maybe it's worth thinking about it from scratch again. From a design perspective:

tsc --init

Should show the diff from the blank tsconfig.

tsc --init --target es2015

Should show es2015 for target, instead of the current es5.

tsc --init --preserveConstEnums false

We probably want to also include preserveConstEnums in that output. For confirmation that it was registered.

This means we have two sets of inputs we want to show, and instead of trying to take two tsconfigs and resolve the difference, we can instead use the known sets of changes and work to present them. So we have this code in the compiler:

export const defaultInitCompilerOptions: CompilerOptions = {
        module: ModuleKind.CommonJS,
        target: ScriptTarget.ES5,
        strict: true,
        esModuleInterop: true,
        forceConsistentCasingInFileNames: true,
        skipLibCheck: true
    };

Which is the tsc --init defaults and generateTSConfig(options: CompilerOptions this options param which should only contain params passed into the CLI (e.g. preserveConstEnums and target above) - so in theory we should be able to use the list of all defaultInitCompilerOptions and options key: values to show the diff. That should reduce the complexity a bunch.

If you struggle to find a way to go from ScriptTarget.ES2015 (1) to "es2015" leave that to me, and I'll figure that out and make something general, it can just print the number

Maybe then we drop the requirement that showInSimplifiedHelpView be set to be true, because then it could act inconsistent with your CLI input.

@AlexMunoz
Copy link
Contributor

Hi @orta, @wizered67 I have tried to fix this, not sure if this is the right way. If you have any advice, let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Effort: Casual Good issue if you're already used to contributing to the codebase. Harder than "good first issue". Experience Enhancement Noncontroversial enhancements Good First Issue Well scoped, documented and has the green light Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants