rlaffers / eslint-plugin-xstate

ESLint plugin to check for common mistakes and enforce good practices when using XState.
MIT License
48 stars 4 forks source link

chore: added predictableActionArguments see: https://xstate.js.org/docs/guides/actions.html\#api #11

Closed EightArmCode closed 1 year ago

EightArmCode commented 1 year ago

Hello, and thanks for the lib!

I stumbled across this today, and found that one of the latest additions to xstate is not included in your lint rules for valid root state types. See the linked url.

I also had to add a peer dependency to eslint-plugin-node, eslint-plugin-n.

ghost commented 1 year ago
👇 Click on the image for a new way to code review - Make big changes easier — review code in small groups of related files - Know where to start — see the whole change at a glance - Take a code tour — explore the change with an interactive tour - Make comments and review — all fully sync’ed with github [Try it now!](https://app.codesee.io/r/reviews?pr=11&src=https%3A%2F%2Fgithub.com%2Frlaffers%2Feslint-plugin-xstate)

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map Legend

rlaffers commented 1 year ago

Hey, thanks for the PR! One question: why did you need to add the eslint-plugin-n lib? It does not seem to be used anywhere (it's not in the eslint config).

EightArmCode commented 1 year ago

Hey, thanks for the PR! One question: why did you need to add the eslint-plugin-n lib? It does not seem to be used anywhere (it's not in the eslint config).

Sure. If you attempt to run the linter, it complains.

Steps to reproduce. Clone the repo. Run npm i. Then run npm run lint. You get the following error:

npm run lint

> eslint-plugin-xstate@0.0.0-semantically-released lint /.../eslint-plugin-xstate // omitted my directory path
> eslint lib/

Oops! Something went wrong! :(

ESLint: 8.22.0

ESLint couldn't find the plugin "eslint-plugin-n".

(The package "eslint-plugin-n" was not found when loaded as a Node module from the directory "/.../eslint-plugin-xstate".)

It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:

    npm install eslint-plugin-n@latest --save-dev

The plugin "eslint-plugin-n" was referenced from the config file in ".eslintrc.json » eslint-config-standard".

If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
EightArmCode commented 1 year ago

Also, you might be interested to know I saw something mentioned on the stately repo about creating an eslint plugin for xstate, and I linked to your repo. It might be smart to reach out to them. 🌻

rlaffers commented 1 year ago

Sure. If you attempt to run the linter, it complains.

Steps to reproduce. Clone the repo. Run npm i. Then run npm run lint. You get the following error:


npm run lint

> eslint-plugin-xstate@0.0.0-semantically-released lint /.../eslint-plugin-xstate // omitted my directory path
> eslint lib/

Oops! Something went wrong! :(

ESLint: 8.22.0

ESLint couldn't find the plugin "eslint-plugin-n".

(The package "eslint-plugin-n" was not found when loaded as a Node module from the directory "/.../eslint-plugin-xstate".)

I can't reproduce this. eslint-plugin-n is a dependency of eslint-config-standard, so you should have it installed just by doing npm install (I do; you can check with npm ls eslint-plugin-n).

If you don't mind, I will exclude this change from the PR for now and keep only the updated rule + test for now. If the problem persists for you, please create another issue.

Also, you might be interested to know I saw something mentioned on the stately repo about creating an eslint plugin for xstate, and I linked to your repo. It might be smart to reach out to them. sunflower

Thanks for linking - I had announced this plugin in the XState Discord channel when it was first created. The maintainers are aware of it. I guess I will make a PR for updating their docs.

EightArmCode commented 1 year ago

Sure, feel free to do whatever you like. However, you might try a fresh install of node_modules to ensure it behaves as expected. I only installed it because otherwise the formatting script failed

rlaffers commented 1 year ago

Here's the PR without the package.json change. https://github.com/rlaffers/eslint-plugin-xstate/pull/12

I think the missing module in your case is due to a legacy version of Node/npm you might be using. Node v16/ Npm 8 installs it just fine. However npm install --legacy-peer-deps mimicking older versions will not. I recommend upgrading your Node/npm combo.

Thanks again.