Skip to content

Expose the typescript diagnostic object to error formatter functions#996

Closed
srgpqt wants to merge 3 commits intoTypeStrong:masterfrom
cortexmedia:feature/expose-diagnostic
Closed

Expose the typescript diagnostic object to error formatter functions#996
srgpqt wants to merge 3 commits intoTypeStrong:masterfrom
cortexmedia:feature/expose-diagnostic

Conversation

@srgpqt
Copy link

@srgpqt srgpqt commented Sep 3, 2019

This makes it easy to use TypeScript's builtin error formatters. Of particular interest is TypeScript's formatDiagnosticsWithColorAndContext function that produces much better error output.

A custom formatter could be defined as:

ts.formatDiagnosticsWithColorAndContext([error.diagnostic], {
    getCurrentDirectory: () => __dirname,
    getCanonicalFileName: (fileName) => path.normalize(fileName),
    getNewLine: () => os.EOL,
})

@johnnyreilly
Copy link
Member

This looks interesting! Could you write an execution test for this please?

You can likely clone this one:

https://github.com/TypeStrong/ts-loader/tree/master/test/comparison-tests/errorFormatter

There's some instructions here: https://github.com/TypeStrong/ts-loader/blob/master/test/comparison-tests/README.md

We only need expected output for the latest version of TypeScript 3.6

@srgpqt
Copy link
Author

srgpqt commented Sep 4, 2019

@johnnyreilly here you go :)

@johnnyreilly
Copy link
Member

Nice work! I can completely see that people might value this.

Would you like to update the README.md with details of this extra option please?

Also could you update the package.json version to reflect we're adding a new feature? So probably this will be 6.1.0. Likewise could you update the CHANGELOG.md as well?

It's probably worth blogging about this new feature BTW, do you write a blog?

@srgpqt
Copy link
Author

srgpqt commented Sep 5, 2019

@johnnyreilly , I've given this some more thought, and now I believe everyone would prefer to have the improved error formatter (formatDiagnosticsWithColorAndContext) used by default if available. What do you think?

@johnnyreilly
Copy link
Member

johnnyreilly commented Sep 5, 2019

I'm not sure.... The thing that makes me slightly cautious about that is that this API isn't always available and it could change. If it changes it might break ts-loader consumers on different versions of TypeScript. So the current approach seems safer.

And if it's well documented then it's easy for others to implement.

@stale
Copy link

stale bot commented Dec 14, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Dec 14, 2019
@cassiozen
Copy link

cassiozen commented Dec 16, 2019

@srgpqt - are you still contributing to this?
@johnnyreilly - I'm also interested in this - what is the best way to move forward? If @srgpqt is not responsive should I fork and re-submit the PR with updates in package.json and CHANGELOG.md? (Not interested in credits, just want to get this goiong =] )

@stale stale bot removed the wontfix label Dec 16, 2019
@srgpqt
Copy link
Author

srgpqt commented Dec 16, 2019

Feel free to pick up where I left off!

This PR was ready to merge already, as far as I can tell.

@cassiozen
Copy link

I think it was just a matter of updating package.json and adding to the changelog. At this point you would also need to fast-forward to the updates on master.

I can do it if you're not interested, but I also don't want to steal your credits by making a new PR.

@srgpqt
Copy link
Author

srgpqt commented Dec 16, 2019

I feel like package.json should be updated when preparing a new release, but I suppose I could do it anyway. It just feels wrong to do that when it’s uncertain when the next release to npm would be...

@cassiozen
Copy link

@johnnyreilly - any directions here?

@johnnyreilly
Copy link
Member

johnnyreilly commented Dec 17, 2019

Could you catch up this branch with master and would you like to update:

  • package.json
  • CHANGELOG.md
  • add some usage instructions to the README.md please?

Could the docs include an example of how to use formatDiagnosticsWithColorAndContext please?

Thanks!

@stale
Copy link

stale bot commented Feb 15, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Feb 15, 2020
@stale
Copy link

stale bot commented Feb 22, 2020

Closing as stale. Please reopen if you'd like to work on this further.

@stale stale bot closed this Feb 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants