add eslint #5041

with-heart · 2024-08-16T01:49:58Z

This PR installs and configures eslint with typescript-eslint.

It uses the new flat config format (which is such a nice improvement imo), along with the recommended rules from @eslint/js and typescript-eslint.

Since #5022 is a thing, I added all test.* files to the global ignores. Once that lands we can remove this and maybe add eslint-plugin-vitest.

changeset-bot · 2024-08-16T01:50:04Z

⚠️ No Changeset found

Latest commit: aee03ce

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

with-heart · 2024-08-16T01:52:13Z

I wanted to try this with type-aware linting but it reported hundreds of errors that didn't seem like they'd be easy to fix. They're likely related to having to set @typescript-eslint/no-explicit-any to warn (581 warnings for that rule alone 😬).

Andarist · 2024-08-16T07:23:53Z

how many we'd get after disabling @typescript-eslint/no-explicit-any?

with-heart · 2024-08-16T15:18:35Z

@Andarist 0. Should we disable it? I wasn't sure if this was something that'd be worth trying to deal with or not

with-heart · 2024-08-16T15:23:10Z

I'm not sure what the deal is with the failed check. The error it reports isn't actually relevant.

Here's the actual error:

This syntax is reserved in files with the .mts or .cts extension. Add a trailing comma, as in `<T,>() => ...`. (20:14)

To make build work, I have to add a trailing comma to type params in packages/xstate-solid:

diff --git a/packages/xstate-solid/src/createImmutable.ts b/packages/xstate-solid/src/createImmutable.ts
index 5624473d5b..e3237c7414 100644
--- a/packages/xstate-solid/src/createImmutable.ts
+++ b/packages/xstate-solid/src/createImmutable.ts
@@ -19,7 +19,7 @@ const updateStore = <Path extends unknown[]>(
   store: Store<any>
 ) => {
   const valueRefs = new WeakMap<any, unknown>();
-  const diff = <CompareValue>(
+  const diff = <CompareValue,>(
     next: CompareValue,
     prev: CompareValue,
     path: Path
diff --git a/packages/xstate-solid/src/deepClone.ts b/packages/xstate-solid/src/deepClone.ts
index e829300845..58bc92e28d 100644
--- a/packages/xstate-solid/src/deepClone.ts
+++ b/packages/xstate-solid/src/deepClone.ts
@@ -17,7 +17,7 @@ export function isWrappable(obj: any): obj is object {
  * @param valueRefs A WeakMap that stores a reference from the original
  *   object/array to the cloned object/array
  */
-const clone = <T>(value: T, valueRefs: WeakMap<any, any>): T => {
+const clone = <T,>(value: T, valueRefs: WeakMap<any, any>): T => {
   if (!isWrappable(value)) {
     return value;
   }
@@ -47,4 +47,4 @@ const clone = <T>(value: T, valueRefs: WeakMap<any, any>): T => {
   return clonedValue;
 };
 
-export const deepClone = <T>(value: T): T => clone(value, new WeakMap());
+export const deepClone = <T,>(value: T): T => clone(value, new WeakMap());

These previously used extends unknown:

diff --git a/packages/xstate-solid/src/createImmutable.ts b/packages/xstate-solid/src/createImmutable.ts
index 7764371bac..e3237c7414 100644
--- a/packages/xstate-solid/src/createImmutable.ts
+++ b/packages/xstate-solid/src/createImmutable.ts
@@ -19,7 +19,7 @@ const updateStore = <Path extends unknown[]>(
   store: Store<any>
 ) => {
   const valueRefs = new WeakMap<any, unknown>();
-  const diff = <CompareValue extends unknown>(
+  const diff = <CompareValue,>(
     next: CompareValue,
     prev: CompareValue,
     path: Path
diff --git a/packages/xstate-solid/src/deepClone.ts b/packages/xstate-solid/src/deepClone.ts
index 1eeaa76a2d..58bc92e28d 100644
--- a/packages/xstate-solid/src/deepClone.ts
+++ b/packages/xstate-solid/src/deepClone.ts
@@ -17,10 +17,7 @@ export function isWrappable(obj: any): obj is object {
  * @param valueRefs A WeakMap that stores a reference from the original
  *   object/array to the cloned object/array
  */
-const clone = <T extends unknown>(
-  value: T,
-  valueRefs: WeakMap<any, any>
-): T => {
+const clone = <T,>(value: T, valueRefs: WeakMap<any, any>): T => {
   if (!isWrappable(value)) {
     return value;
   }
@@ -50,5 +47,4 @@ const clone = <T extends unknown>(
   return clonedValue;
 };
 
-export const deepClone = <T extends unknown>(value: T): T =>
-  clone(value, new WeakMap());
+export const deepClone = <T,>(value: T): T => clone(value, new WeakMap());

Not really sure what the issue is here and prettier removes the dangling commas so it's not really a fix.

boneskull · 2024-08-29T21:33:46Z

I wanted to try this with type-aware linting but it reported hundreds of errors that didn't seem like they'd be easy to fix. They're likely related to having to set @typescript-eslint/no-explicit-any to warn (581 warnings for that rule alone 😬).

IMO this is worth doing. You will probably need to disable stuff if using the recommendedTypeChecked preview; it's pretty strict.

Here's some of the rules I'd suggest tweaking:

// and sometimes you gotta use any
'@typescript-eslint/no-explicit-any': 'off',

// allow ! 
'@typescript-eslint/no-non-null-assertion': 'off',

// allow prefixing unused vars with _
'@typescript-eslint/no-unused-vars': [
  'error',
  {
    argsIgnorePattern: '^_',
    varsIgnorePattern: '^_',
  },
],

// better for overloads
'@typescript-eslint/unified-signatures': [
  'error',
  {
    ignoreDifferentlyNamedParameters: true,
  },
],

Something like the above might get you out of the woods.

Also I'd recommend enabling this (if it's not already enabled by default), which surfaces unused // eslint-disable directives:

{
  linterOptions: {
    reportUnusedDisableDirectives: 'error',
  },
}

I'm not sure what you were using for the parser config, but this is what has worked well for me with tseslint@v8.

languageOptions: {
  parserOptions: {
    projectService: {
      // type-aware linting for non-project config files. modify as necessary
      allowDefaultProject: ['*.js', '.*.js'],
    },
    tsconfigRootDir: import.meta.dirname, // (or __dirname, depending)
  },
},

boneskull · 2024-08-29T21:41:05Z

I don't think T extends unknown is doing anything in that function, and can just be T.

davidkpiano · 2024-09-12T03:25:16Z

@with-heart Can you resolve merge conflicts?

with-heart · 2024-09-12T18:04:19Z

@with-heart Can you resolve merge conflicts?

Done! I think we should be good to go assuming the changes look fine 🙏

davidkpiano

cc. @Andarist

Andarist · 2024-09-19T20:22:51Z

packages/xstate-store/src/types.ts

@@ -109,7 +109,7 @@ export interface Store<

 export type AnyStore = Store<any, any, any>;

-export type Compute<A extends any> = { [K in keyof A]: A[K] } & unknown;
+export type Compute<A> = { [K in keyof A]: A[K] };


& unknown is required sometimes

Suggested change

export type Compute<A> = { [K in keyof A]: A[K] };

export type Compute<A> = { [K in keyof A]: A[K] } & unknown;

Andarist · 2024-09-19T20:24:42Z

packages/xstate-solid/src/deepClone.ts

+// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-constraint
 const clone = <T extends unknown>(


would this work instead?

Suggested change

// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-constraint

const clone = <T extends unknown>(

const clone = <T,>(

packages/xstate-inspect/src/server.ts

packages/core/src/types.ts

Andarist · 2024-09-19T20:35:00Z

packages/core/src/types.ts

@@ -41,7 +41,7 @@ export type GetParameterizedParams<T extends ParameterizedObject | undefined> =
 * This type can be used to avoid this problem. This union represents the same
 * value space as `unknown`.
 */
-export type NonReducibleUnknown = {} | null | undefined;
+export type NonReducibleUnknown = NonNullable<unknown> | null | undefined;


I officially hate this... 😂 I kinda understand why {} has such a bad rep... but if I'll have to start using NonNullable<unknown> everywhere to workaround a lint rule... 😬

Personally, I'd vote for disabling that rule and allowing {}

Andarist · 2024-09-19T20:39:43Z

packages/core/src/guards.ts

+    _args: GuardArgs<TContext, TExpressionEvent>,
+    _params: TParams


those might be seen by users when debugging in devtools, I think it would be best to avoid _ for those

Andarist · 2024-09-19T21:26:21Z

packages/core/src/actions/stopChild.ts

@@ -102,8 +101,8 @@ export function stopChild<
  actorRef: ResolvableActorRef<TContext, TExpressionEvent, TParams, TEvent>
 ): StopAction<TContext, TExpressionEvent, TParams, TEvent> {
  function stop(
-    args: ActionArgs<TContext, TExpressionEvent, TEvent>,
-    params: TParams
+    _args: ActionArgs<TContext, TExpressionEvent, TEvent>,


same in all of those actions

Co-authored-by: Mateusz Burzyński <[email protected]>

* add eslint (#5041) * remove tslint.json * install and configure eslint * resolve lint errors * add eslint to ci-checks * enable eslint type-checked rules * fix type-check errors * add eslint fix commits to .git-blame-ignore-revs * allow empty object types * disable no-unnecessary-type-constraints on specific lines * revert removal of non-conforming type check in actions * resolve lint errors * Update packages/core/src/types.ts Co-authored-by: Mateusz Burzyński <[email protected]> * Update packages/core/src/types.ts Co-authored-by: Mateusz Burzyński <[email protected]> * Update packages/xstate-inspect/src/server.ts Co-authored-by: Mateusz Burzyński <[email protected]> --------- Co-authored-by: David Khourshid <[email protected]> Co-authored-by: Mateusz Burzyński <[email protected]> * Fix lint issues --------- Co-authored-by: with-heart <[email protected]> Co-authored-by: Mateusz Burzyński <[email protected]>

with-heart added 4 commits August 15, 2024 21:35

remove tslint.json

aaebb2c

install and configure eslint

472570f

resolve lint errors

8bff3b8

add eslint to ci-checks

de4b3d9

with-heart added 3 commits August 30, 2024 14:40

enable eslint type-checked rules

e014d30

fix type-check errors

aeec38a

add eslint fix commits to .git-blame-ignore-revs

28b8b72

with-heart added 4 commits September 12, 2024 11:30

allow empty object types

ef5c51d

Merge remote-tracking branch 'upstream/main' into eslint

17a6a1f

disable no-unnecessary-type-constraints on specific lines

cc323c7

revert removal of non-conforming type check in actions

838e53a

with-heart added 2 commits September 19, 2024 11:09

Merge branch 'main' into eslint

b597d16

resolve lint errors

560cac9

with-heart force-pushed the eslint branch from 3f6fc4a to 560cac9 Compare September 19, 2024 15:15

davidkpiano approved these changes Sep 19, 2024

View reviewed changes

Andarist reviewed Sep 19, 2024

View reviewed changes

packages/xstate-inspect/src/server.ts Outdated Show resolved Hide resolved

Andarist reviewed Sep 19, 2024

View reviewed changes

packages/core/src/types.ts Outdated Show resolved Hide resolved

Andarist reviewed Sep 19, 2024

View reviewed changes

packages/core/src/types.ts Outdated Show resolved Hide resolved

Andarist reviewed Sep 19, 2024

View reviewed changes

davidkpiano and others added 3 commits September 28, 2024 14:32

Update packages/core/src/types.ts

a7bd0a1

Co-authored-by: Mateusz Burzyński <[email protected]>

Update packages/core/src/types.ts

a2c72e3

Co-authored-by: Mateusz Burzyński <[email protected]>

Update packages/xstate-inspect/src/server.ts

1738557

Co-authored-by: Mateusz Burzyński <[email protected]>

davidkpiano changed the base branch from main to davidkpiano/with-heart-eslint September 29, 2024 16:12

Merge branch 'davidkpiano/with-heart-eslint' into eslint

aee03ce

davidkpiano merged commit ca0a5f8 into statelyai:davidkpiano/with-heart-eslint Sep 29, 2024
1 check failed

davidkpiano mentioned this pull request Sep 29, 2024

Add eslint #5097

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add eslint #5041

add eslint #5041

with-heart commented Aug 16, 2024

changeset-bot bot commented Aug 16, 2024 •

edited

Loading

with-heart commented Aug 16, 2024

Andarist commented Aug 16, 2024

with-heart commented Aug 16, 2024

with-heart commented Aug 16, 2024 •

edited

Loading

boneskull commented Aug 29, 2024

boneskull commented Aug 29, 2024

davidkpiano commented Sep 12, 2024

with-heart commented Sep 12, 2024

davidkpiano left a comment

Andarist Sep 19, 2024

Andarist Sep 19, 2024

Andarist Sep 19, 2024

Andarist Sep 19, 2024

Andarist Sep 19, 2024

	export type Compute<A> = { [K in keyof A]: A[K] };
	export type Compute<A> = { [K in keyof A]: A[K] } & unknown;

		// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-constraint
		const clone = <T extends unknown>(

	// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-constraint
	const clone = <T extends unknown>(
	const clone = <T,>(

		_args: GuardArgs<TContext, TExpressionEvent>,
		_params: TParams

add eslint #5041

add eslint #5041

Conversation

with-heart commented Aug 16, 2024

changeset-bot bot commented Aug 16, 2024 • edited Loading

⚠️ No Changeset found

with-heart commented Aug 16, 2024

Andarist commented Aug 16, 2024

with-heart commented Aug 16, 2024

with-heart commented Aug 16, 2024 • edited Loading

boneskull commented Aug 29, 2024

boneskull commented Aug 29, 2024

davidkpiano commented Sep 12, 2024

with-heart commented Sep 12, 2024

davidkpiano left a comment

Choose a reason for hiding this comment

Andarist Sep 19, 2024

Choose a reason for hiding this comment

Andarist Sep 19, 2024

Choose a reason for hiding this comment

Andarist Sep 19, 2024

Choose a reason for hiding this comment

Andarist Sep 19, 2024

Choose a reason for hiding this comment

Andarist Sep 19, 2024

Choose a reason for hiding this comment

changeset-bot bot commented Aug 16, 2024 •

edited

Loading

with-heart commented Aug 16, 2024 •

edited

Loading