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

Check template tags for service usages in no-unused-services #1899

Merged
merged 3 commits into from Jun 26, 2023

Conversation

lin-ll
Copy link
Collaborator

@lin-ll lin-ll commented Jun 26, 2023

With template imports, we need to check template tags for usages of services. Otherwise, no-unused-services will have false positives.

I brought it ember-template-recast to parse the AST for the template tag. Is this how we want to do this moving forward? If not, suggestions are welcome.

@lin-ll lin-ll self-assigned this Jun 26, 2023
@lin-ll
Copy link
Collaborator Author

lin-ll commented Jun 26, 2023

@bmish bug fix or enhancement? 🤔

@lin-ll
Copy link
Collaborator Author

lin-ll commented Jun 26, 2023

@chrisrng fyi

Copy link
Member

@bmish bmish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Since this fixes false positive, I might consider it a bug fix, but enhancement is fine too as it could also be considered to be "adding support for template tags".

@@ -170,7 +172,22 @@ module.exports = {
return;
}

if (
if (node.callee.name === TEMPLATE_TAG_PLACEHOLDER) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make this into a common util? Seems like something most rules will start needing to know about

(Can always do it when the 2nd use case shows up)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I'll wait for the next case to show up -- I'm sure there are things with the recast parsing that can be extracted out cleverly too, but I think it would be helpful to see some more usages of it first in order to know what common patterns people use.

@lin-ll lin-ll merged commit 684410c into ember-cli:master Jun 26, 2023
8 checks passed
@lin-ll lin-ll deleted the lulin/unused-services branch June 26, 2023 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants