search

solidjs-community / eslint-plugin-solid

Solid-specific linting rules for ESLint.
MIT License
216 stars 26 forks source link

Rule "reactivity" triggers on effector stores #6

Closed ainursharaev closed 2 years ago

ainursharaev commented 2 years ago

Describe the bug The rule reactivity triggers on effector stores, I assume it's because of naming convention.

To Reproduce Steps to reproduce the behavior:

  1. Scaffold a SolidJS+TS project
  2. Install effector as a dependency
  3. Setup ESLint, it's config should extends "plugin:solid/typescript"
  4. Make an effector store in code, e.g. const $user = createStore({})
  5. See an error ESLint: Array destructuring should be used to capture the first result of this function call.(solid/reactivity)

Expected behavior This rule shouldn't trigger on effector stores

Screenshots If applicable, add screenshots to help explain your problem. image

joshwilsonvu commented 2 years ago

Thanks for the report and for using the plugin. You're right in a way, the rule triggers because it matches createStore which is usually imported from solid-js/store. Currently Solid's primitives are all based on name only.

I think a good solution for you might be to import { createStore as createEffectorStore } from "effector";. Since you're using Solid, reserving that name for the createStore Solid primitive is best for readability, in my opinion. Alternatively you could disable solid/reactivity for that line.

I could be convinced to add an allowlist/blocklist option if you really want to ignore usages of createStore altogether, but still want reactivity checked on everything else.

ainursharaev commented 2 years ago

effector library has an ESLint plugin, which includes a rule for enforcing store naming conventions.

I think it could be a helpful example as to how to validate primitives based on other parameters than name.

I'd make a PR but I don't have prior experience in creating ESLint plugins

joshwilsonvu commented 2 years ago

Thanks, that example is helpful. The reason I checked by name only and didn't want to track import aliases was because true variable analysis is tricky and potentially expensive. But the method they're using here is much less complicated and should cover 99.9% of cases.

It does look like the rule you mentioned can support renaming to createEffectorStore, or at least it should. But I'll see if I can do the same thing for the next release of this plug-in, seems nicer than a ESLint config option.

joshwilsonvu commented 2 years ago

This issue is fixed with v0.4.4!