AndreaPontrandolfo / sheriff

A comprehensive and opinionated Typescript-first ESLint configuration.
https://www.eslint-config-sheriff.dev
MIT License
106 stars 7 forks source link

Improve Astro support #130

Closed chalkygames123 closed 2 months ago

chalkygames123 commented 8 months ago

Hi,

Thank you so much for adding support for Astro in #45, and I'm sorry for the silence.

I just tried the latest Sheriff on my Astro projects and noticed a few issues.

  1. The default import of Astro components has to be named in Pascal case so it can be used as an HTML tag name (see: https://docs.astro.build/en/core-concepts/astro-syntax/#dynamic-tags). So, the @typescript-eslint/naming-convention rule needs to be updated like this:
         selector: "typeProperty",
         format: null,
       },
+      {
+        selector: "import",
+        format: ["camelCase", "PascalCase"],
+      },
     ],
   };
 };
  1. Astro's built-in components import specifier starts with 'astro:', so the import/no-unresolved rule needs to be updated like this:
   'import/namespace': 0,
   'import/default': 0,
   'import/no-named-as-default-member': 0,
-  'import/no-unresolved': [2, { commonjs: true, caseSensitiveStrict: true }],
+  'import/no-unresolved': [2, { commonjs: true, caseSensitiveStrict: true, ignore: ['^astro:.+'] }],
   'import/first': 2,
   'import/order': [2, { 'newlines-between': 'never' }],
   'import/no-default-export': 2,

I'm happy to send PRs if you want.

AndreaPontrandolfo commented 8 months ago

Hey @chalkygames123 thank you for the great feedback again!

I'll look into these.

chalkygames123 commented 7 months ago

About the item 1, I also noticed that it conflicts with the naming convention that Astro requires for endpoint functions. See: https://docs.astro.build/en/guides/endpoints/#http-methods

AndreaPontrandolfo commented 7 months ago

@chalkygames123 i also started a little astro support update here https://github.com/AndreaPontrandolfo/sheriff/pull/128, but it's unrelated to the problems that you are highlighting. The problem is that the @typescript-eslint/naming-convention rule in itself is already pretty complex, so i need to look into this carefully. Then we can work together for the solution and you can submit the PR if you want. In the meantime you can override the rule, just for the astro files, with your own implementation.

chalkygames123 commented 7 months ago

Thank you for the update.

I agree that it is pretty complex, and I'm not sure if there is a simple solution.

I'm sorry, but I don't have much time, so I'd like to leave you the PR and final decision. That said, please feel free to let me know if you want me to test your PR in my project.

I'm working around with patch-package, so there is no rush.

chalkygames123 commented 6 months ago

In case you didn't know, I just wanted to share a shareable config, @antfu/eslint-config, which is quite informative and inspiring (off-topic, sorry).

AndreaPontrandolfo commented 6 months ago

Hey @chalkygames123 ! Sure i know about Antfu config, it's highlighted in the docs. I plan to write more extensively how Sheriff compares to other projects, in the near future. Sheriff and Antfu config shares some similarities (and i'm a fan of his works, i even contributed to one of his repos!), but i would say without a doubt that Sheriff is more feature complete than Antfu config and generally a more mature project. Also it has to be noted that he has strong opinions on formatting stylistically with Eslint, whereas Sheriff delegates that responsability entirely to Prettier. He has a very controversial perspective on that front.

chalkygames123 commented 6 months ago

Oh yes! I had missed that line.

Yeah, @antfu/eslint-config is "very personal and opinionated," as he notes himself, so I have to admit I could not accept its formatting style. Also, the strictness of its rules is currently limited. Rather, I find its type-safe high configurability with eslint-flat-config-utils and the auto-detection of languages/frameworks to be remarkable.

It may also be worthwhile to incorporate @eslint/config-inspector into the Rules doc.

Anyway, I am looking forward to seeing Sheriff evolve further!

AndreaPontrandolfo commented 5 months ago

I had a look at eslint-flat-config-utils, it seems mildly interesting but honestly i'm having a hard time figuring out what those utils are for. The documentation is kinda scarce. The FlatConfig format is very powerful and surprisingly easy and straightforward to use, those utils seems fairly redundant and seem to re-introduce most of the complications of the old eslintrc format for no reason. I think i don't see the usefulness of that but please if you have more insights on that let me know. Also note that internally Sheriff use the tseslint.config util, provided by @typescript-eslint library.

AndreaPontrandolfo commented 5 months ago

@chalkygames123 related to your second point https://github.com/ota-meshi/eslint-plugin-astro/issues/300

AndreaPontrandolfo commented 5 months ago

I tried to have a look at this, but the eslint-plugin-astro is still very early. It changes very rapidly (they do like weekly releases) and it has tons of compatibility issues with other stuff. On-top of the fact that also Astro changes very rapidly. Also the docs are all scattered around, they reference the old eslintrc config a lot and the eslint-typescript support is unclear. I'll let eslint-plugin-astro stabilize a bit in the next few months and then i'll revisit.

lishaduck commented 2 months ago

I updated Astro as part of #157. item 1 seems to be the default for tseslint, and we aren't customizing it, so ¯\_(ツ)_/¯. Item 2 was corrected though.