Parsely / wp-parsely

The official WordPress plugin for Parse.ly - makes it a snap to add the required tracking code to enable Parse.ly on your WordPress site.
https://wordpress.org/plugins/wp-parsely/
GNU General Public License v2.0
62 stars 31 forks source link

ESLint: Enable the method-signature-style rule #2618

Closed acicovic closed 1 month ago

acicovic commented 1 month ago

Is your feature request related to a problem?

The TL;DR is that we should enable the https://typescript-eslint.io/rules/method-signature-style rule in our code, and fix any violations.

To see why, this is an example (originally quoted by @sirreal, can be found here):

type AnyItem = Record<string, any>;

interface Post extends AnyItem { status: string; }

type F<T> = (item: T) => boolean;
interface IF<T> { (item: T): boolean; }

interface Base<T extends AnyItem> {
  isEligible(item: T): boolean;
  isEligible1: F<T>;
  isEligible2: IF<T>;
}

const base: Base<AnyItem> = {
  // Why is this is OK?
  isEligible: (item: Post) => item.status.startsWith('eli:'),

  // These are both type errors as expected.
  isEligible1: (item: Post) => item.status.startsWith('eli:'),
  isEligible2: (item: Post) => item.status.startsWith('eli:'),
}

Relatedly: https://x.com/mattpocockuk/status/1803753059480121593

sirreal commented 1 month ago

I endorse this change 😀 The next version of wp-scripts will enable this in the recommended ruleset.

acicovic commented 1 month ago

Note to our future selves: As we aren't currently using the last version of wp-scripts due to https://github.com/WordPress/gutenberg/issues/62202, we do want to add this rule, at least until we're able to upgrade.

acicovic commented 1 month ago

Related to #2624. If we become able to upgrade wp-scripts, then this rule will be auto-included. It probably makes sense to focus on #2624, as it should solve two issues at once.

acicovic commented 1 month ago

I finally went to fix this, since it was an easy win/change.