Skip to content

Improve diagnostics and add code fixes for top-level await #36173

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

Merged
merged 3 commits into from
Jan 18, 2020

Conversation

rbuckton
Copy link
Contributor

This improves the diagnostic messages for top-level await and adds several code-fixes to improve the experience.

In addition, this fixes a small bug in our textChanges algorithm to ensure we correctly add new-lines when inserting nodes at the top of a class body/object literal (and elide trailing commas in an object literal when they aren't needed.

Fixes #36036
Fixes #36171

@DanielRosenwasser
Copy link
Member

Looks like there's a merge conflict.


@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 14, 2020

Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 80b065f. You can monitor the build here. It should now contribute to this PR's status checks.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jan 14, 2020

The message improvements look good.

I would suggest always keeping the trailing comma to be honest - but I happen to be especially prone to adding new fields after members that lack a comma.

@rbuckton
Copy link
Contributor Author

@DanielRosenwasser: Trailing comma is illegal in JSON, so if we ever have a codefix that affects package.json, it would make that file invalid. I'd rather trim the trailing comma to be safe.

@DanielRosenwasser
Copy link
Member

Would it be too much trouble to have the codefix check if it's in ES3 or working under JSON?

@DanielRosenwasser
Copy link
Member

or have the printer "do the right thing" under either mode?

@rbuckton rbuckton force-pushed the topLevelAwaitDiagnostics branch from 2ee66fd to 05a88f6 Compare January 15, 2020 01:23
# Conflicts:
#	src/services/textChanges.ts
@rbuckton rbuckton force-pushed the topLevelAwaitDiagnostics branch from 05a88f6 to 5963496 Compare January 18, 2020 01:39
# Conflicts:
#	src/compiler/diagnosticMessages.json
@rbuckton rbuckton force-pushed the topLevelAwaitDiagnostics branch from ed5bcf9 to 2d80fed Compare January 18, 2020 09:22
@rbuckton
Copy link
Contributor Author

I've updated it so that it only elides the trailing comma when the source file is a .json file (since there's no way to tell whether it's a normal .json or a non-standard .json like our tsconfig files).

@rbuckton rbuckton merged commit 50adabe into master Jan 18, 2020
@rbuckton rbuckton deleted the topLevelAwaitDiagnostics branch January 18, 2020 20:37
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.

"Implement interface" codefix missing newline after first member Top-level await error message is extremely unclear
4 participants