eslint-plugin-import-x icon indicating copy to clipboard operation
eslint-plugin-import-x copied to clipboard

refactor: rewrite `createRule` and `RuleTester`

Open SukkaW opened this issue 1 year ago • 7 comments

Currently, eslint-plugin-import-x patches @typescript-eslint/utils to add custom properties like the category and modify properties like the recommended.

The PR accomplishes the same thing, by doing some TypeScript exercises instead.

SukkaW avatar Apr 04 '24 15:04 SukkaW

⚠️ No Changeset found

Latest commit: 7196d933b3e8aa35dfc5a41241198872f2af66ec

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar Apr 04 '24 15:04 changeset-bot[bot]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

codesandbox-ci[bot] avatar Apr 04 '24 15:04 codesandbox-ci[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 96.23%. Comparing base (8b53954) to head (d7fc095).

:exclamation: Current head d7fc095 differs from pull request most recent head 7196d93. Consider uploading reports for the commit 7196d93 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #72   +/-   ##
=======================================
  Coverage   96.22%   96.23%           
=======================================
  Files          91       91           
  Lines        4399     4409   +10     
  Branches     1497     1498    +1     
=======================================
+ Hits         4233     4243   +10     
  Misses        160      160           
  Partials        6        6           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 04 '24 16:04 codecov[bot]

@JounQin @silverwind What do you think? Do you prefer creating our own utilities or patching ts-eslint?

SukkaW avatar Apr 05 '24 06:04 SukkaW

@JounQin Would you like to cleat the cache of the GitHub Action? It seems that the CI failed to the corrupt cache.

SukkaW avatar Apr 08 '24 13:04 SukkaW

@SukkaW

I raised https://github.com/typescript-eslint/typescript-eslint/issues/8897 to discuss about the optional parser property.

And I notice patches are also used in tseslint's repository https://github.com/typescript-eslint/typescript-eslint/blob/main/.yarn/patches, and considering there are some // @ts-expect-error - flow type comments in source codes, I'm also considering to add more patches for typings maybe, and the maintenance for such small patches should be easier than TypeScript exercises.

HDYT?

JounQin avatar Apr 10 '24 14:04 JounQin

HDYT?

@JounQin

The debate between patches and self-implementation essentially comes down to which method is easier to maintain.

IMHO, self-implementation is the superior choice. The dist files of @typescript-eslint could undergo substantial changes (e.g. build system change, dist structure change), and in a worst-case scenario, we might find ourselves creating a new patch with each dependency update.

Conversely, existing public APIs tend to offer more stability, providing a solid foundation upon which we can build.

The patch method is only effective for projects that are either no longer actively maintained or are extremely active to the point where patches quickly become obsolete. @typescript-eslint falls into neither of these categories. The response to feature requests and issues from @typescript-eslint is slow, and they frequently alter their build system.

SukkaW avatar Apr 13 '24 15:04 SukkaW

I am closing the PR now since TypeScript ESLint 8 has addressed the custom property of createRule through generic parameters. The issue has already been resolved by me in another PR https://github.com/un-ts/eslint-plugin-import-x/pull/112.

SukkaW avatar Aug 26 '24 23:08 SukkaW