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

flow: Fix printing of methods and static properties in declare class #1106

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

gnprice
Copy link
Contributor

@gnprice gnprice commented May 8, 2022

Also make some infrastructural improvements to the Flow-related tests which I spotted along the way.

This adds a @ts-expect-error because of a property that's missing in ast-types. I've sent benjamn/ast-types#755 to fix that, so it'll naturally go away after that fix is merged and Recast's dependency on ast-types is updated to use it.

At this point there are a number of Flow features that the Esprima
parser doesn't support.  Rather than switch to the Flow parser ad hoc
for those test cases, simplify by just using it systematically.
Use the same solution already present for ObjectTypeInternalSlot.

This doesn't feel super clean -- plus, it'd be nice to unify with
ObjectTypeInternalSlot, since that's so similar.  But it doesn't seem
materially less clean than the existing logic here, and it works,
so it's a step forward for now.
I started by converting this test to a round-trip test with the same
source code verbatim, just intending to make the test simpler and
easier to read.

But then it turned out that that version failed, because this code
doesn't parse as this tree!  It parses as something wacky, which spits
out wacky output.  And in fact Flow itself doesn't accept this code;
it gives a series of parse errors.

Instead, write a version of the code that Flow accepts (at least as
far as parsing it -- it gives a later-stage error saying it doesn't
recognize these particular internal slots), mainly by saying
`declare class C` instead of `type A =`.

So this version is not only simpler but more correct.

Also add a few more cases that weren't covered in the previous test.
Now that it only takes one line of code per case, it's easy to be
thorough.
…ternalSlot

This makes the logic for each of these less vulnerable to missing some
functionality that was added for another one.

Some of these features never appear in some of these cases; for
example, indexers can't be optional.  But when absent, they're
harmlessly ignored.
gnprice added a commit to gnprice/tsflower that referenced this pull request May 8, 2022
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.

1 participant