Skip to content

Fix emitting ?. #37019

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 1 commit into from
Mar 4, 2020
Merged

Fix emitting ?. #37019

merged 1 commit into from
Mar 4, 2020

Conversation

elibarzilay
Copy link
Contributor

@elibarzilay elibarzilay commented Feb 25, 2020

Use writeTokenNode for writing questionDotToken, leading to properly
calling the emit hooks (which emitTokenWithComment doesn't). This
fixes #35372 by calling its hooks to set the .__pos and .__end
fields.

Also, remove getDotOrQuestionDotToken which was used only here --
mainly because it seems likely to encourage misusing the
questionDotToken again.

Also, fix a bunch of visitor -> tokenVisiton calls in
visitorPublic.ts.

@dragomirtitian
Copy link
Contributor

Ok, guess I'll close #35864 then.

@rbuckton
Copy link
Contributor

In general this looks good, but there are 5 failing tests.

Use `emit()` for writing `questionDotToken`, leading to properly calling
the emit hooks (which `emitTokenWithComment` doesn't) and printing the
comments.  This fixes microsoft#35372 by calling its hooks to set the `.__pos`
and `.__end` fields.

Also, remove `getDotOrQuestionDotToken` which was used only here --
mainly because it seems likely to encourage misusing the
`questionDotToken` again.

Also, fix a bunch of `visitor` -> `tokenVisiton` calls in
`visitorPublic.ts`.
@elibarzilay
Copy link
Contributor Author

In general this looks good, but there are 5 failing tests.

Everything should be fixed now (except for the existing emitting issue).

@elibarzilay elibarzilay merged commit 5c8def9 into microsoft:master Mar 4, 2020
@elibarzilay elibarzilay deleted the 35372 branch March 4, 2020 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactorings with optional chaining fail
4 participants