Closed NullVoxPopuli closed 1 year ago
Thanks for the report.
Does running Prettier directly work properly?
I suspect this is because your project already has https://github.com/ember-template-lint/ember-template-lint-plugin-prettier installed, which apparently implements its own undocumented(?) prettier-for-template-tag support. Looking into a solution.
Does running Prettier directly work properly?
it does!
because your project already has
hmmm if I comment out that plugin in my template-lint config, I still get the errors:
It turns out it's not related to the ember-template-lint-plugin-prettier at all.
HOWEVER: That plugin is fully redundant with this one for gjs/gts files. (Except that this one will also format the js portion of your file whereas that one will only format the template portion.)
I am working on a minimal repro currently.
Seems like there should be some escaping happening here: https://github.com/ember-cli/eslint-plugin-ember/blob/db745a3881686d50e486be67d6851d806ac770d6/lib/preprocessors/glimmer.js#L120
It's generating new RegExp('\b[__GLIMMER_\b')
which indeed has an opening [
, which the RegExp constructor assumes is the beginning of a character set.
~Now going to figure out if/why this plugin causes this to happen in that one.~
Because eslint-plugin-prettier
runs Prettier as an ESLint rule, this plugin causes the Prettier ESLint rule to log the following violation message:
{
ruleId: "prettier/prettier",
severity: 2,
message: "Replace `[__GLIMMER_TEMPLATE(`⏎··<h1>···Hello·{{@who}}.·</h1>⏎`,·{·strictMode:·true·})];` with `<template><h1>·Hello·{{@who}}.·</h1></template>`",
line: 1,
column: 16,
nodeType: null,
messageId: "replace",
endLine: 3,
endColumn: 27,
}
Interestingly, the message
has the preprocessed [__GLIMMER_TEMPLATE(
shenanigans when it probably should not. (Not sure if that's up to this plugin or the ember-plugin-prettier plugin to fix that yet).
I'm having quite a journey on this one:
And resulted into a debugging session this morning. I think I've come to the root cause of this.
When files processed by eslint, they run through the given parser (most likely @babel/eslint-parser
or @typescript-eslint/parser
). As they already parse <template>
tag into [__GLIMMER_TEMPLATE(...)]
to make the code understood to eslint and all the eslint plugins as they operate on plain js. Now, this is also the code that is then passed down into prettier.
Here is my original code:
import Component from '@glimmer/component';
export interface GreetingSignature {
Element: HTMLSpanElement;
Args: {
hello: string;
to: string;
};
}
export default class Greeting extends Component<GreetingSignature> {
get to() {
return `${this.args.to[0]?.toUpperCase()}${this.args.to.slice(1).toLowerCase()}`;
}
<template>
<span ...attributes>{{@hello}} to {{this.to}}</span>
</template>
}
which, when it comes to prettier through eslint is looking like so:
import Component from '@glimmer/component';
export interface GreetingSignature {
Element: HTMLSpanElement;
Args: {
hello: string;
to: string;
};
}
export default class Greeting extends Component<GreetingSignature> {
get to() {
return `${this.args.to[0]?.toUpperCase()}${this.args.to.slice(1).toLowerCase()}`;
}
[__GLIMMER_TEMPLATE(`
<span ...attributes>{{@hello}} to {{this.to}}</span>
`, { strictMode: true })]
}
which results in this error message:
/Users/thomas/code/gossi/frontend-configs/testing/ember-addon-v2-gts/src/components/greeting.gts
16:24 error Replace `⏎····<span·...attributes>{{@hello}}·to·{{this.to}}</span>⏎··` with `<span·...attributes>{{@hello}}·to·{{this.to}}</span>` prettier/prettier
I would see the problem in the leading and trailing whitespace within the template literal as first param to ___GLIMMER_TEMPLATE()
. Perhaps some special treatment here?
I wonder if we need to omit that block surrounded by prettier-disable/prettier-enable directive comments?
I don't think so. As this input:
[__GLIMMER_TEMPLATE(`
<span ...attributes>{{@hello}} to {{this.to}}</span>
`, { strictMode: true })]
when run through pretter turns into:
[__GLIMMER_TEMPLATE(
`
<span ...attributes>{{@hello}} to {{this.to}}</span>
`,
{ strictMode: true }
)];
wrapping this in prettier-disable/prettier-enable would keep the [__GLIMMERT_TEMPLATE()]
as is. We are interested about the contents within the template literal, which prettier would keep intact.
My assumption here is, that when the parsed code reaches the plugin, it is unsure whether this is trimmed or not (depending on what happened during eslint or during parsing already). Trimming or keeping this by default can be understood as normalization, whichever seems to be the best sensible default for this plugin. If this deviates from the expectations, would might need an extract parameter to control that behavior from the outside.
maybe -- looks like we'd want to debug around here: https://github.com/gitKrystan/prettier-plugin-ember-template-tag/blob/main/src/parse.ts#L162 To see if we're missing information when trying to run prettier through eslint, vs running prettier standalone
@gossi
Based on the prettier/prettier
error message you are seeing, I think it's telling you to replace:
<template>
<span ...attributes>{{@hello}} to {{this.to}}</span>
</template>
with
<template><span ...attributes>{{@hello}} to {{this.to}}</span></template>
Can you confirm that moving the <template>
tags to the same line resolves the linter issue?
This is, however, unexpected behavior for top-level class templates in the current version of this plugin: https://github.com/gitKrystan/prettier-plugin-ember-template-tag#new-lines
Which version are you using?
I suspect this all comes down to https://github.com/ember-cli/eslint-plugin-ember/issues/1659 and that the fix will be more related to how eslint-plugin-ember reports the mismatch.
I'm on the most recent version v0.3.2
(and eslint-plugin-ember@11.5.2
)
Yes, exactly as you are describing the issue in the linked https://github.com/ember-cli/eslint-plugin-ember/issues/1659 (wasn't aware about that one).
As you pointed there the \\b${escapeRegExp(token)}\\b
regex perhaps needs some special treatment over at eslint-plugin-ember
Hey y'all. I heard a rumor that upgrading to the latest versions of this plugin + eslint-plugin-ember should resolve this issue.
Can you let me know how it goes? If everything looks good, I will close this issue. 🎉
Still seeing this issue using "eslint-plugin-ember: "^11.10.0"
& "prettier-plugin-ember-template-tag": "^0.3.2"
.
Can anyone confirm that it was fixed previously and is just broken again or if it was never actually fixed?
@HeroicEric I responded here: https://github.com/gitKrystan/prettier-plugin-ember-template-tag/issues/81#issuecomment-1650560485
I am marking this issue as fixed, but there is a bug with the fix as mentioned in #81
Closing because this issue is mostly resolved, but there is still a small hiccup as described in #81
I just updated to 1.0.0 from 0.3.2 to try to fix the linting errors outlined here with <template>
and now it is prompting me to completely delete all content in my .gjs
components. (and npm run lint:fix
actually did delete all content)
Same, when prettier runs as part of the linting, entire content of gjs file is deleted.
After finding the issue https://github.com/gitKrystan/prettier-plugin-ember-template-tag/issues/61, I was using prettier 2.8.8. Updating to prettier 3.0.0 looks like it fixed the issue.
@cah-brian-gantzler do you mind sharing the versions of related packages? I upgraded prettier and now getting errors like
Oops! Something went wrong! :(
ESLint: 7.32.0
TypeError: prettier.resolveConfig.sync is not a function
Tried to include anything that might be relevant. Let me know if you need any others
"@babel/eslint-parser": "^7.22.6",
"ember-template-imports": "^3.4.2",
"ember-template-lint": "^5.11.0",
"eslint": "^8.44.0",
"eslint-config-prettier": "^8.8.0",
"eslint-plugin-ember": "^11.9.0",
"eslint-plugin-node": "^11.1.0",
"eslint-plugin-prettier": "^4.2.1",
"eslint-plugin-qunit": "^7.3.4",
"prettier": "^3.0.0",
"prettier-plugin-ember-template-tag": "^1.0.0",
It's just running into more and more errors. I'll have to keep playing with it. I think related to this https://github.com/ember-cli/ember-cli/pull/10142/files and then other prettier related errors are coming into it now.
@cah-brian-gantzler I got it all working, thanks for the help.
However, ember-template-lint
is still trying to force my components to start on the same line as the initial <template>
tag which I thought was part of what this was fixing
So prettier will auto fix
const AgeFilter = <template><Text @type='body-s' @color='secondaryText' @spacing='pt-3'>....</Text></template>
to
const AgeFilter = <template>
<Text @type='body-s' @color='secondaryText' @spacing='pt-3'>....</Text>
</template>
But then ember template lint will complain and force the first component to always be on the same line as <template>
Here's current relevent versions in package.json
"eslint": "^8.44.0",
"ember-template-lint": "^5.11.0",
"ember-template-lint-plugin-prettier": "^5.0.0",
"prettier": "^3.0.0",
"prettier-plugin-ember-template-tag": "^1.0.0",
"eslint-config-prettier": "^8.8.0",
"eslint-plugin-ember": "^11.9.0",
"eslint-plugin-node": "^11.1.0",
"eslint-plugin-prettier": "^5.0.0",
"eslint-plugin-qunit": "^7.2.0",
The problem I had was prettier was completely blanking the file. Which is fixed.
Just started using template and all mine are local to the class, so dont assign it to a variable. When I get to multiple templates in the same file will let you know what happens
@jgadbois I believe there are conflicting or redundant Prettier rules when using prettier-plugin-ember-template-tag
and ember-template-lint-plugin-prettier
. Try adding this to .template-lintrc.js
:
module.exports = {
...
overrides: [
{
files: ['**/*.gjs'],
rules: {
prettier: false,
},
},
],
};
@charlesfries Thanks - that does remove the errors, but then npm run lint:js
and npm run lint:hbs
don't do any linting of <template>
formatting at all.
🐞 Describe the Bug
A clear and concise description of what the bug is.
Ran in to this error:
🔬 Minimal Reproduction
https://github.com/NullVoxPopuli/limber/pull/545
😕 Actual Behavior
the above error
🤔 Expected Behavior
no error
🌍 Environment
➕ Additional Context
Add any other context about the problem here.