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

Explain ASI issues aren't always auto-fixed #14174

Merged
merged 2 commits into from Feb 18, 2023
Merged

Explain ASI issues aren't always auto-fixed #14174

merged 2 commits into from Feb 18, 2023

Conversation

theScottyJam
Copy link
Contributor

Add a note about how Prettier won't auto-fix all ASI issues.

This is being done to help remove confusion in the community about how Prettier deals with ASI hazards.

Description

I've noticed that a lot of developers feel that they can run with a no-semicolon style of programming by simply throwing in prettier and not bothering to actually learn the rules behind ASI. This isn't entirely true. Prettier can help fix some issues before they become a problem, but if you write code that already contains an ASI-related bug, Prettier won't auto-fix that bug for you. While it's true that Prettier will still reformat ASI-related buggy code in such a way as to make it more obvious that there was a bug there, developers aren't generally in the habit of double-checking how Prettier is reformatting their code.

The way the documentation is currently written, it almost sounds like Prettier is backing up this false claim that you can just throw Prettier into your project and never worry about ASI hazards again. This PR updates the docs with a note that attempts to clarify Prettier's limitations in regard to ASI.

I did my best to try and word this in a clear way, but feel free to edit the PR or suggest a clearer, or more concise way of describing these limitations. I'm not overly picky about how it gets presented, I just feel that it's important to present Prettier's limitations in some form or another, so developers have a more accurate picture of what Prettier can and can not do when it comes to handling ASI.

Checklist

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
  • (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
  • I’ve read the contributing guidelines.

Try the playground for this PR

Add a note about how Prettier won't auto-fix all ASI issues.

This is being done to help remove confusion in the community about how Prettier deals with ASI hazards.
@fisker
Copy link
Member

fisker commented Jan 14, 2023

I don't think we can know such case is bug or not.

There is nothing to explain, we respect whatever the AST is.

@theScottyJam
Copy link
Contributor Author

theScottyJam commented Jan 15, 2023

I don't think we can know such case is bug or not.

Correct, which is why Prettier doesn't try to auto-fix it, nor should it. I don't think Prettier is doing anything wrong with how it's implemented.

There is nothing to explain, we respect whatever the AST is.

Then explain that - that Prettier respects whatever the AST is. There's currently nothing that's explicitly stating this, beyond the section talking about how Prettier won't change the behavior of the code it is formatting.

If I were to give a high-level summary of how this semicolon section of the documentation is currently written, it would go something like this:

Prettier can auto-prefix specific lines with semicolons.
Here's an example of a mistake that can happen if a line isn't auto-prefixed with a semicolon.
When a semicolon is in front of characters like [ "such issues never happen". You can move lines around "without having to think about ASI rules". (quoted text comes directly from the doc page).

In even shorter words: "Prettier does X. When X isn't done, undesirable Y happens. When X is done, you never have to worry about Y".

Or, in even shorter words: "Prettier does X, and X fixes undesirable Y".

It's no wonder people get confused and think that "Prettier prevents Y". The docs never technically say that, but the way this whole section is structured, it's incorrectly being implied to some degree (would you disagree?).

To demonstrate that there's actual real-world confusion coming from this docs page, take a peek at this recent Reddit thread. The O.P. was asking if you should put a semicolon at the end of each line in JavaScript. As you would expect, there's many answers, all over the place. In there, I saw two different people independently state that you can just use Prettier to program safely without semicolons, one of which linked to this exact docs page to back them up.

In the end, the decision is the Prettier's team's to make, but this would be a good way to help fight some of this misinformation floating around.

Copy link
Member

@thorn0 thorn0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this extra explanation makes the docs a little better.

@theScottyJam
Copy link
Contributor Author

I see it's been approved - just wondering what the next step is.

@fisker
Copy link
Member

fisker commented Feb 18, 2023

Close and reopen to trigger CI

@fisker fisker closed this Feb 18, 2023
@fisker fisker reopened this Feb 18, 2023
@fisker fisker merged commit 2a5b391 into prettier:main Feb 18, 2023
@fisker
Copy link
Member

fisker commented Feb 18, 2023

Thank you for contributing to Prettier! ❤️

@theScottyJam theScottyJam deleted the patch-1 branch February 18, 2023 16:57
fisker added a commit to seiyab/prettier that referenced this pull request Feb 23, 2023
Co-authored-by: fisker Cheung <lionkay@gmail.com>
medikoo pushed a commit to medikoo/prettier-elastic that referenced this pull request Jan 4, 2024
Co-authored-by: fisker Cheung <lionkay@gmail.com>
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.

None yet

3 participants