Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pinia Eslint plugin #2612

Open
MickL opened this issue Mar 14, 2024 · 9 comments
Open

Pinia Eslint plugin #2612

MickL opened this issue Mar 14, 2024 · 9 comments

Comments

@MickL
Copy link

MickL commented Mar 14, 2024

What problem is this solving

Would be cool to have an Eslint plugin that provides best practices. E.g. With a setup store all variables need to be exported (according to the docs) and a plugin could warn / error if we forgot to export a variable / function.

Maybe there are more best practices or anti patterns that the plugin could warn / error about.

Proposed solution

Describe alternatives you've considered

No response

@posva
Copy link
Member

posva commented Mar 14, 2024

Yes, I've been hoping for an eslint expert to show up for the last 3 years 😄. I still haven't lost hope

@lisilinhart
Copy link

lisilinhart commented Apr 2, 2024

I've been looking into what rules would make sense for a pinia eslint plugin. Looking at the docs and came up with some ideas, what do you think @posva and @MickL?

  • prefer-use-store-naming-convention: Enforces the convention of naming stores with the prefix use followed by the store name and suffixed with Store
  • prefer-single-store-per-file: Encourages defining each store in a separate file
  • require-setup-store-exports: Checks the structure of stores defined using the setup function, ensuring state variable properties are returned.
  • no-return-global-properties: Warns against returning global properties like route or appProvided in setup stores.

@MickL
Copy link
Author

MickL commented Apr 2, 2024

Sounds very good! Would it be possible to enforce that ALL variables that are defined in the store also need to be exported by the store?

@lisilinhart
Copy link

lisilinhart commented Apr 2, 2024

Hi @MickL, here is the first version including three rules, I tried it with a vue scaffold already:

image

Copy link
Member

posva commented Apr 3, 2024

Nice, thanks a lot for the initiative @lisilinhart ! A few notes regarding the rules:

  • prefer-use-store-naming-convention: Yes for the use but maybe the Store suffix should not be activated by default because it's okay to name it something differently like Service or other name. So I think it should be configurable
  • prefer-single-store-per-file: I don't think this should be activated by default or maybe it should be applied only to exported stores, only one exported store. I'm thinking of this https://masteringpinia.com/blog/how-to-create-private-state-in-stores. In that case you might even export the private store for testing purposes and it's not convenient to split it into multiple files
  • require-setup-store-exports: I think the default should only apply to ref, reactive and other state properties. It shouldn't warn for missing functions, computed or raw data (not state)
  • no-return-global-properties: 👍

As for other other ideas:

What do you think?
I think Discussions is a great place to find where people are struggling

@lisilinhart
Copy link

lisilinhart commented Apr 3, 2024

Hey @posva, thanks for the detailed feedback. Your suggestions make a lot of sense and I'll look into adding those changes into the already existing rules in the next few days. I thought the same thing about only warning around ref reactive, it makes sense not to warn about more constant / helper variables or inner functions. I also need to look into more detail, which ones would be possible to autofix.

Discussions sounds like a good place to gain insight from actual use cases, should I open one and summarise the points that were mentioned here?

Copy link
Member

posva commented Apr 4, 2024

Feel free to open a discussion if that helps you

@MickL
Copy link
Author

MickL commented Apr 4, 2024

Hey @lisilinhart this sounds amazing! Unfortunately I am currently focusing on backend work but next week I will be back at Nuxt and try it out!

@lisilinhart
Copy link

Feel free to open a discussion if that helps you

I opened the discussion here: #2638

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants