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: pugClassNotation: 'attribute' outputs junk if class literals are after attributes #508

Closed
1 of 7 tasks
MrHaila opened this issue Sep 8, 2024 · 3 comments · Fixed by #509
Closed
1 of 7 tasks

Comments

@MrHaila
Copy link
Contributor

MrHaila commented Sep 8, 2024

Plugin Version

3.1.0

Prettier Version

3.3.3

Which frameworks are affected?

  • none
  • vue
  • angular
  • svelte

Node Version

20.17.0

Which operating systems have you used?

  • Linux
  • macOS
  • Windows

Prettier config

{
  pugClassNotation: 'attribute'
}

Input

div.foo
div(attribute="value").foo
div(attribute="value" class="bar").foo
div(attribute="value" class="bar").foo.baz
div(
  attribute="value"
  class="bar"
).foo.baz
div.foo(attribute="value" class="bar").baz
div: span.foo
div: span(class="foo")
div.foo: span.bar(class="baz").foo
div.foo(class="bar").baz: h1.foo(class="bar").baz Let's really #[span.foo(class="bar").baz get nasty]!

Output or Error

(class="foo")
div(attribute="value")div(class="foo")
div(attribute="value", class="bar")div(class="foo")
div(attribute="value", class="bar")div(class="foo baz")
div(attribute="value", class="bar")div(class="foo baz")
(attribute="value", class="foo bar")div(class="baz")
div: span(class="foo") // <- this line was correct!
div: span(class="foo") // <- this line was correct!
div: span(class="foo")
: span(class="foo bar baz")div(class="foo")
(class="foo bar")div: h1(class="baz foo bar")div(class="baz") Let's really #[span(class="foo bar")div(class="baz") get nasty]!

Expected Output

div(class="foo")
div(attribute="value", class="foo")
div(attribute="value")
div(attribute="value", class="bar foo")
div(attribute="value", class="bar foo baz")
div(attribute="value", class="bar foo baz")
div(attribute="value", class="bar foo baz")
div: span(class="foo")
div: span(class="foo")
div(class="foo"): span(class="bar baz foo")
div(class="foo bar baz"): h1(class="foo bar baz") Let's really #[span(class="foo bar baz") get nasty]!

Additional Context

My workaround for this issue was to first use the option that refactors all class literals to the beginning and only then enable this class attribute option.

This is easy to repro by expanding the pugClassNotation test with the above code.

The offending code is easy to spot at the beginning of the private class function in printer.ts, but it's not immediately clear to me why that code works in the first place (do multiple chained attributes get merged by some follow-up logic?) or how to fix it properly. I'll open a PR if I can puzzle it out 🤷‍♂️

My verbosely commented version of the code that needs fixing:

    // Special handling for class literals when using the attribute notation.
    if (this.options.pugClassNotation === 'attribute') {
      // Add the class to the list of class literals to be converted to attributes.
      this.classLiteralsToBeConvertedToAttributes.push(token.val);

      // TODO: This can print junk (bug) if the class literal is after attributes. Investigate.
      if (
        this.previousToken?.type !== 'tag' &&
        this.previousToken?.type !== 'class'
      ) {
        this.result += `${this.computedIndent}div`;
      }

      // If this is the last class token (i.e. we are done with the literals)...
      // TODO: This probably does not handle inline elements chained with : correctly. Investigate.
      if (
        this.nextToken &&
        ['text', 'newline', 'indent', 'outdent', 'eos'].includes(
          this.nextToken.type,
        )
      ) {
        // Take a copy of the class literals to be converted to attributes and clear the list.
        const classes: string[] =
          this.classLiteralsToBeConvertedToAttributes.splice(
            0,
            this.classLiteralsToBeConvertedToAttributes.length,
          );

        // Print all the class literals as one class attribute.
        // TODO: What happens if the class attribute is already present? Investigate.
        this.result += `(class=${this.quoteString(classes.join(' '))})`;
        if (this.nextToken.type === 'text') {
          this.result += ' ';
        }
      }

      // Done processing the class token. Return early.
      return;
    }
@Shinigami92
Copy link
Member

@MrHaila
Copy link
Contributor Author

MrHaila commented Sep 8, 2024

I believe these are bugs in #203 that partially implemented #167 so not a direct duplicate.

Fully completing #167 would mean adding a new option. This issue highlights bugs in the already existing option that were not covered by tests.

@Shinigami92
Copy link
Member

You could either extend the tests in https://github.com/prettier/plugin-pug/tree/main/tests/options/pugClassNotation or add a new test in https://github.com/prettier/plugin-pug/tree/main/tests/issues in a new folder issue-508

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 a pull request may close this issue.

2 participants