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

<template> tag + eslint-plugin-sort-class-members (and possibly other plugins) #1896

Open
chancancode opened this issue Jun 20, 2023 · 1 comment

Comments

@chancancode
Copy link

This may not end up being a thing that needs to be handled in eslint-plugin-ember itself, but it may, and in any case it seems like a good place to have the discussion.

The issue I ran into is with an external plugin eslint-plugin-sort-class-members. I found it because it's used by Discourse, but it's not specific to Discourse at all, and seems somewhat popular outside of Discourse, and seems like the kind of plugin that we would expect people to be able to use with .gjs files. I also think the issue is probably more general.

I ran into are two issues.

The first issue is that their default error message leaks the internal __GLIMMER_TEMPLATE:

  3:3  error  Expected property __GLIMMER_TEMPLATE to come before method bar  sort-class-members/sort-class-members

The other issue is that if you try to relocate the template tag with --fix, it will emit [__GLIMMER_TEMPLATE(...)] to the moved location rather than <template>.

It seems like this would be a problem in general for any rules that operate over the range of the template tag. Ideally, we would find some ways to fix this generally rather than something specific to the eslint-plugin-sort-class-members plugin, though I don't know enough about the eslint architecture to know what is possible.

Perhaps this is related to the idea of representing the template tag as a custom AST node? Not sure if that's actually going to help or make things worse (hard errors on the unknown AST node) with plugins in general.

chancancode added a commit to tildeio/eslint-config-discourse that referenced this issue Jun 20, 2023
See the comments and the linked issues for additional context.

I arbitrarily decided to put the template tag at the top of the
class but happy to change that to whatever seems reasonable.

ember-cli/eslint-plugin-ember#1895
ember-cli/eslint-plugin-ember#1896
CvX pushed a commit to discourse/lint-configs that referenced this issue Jun 21, 2023
See the comments and the linked issues for additional context.

I arbitrarily decided to put the template tag at the top of the
class but happy to change that to whatever seems reasonable.

ember-cli/eslint-plugin-ember#1895
ember-cli/eslint-plugin-ember#1896
@patricklx
Copy link
Contributor

patricklx commented Aug 22, 2023

@bmish this now works with the new parser strategy. though it just leaves the template where it is

CvX added a commit to discourse/lint-configs that referenced this issue Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants