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: version 2.8.1 removes trailing comma on save #847

Open
jonyo opened this issue Mar 29, 2024 · 6 comments
Open

🐛 BUG: version 2.8.1 removes trailing comma on save #847

jonyo opened this issue Mar 29, 2024 · 6 comments
Labels
good first issue Good for newcomers needs investigation The cause of the issue is unknown and a deeper investigation is needed to fix it

Comments

@jonyo
Copy link

jonyo commented Mar 29, 2024

Describe the Bug

When I save an astro file, it removes trailing commas from multi-line function calls.

If I downgrade to version 2.8.0 and save, it adds them back again when I save.

I wonder if it is also related to #823 - I found another issue that sounded like it was similarly affected, #829 - though the specific fix for that one did not address this one.

Steps to Reproduce

  1. Open any .astro file with multi-line function calls. E.g.
    foo(
      var1,
      var2,
    );
  2. save the file
  3. trailing commas are removed. For the above example it would change to:
    foo(
      var1,
      var2
    );

EDIT: After comments, here is a better example, since the above ends up on a single line which does not have trailing comma:

  someReallyLongNAmeThingThatIsReallyLong(
    someReallyLongVarThatISLongAnd,
    SomeOtherReallyLongThing,
  );
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Mar 29, 2024
@Princesseuh
Copy link
Member

This is controlled by your Prettier settings, we don't really set something there. It's possible that the default Prettier setting changed, not sure.

@jonyo
Copy link
Author

jonyo commented Apr 1, 2024

This is controlled by your Prettier settings, we don't really set something there. It's possible that the default Prettier setting changed, not sure.

The prettier version did not change on the app I'm seeing this on, not for a while. And looking at the prettier docs, it did change, see the docs - in version 3 they went from es5 to all, so if anything the opposite should have happened (it would have added more commas, not taken them away).

Like I said, it works fine if I downgrade the vscode Astro extension to version 2.8.0, and I found this isn't the only thing that was affected: I found that other bug #829 also affected by upgrading volar-service-prettier internally, that one was already fixed.

@Princesseuh
Copy link
Member

Princesseuh commented Apr 1, 2024

That other issue isn't related to volar-service-prettier, it was related to another dep upgrade that was done in the same patch that wasn't supposed to have an effect.

Do you set any Prettier config anywhere? Or is this using completely default config? Using the Prettier playground, the snippet you shared always get its last trailling comma removed when using the default settings

@jonyo
Copy link
Author

jonyo commented Apr 1, 2024

Using the Prettier playground, the snippet you shared always get its last trailling comma removed when using the default settings

Ah, yeah my example is too short, it ends up combining it to a single line foo(var1, var2) and it does not use trailing comma in that case.

If you use something that is long enough to go multi-line, like this:

  someReallyLongNAmeThingThatIsReallyLong(
    someReallyLongVarThatISLongAnd,
    SomeOtherReallyLongThing,
  );

You'll see it does add the last comma in the default prettier playground here.

@jonyo
Copy link
Author

jonyo commented Apr 1, 2024

Just to see what happened, I specifically set trailingComma: always in the prettier config in my project. Upgraded the Astro extension to the latest, reloaded the window, saved an astro file, it still removed the trailing comma on all multi-line function calls. Downgraded to 2.8.0 and save, it added them back.

@jonyo
Copy link
Author

jonyo commented Apr 1, 2024

Sorry I forgot to answer your question:

Do you set any Prettier config anywhere? Or is this using completely default config?

We set singleQuote: true and it also inherits settings from our .editorconfig file, but we do not set that setting specifically. As mentioned, I tried it just now and it still had no effect.

@Princesseuh Princesseuh added needs investigation The cause of the issue is unknown and a deeper investigation is needed to fix it good first issue Good for newcomers and removed needs triage Issue needs to be triaged labels Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers needs investigation The cause of the issue is unknown and a deeper investigation is needed to fix it
Projects
None yet
Development

No branches or pull requests

2 participants