-
-
Notifications
You must be signed in to change notification settings - Fork 204
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
Add new rule template-no-let-reference
#1939
Conversation
lib/parsers/gjs-parser.js
Outdated
if (!scope) { | ||
return { scope: findParentScope(scopeManager, nodePath) }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needed to fix the current scope, as the references should be added in the scope where the references are and not where the variable definition is
lib/parsers/gjs-parser.js
Outdated
@@ -399,6 +386,20 @@ function convertAst(result, preprocessedResult, visitorKeys) { | |||
registerNodeInScope(node, scope, variable); | |||
} | |||
} | |||
|
|||
if ('blockParams' in node) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needed to move this down. this creates the scope for elementnodes and mustache blocks.
it should do this after creating the references, as otherwise the current scope would be the element or mustache block itself
lib/recommended-rules.js
Outdated
"ember/use-ember-data-rfc-395-imports": "error", | ||
"ember/template-no-let-reference": "error" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The set of recommended rules should only be updated during a major release. If you like, we can enable it as recommended at that time.
ecd1507
to
d7e52d8
Compare
needs rebase it looks like |
d7e52d8
to
5ff6570
Compare
} | ||
|
||
export default <template>{{foo}}</template>; | ||
```` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an extra tick.
lib/recommended-rules.js
Outdated
@@ -73,6 +73,7 @@ module.exports = { | |||
"ember/require-tagless-components": "error", | |||
"ember/require-valid-css-selector-in-test-helpers": "error", | |||
"ember/routes-segments-snake-case": "error", | |||
"ember/template-no-let-reference": "error", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have a separate PR to enable any rules as recommended? It's more clear what's going on then and will be better for the resulting changelog. Normally, we never create rules at the same time as enabling them as recommended.
5ff6570
to
b5ab4aa
Compare
b5ab4aa
to
2d10fe0
Compare
template-no-let-reference
resolves #1897