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

Improve error message when compiling a .js file #34861

Merged

Conversation

lukahartwig
Copy link
Contributor

Fixes #33585

I noticed this grammar error and saw that there already was an issue for it so I implemented the suggested changes.

@msftclas
Copy link

msftclas commented Nov 1, 2019

CLA assistant check
All CLA requirements met.

@orta orta self-assigned this Nov 1, 2019
@orta
Copy link
Contributor

orta commented Nov 1, 2019

Hi @lukahartwig - I really like this idea, I'm open to getting this in 👍 .

Is it possible that for the last part of the sentence, you first check whether we are looking at a JavaScript or JSON file first? (It might need two error messages then)

@lukahartwig
Copy link
Contributor Author

Good point, I try to make it work.

@lukahartwig
Copy link
Contributor Author

Okay, so I added two error messages for JS and JSON files. For other extensions, it will fall back to the old message. I might need some help with the wording since I'm not a native speaker.

I also wasn't able to reproduce the JSON file case with the baseline tests because apparentlyresolveJsonModule is already handled at an earlier stage. I'm not super familiar with the codebase though so maybe I missed something.

@orta
Copy link
Contributor

orta commented Nov 4, 2019

This is great @lukahartwig - the messages read good to me, let's get this in. I think it makes sense to remove the JSON error message then I'll merge! Thanks!

@lukahartwig
Copy link
Contributor Author

Cool, I removed the JSON error message. Thanks for your help :)

@orta
Copy link
Contributor

orta commented Nov 4, 2019

Thanks for contributing!

@orta orta merged commit 9a3ec5f into microsoft:master Nov 4, 2019
@lukahartwig lukahartwig deleted the improve-error-message-for-js-files branch November 4, 2019 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Poor error message when compiling a .js file
3 participants