gitKrystan / prettier-plugin-ember-template-tag

A prettier plugin for formatting Ember template tags
MIT License
22 stars 12 forks source link

[Bug] Invalid formatting of `render` function in tests when running prettier via eslint-plugin-ember + eslint-plugin-prettier #81

Closed HeroicEric closed 3 months ago

HeroicEric commented 11 months ago

šŸž Describe the Bug

Prettier is trying to format tests written as .gts in a way that is invalid. I'm not sure if I have something configured incorrectly or if this is a bug with the library but I figured I'd add this here in case anyone else runs into this before I can get an example repo set up to confirm.

Edit: It seems this is related to https://github.com/gitKrystan/prettier-plugin-ember-template-tag/issues/20 and is likely a bug that is from using eslint-plugin-prettier.

Using prettier directly results in the correct formatting but using prettier through eslint-plugin-prettier does not. eslint-plugin-prettier also reports some somewhat confusing errors but they do go away if things are formatted correctly using prettier directly.

šŸ”¬ Minimal Reproduction

See examples below. I'll try to get an example repo set up and add a link here.

šŸ˜• Actual Behavior

Prettier wants render in my test formatted as:

await render([
  <template>
    <Button @onActivate={{onActivate}}>
      Click me!
    </Button>
  </template>
]);

šŸ¤” Expected Behavior

Prettier should format call to render as:

await render(<template>
  <Button @onActivate={{onActivate}}>
    Click me!
  </Button>
</template>);

Which causes the following error:

TypeError: (0 , _util.unwrapTemplate)(...).asLayout is not a function
    at new OutletComponentDefinition (http://localhost:4200/assets/vendor.js:4343:152)
    at http://localhost:4200/assets/vendor.js:5795:76
    at http://localhost:4200/assets/vendor.js:31666:37
    at track (http://localhost:4200/assets/vendor.js:39488:7)
    at valueForRef (http://localhost:4200/assets/vendor.js:31665:44)
    at Assert.evaluate (http://localhost:4200/assets/vendor.js:34016:48)
    at UpdatingVMImpl._execute (http://localhost:4200/assets/vendor.js:36184:16)
    at http://localhost:4200/assets/vendor.js:36158:63
    at runInTrackingTransaction (http://localhost:4200/assets/vendor.js:38991:21)
    at UpdatingVMImpl.execute (http://localhost:4200/assets/vendor.js:36158:51)

šŸŒ Environment

{
  "@glint/core": "^1.0.2",
  "@glint/environment-ember-loose": "^1.0.2",
  "@glint/environment-ember-template-imports": "^1.0.2",
  "@glint/template": "^1.0.2",
  "ember-template-imports": "^3.4.2",
  "eslint-plugin-ember": "^11.10.0",
  "eslint-plugin-prettier": "^4.2.1",
  "prettier": "^2.8.8",
  "prettier-plugin-ember-template-tag": "^0.3.2",
}

āž• Additional Context

Screenshot 2023-07-24 at 7 59 36 PM
HeroicEric commented 11 months ago

Upon looking further into this it appears to be the same issue as https://github.com/gitKrystan/prettier-plugin-ember-template-tag/issues/20

gitKrystan commented 11 months ago

I agree that this is an issue with the eslint-plugin-ember/eslint-plugin-prettier integration, related to https://github.com/gitKrystan/prettier-plugin-ember-template-tag/issues/20. If it were exactly the same issue, however, we'd see Insert [__GLIMMER_TEMPLATE( and not just Insert [. I think what is happening is that https://github.com/gitKrystan/prettier-plugin-ember-template-tag/issues/20 is fixed via the fix for https://github.com/ember-cli/eslint-plugin-ember/issues/1659, but there is a bug in the fix:

I think these regexes incorrectly assume that the intermediate [__GLIMMER_TEMPLATE(...)] form of template tag always occurs on one-line. https://github.com/ember-cli/eslint-plugin-ember/blob/900e0026a1f23313108549b0ea5d6240c7c85c0c/lib/preprocessors/glimmer.js#L18-L21

HeroicEric commented 11 months ago

@gitKrystan FWIW I do see the Insert [__GLIMMER_TEMPLATE( stuff if I move things around a little bit.

For example, adding an extra space:

Screenshot 2023-07-25 at 5 09 19 PM
gitKrystan commented 11 months ago

Interesting! I still suspect that regex is related somehow.

I forgot to mention that I added tests for this after you opened this issue (https://github.com/gitKrystan/prettier-plugin-ember-template-tag/pull/83) bc it's definitely something this plugin should avoid breaking.

Can you confirm my suspicion that this only happens when you run prettier via eslint-plugin-ember/eslint-plugin-prettier and not when you run prettier directly? If so, we should move this conversation to https://github.com/ember-cli/eslint-plugin-ember/issues/1659 or a new issue in that repo so that the correct people see it.

HeroicEric commented 11 months ago

Can you confirm my suspicion that this only happens when you run prettier via eslint-plugin-ember/eslint-plugin-prettier and not when you run prettier directly? If so, we should move this conversation to ember-cli/eslint-plugin-ember#1659 or a new issue in that repo so that the correct people see it.

yes that is the case. Running prettier directly does the right thing

HeroicEric commented 11 months ago

After investigating a little bit more it seems using prettier-plugin-ember-template-tag through eslint via eslint-plugin-prettier is more broken than I had initially realized.

For example, the regular TypeScript code in a .gts file is linted/formatted correctly but the content inside the <template> tags does not seem to be linted or formatted at all.

Screenshot 2023-07-26 at 10 10 17 AM

Note: My editor is only configured to show ESLint errors in the screenshot above

Again, running prettier directly with yarn prettier ./app/components/button.gts does still do the right thing:

import Component from '@glimmer/component';
import { on } from '@ember/modifier';

interface Signature {
  Element: HTMLButtonElement;
  Args: {
    isDisabled?: boolean;
    onActivate?: () => void;
  };
  Blocks: {
    default: [];
  };
}

export default class ButtonComponent extends Component<Signature> {
  get isDisabled() {
    return this.args.isDisabled ?? false;
  }

  handleClick = () => this.args.onActivate?.();

  <template>
    <button disabled={{this.isDisabled}} type='button' {{on 'click' this.handleClick}}>
      {{yield}}
    </button>
  </template>
}

declare module '@glint/environment-ember-loose/registry' {
  export default interface Registry {
    Button: typeof ButtonComponent;
  }
}
gossi commented 11 months ago

It does however work with inline comments (which was the problem earlier). So this my test line of my config:

await render(<template><Greeting @hello="hi" @to="me"/></template>);

which stays in tact after all. But when I ripped it apart:

await render(
  <template>
    <Greeting @hello="hi" @to="me"/>
  </template>
);

it turned into, only the opening [ (see the entire code for this file at the end):

    await render([
  <template>
    <Greeting @hello="hi" @to="me"/>
  </template>
);

When this is indented:

    await render(
      <template>
        <Greeting @hello="hi" @to="me"/>
      </template>
    );

will be formatted as above:

    await render([
      <template>
        <Greeting @hello="hi" @to="me"/>
      </template>
    ]);

entire code:

import { render } from '@ember/test-helpers';
import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';

import Greeting from 'ember-app-gts/components/greeting';

module('Rendering | <Button>', function (hooks) {
  setupRenderingTest(hooks);

  test('it renders with defaults', async function (assert) {
    await render(
      <template>
        <Greeting @hello="hi" @to="me"/>
      </template>
    );

    assert.dom().hasText('hi to me');
  });
});

apparently the indentation has an effect on this.

BoussonKarel commented 4 months ago

I fixed it in my app by updating eslint-plugin-ember and adding the right config: https://github.com/ember-cli/eslint-plugin-ember?tab=readme-ov-file#gtsgjs