ember-tooling / 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 7 months ago

HeroicEric commented 1 year 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 1 year 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 1 year 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 1 year 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 1 year 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 1 year 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 1 year 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 1 year 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 7 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