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

prettier: use Babylon (babel-flow) as the parser #3969

Merged
merged 1 commit into from
Mar 13, 2020

Conversation

rk-for-zulip
Copy link
Contributor

@rk-for-zulip rk-for-zulip commented Mar 12, 2020

The babel-flow and flow parsers produce subtly different ASTs, and Prettier's support for the latter is relatively poor.

In particular, when using the flow parser:

This commit will enable some simple uses of private fields, but not all. See our commit 86a4258 and its associated PR (#3725) for more detail.

The more important fix (and the one that can't be obtained by updating Prettier, at least not yet) is the second. This will allow us to rely on Flow's typechecking in scripts which will be executed directly by Node without a preliminary Babel run -- e.g., scripts residing in tools/, or plugins for support tools such as Jest or ESLint.

@rk-for-zulip
Copy link
Contributor Author

(This PR was motivated by the fact that I've needed to make this change for two separate scripts currently in work – the translation automation script, and a better-for-our-purposes import-rules ESLint plugin.)

rk-for-zulip added a commit to rk-for-zulip/zulip-mobile that referenced this pull request Mar 12, 2020
(This is the commit from PR zulip#3969. It should be eliminated by a rebase
after that's approved. *This* PR, in addition to its actual use, is a
motivating example for it.)
The `babel-flow` and `flow` parsers produce subtly different ASTs, and
Prettier's support for the latter has historically been relatively poor.

In particular, when using the `flow` parser:
  * prettier/prettier#5288: the private field sigil `#` is stripped.
    (Resolved, but not in our current version.)
  * prettier/prettier#2597: type-declaration comments are promoted to
    inline type declarations.

This commit will enable some simple uses of private fields, but not
all. See our commit 86a4258 and its
associated PR (zulip#3725) for more detail.

The more important fix (and the one that can't be obtained by updating
Prettier, at least not yet) is the second. This will allow us to rely
on Flow's typechecking in scripts which will be executed directly by
Node without a preliminary Babel run -- e.g., scripts residing in
`tools/`, or plugins for support tools such as Jest or ESLint.
@gnprice gnprice merged commit fcd233b into zulip:master Mar 13, 2020
@gnprice
Copy link
Member

gnprice commented Mar 13, 2020

Thanks @ray-kraesig ! Merged. Glad to enable that tools use-case.

I tweaked the commit message slightly so "prettier issue NNNN" becomes "prettier/prettier#NNNN", just to save the reader having to look up the exact location of Prettier's issue tracker.

@rk-for-zulip rk-for-zulip deleted the change-babel-parser branch March 13, 2020 17:20
@rk-for-zulip
Copy link
Contributor Author

Thanks @ray-kraesig ! Merged. Glad to enable that tools use-case.

I tweaked the commit message slightly so "prettier issue NNNN" becomes "prettier/prettier#NNNN", just to save the reader having to look up the exact location of Prettier's issue tracker.

That was thoughtful and intentional on my part; I'd wanted to avoid spamming the Prettier bug-tracker with additional backtrack-notices. 😞

@gnprice
Copy link
Member

gnprice commented Mar 27, 2020

I'd wanted to avoid spamming the Prettier bug-tracker with additional backtrack-notices.

I see. Looking at those two particular Prettier issue threads now, they look fine to me. For the one that's still open, I think the trackback link is actually helpful -- it provides some signal "hey, people are running into this bug".

Certainly there can be cases where it's annoying, though. When you have the goal of defeating that GitHub feature, I'd still appreciate saving the reader from having to look up the exact location of some other project's tracker. One way to do so would be to write something like https:// github.com/prettier/prettier/issues/12345, with a mangled but readable link. Another would be like "prettier/prettier issue 12345".

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.

None yet

2 participants