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] Invalid formatting of render function in tests when running prettier via eslint-plugin-ember + eslint-plugin-prettier #81

Closed
HeroicEric opened this issue Jul 25, 2023 · 8 comments · Fixed by ember-cli/eslint-plugin-ember#1920 or ember-cli/eslint-plugin-ember#1942

Comments

@HeroicEric
Copy link

HeroicEric commented Jul 25, 2023

🐞 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 #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
Copy link
Author

Upon looking further into this it appears to be the same issue as #20

@gitKrystan
Copy link
Collaborator

I agree that this is an issue with the eslint-plugin-ember/eslint-plugin-prettier integration, related to #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 #20 is fixed via the fix for ember-cli/eslint-plugin-ember#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

@gitKrystan gitKrystan changed the title [Bug] Invalid formatting of render function in tests using .gts [Bug] Invalid formatting of render function in tests when running prettier via eslint-plugin-ember + eslint-plugin-prettier Jul 25, 2023
@HeroicEric
Copy link
Author

HeroicEric commented Jul 25, 2023

@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
Copy link
Collaborator

Interesting! I still suspect that regex is related somehow.

I forgot to mention that I added tests for this after you opened this issue (#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 ember-cli/eslint-plugin-ember#1659 or a new issue in that repo so that the correct people see it.

@HeroicEric
Copy link
Author

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
Copy link
Author

HeroicEric commented Jul 26, 2023

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
Copy link

gossi commented Jul 26, 2023

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants