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

[Bug] Required semicolons incorrectly removed (semi: false config) #218

Open
Techn1x opened this issue Jan 2, 2024 · 4 comments
Open

[Bug] Required semicolons incorrectly removed (semi: false config) #218

Techn1x opened this issue Jan 2, 2024 · 4 comments

Comments

@Techn1x
Copy link

Techn1x commented Jan 2, 2024

🐞 Describe the Bug

I have semi: false in my prettier config. In general this works, but a semicolon is required between certain statements in a class and the <template> tag. This plugin seems to remove those required semicolons when it shouldn't. This then causes glint errors.

Example where the semicolon is required - prettier removes it, when it shouldn't.

class MyComponent extends Component {
  historyBack = () => history.back();

  <template>
    <button type="button" {{on "click" historyBack}}>Go back</button>
  </template>
}

🔬 Minimal Reproduction

Added failing test snapshot
#219

😕 Actual Behavior

  • use example code
  • run prettier --write .
  • semicolon is removed

(resulting in glint errors)
Screenshot 2024-01-02 at 8 18 37 pm

🤔 Expected Behavior

  • use example code
  • run prettier --write .
  • semicolon should not be removed

🌍 Environment

  • prettier-plugin-ember-template-tag version: 2.0.0
  • ember-template-imports version (if applicable): 4.0.0
  • content-tag version (if applicable): 1.2.2
  • eslint-plugin-ember version (if applicable): 11.12.0

➕ Additional Context

I was having this issue before v2.0.0 as well, so it's not a regression.

A workaround is to disable prettier for the line that the semicolon is on, exactly like this

historyBack = () => history.back(); /* prettier-ignore */ /* prettier-ignore-end */

<template>...

That looks really weird, but it "works" - I believe the weirdness is because I don't think this addon parses prettier-ignore directives correctly?
The prettier-ignore-end is important so that prettier is reenabled for the lines afterward

@gitKrystan
Copy link
Owner

Thanks for the report!

@patricklx
Copy link
Contributor

patricklx commented Mar 15, 2024

this will probably solved when glint moves to content-tag instead of ember-template-imports.

typed-ember/glint#655

@gitKrystan gitKrystan changed the title [Bug] Required semicolons incorrectly removed [Bug] Required semicolons incorrectly removed (semi: false config) May 23, 2024
@Techn1x
Copy link
Author

Techn1x commented Jun 12, 2024

this will probably solved when glint moves to content-tag instead of ember-template-imports.

Unfortunately I still seem to be having this issue (I'm on glint 1.4.0)

This repo has the issue replicated (see the failing test I wrote) but does not depend on glint, so I am not sure if glint version matters, but I am no expert here


A workaround is to disable prettier for the line that the semicolon is on

I want to mention a caveat to the workaround I gave in the description, is that if applied to anything more complex it disables prettier for that entire block

class Abc extends Component {
  fetchStats = trackedFunction(this, async () => {
    // all of this inside the function is ignored by prettier
  }); /* prettier-ignore */ /* prettier-ignore-end */

  <template> ...

@patricklx
Copy link
Contributor

@Techn1x is the issue about glint or prettier removing semicolon?
It's expected now to be without semi, when configured so. And newest glint should be able to parse it

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

No branches or pull requests

3 participants