solidjs-community / eslint-plugin-solid

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

solid/reactivity - array destructing #41

Closed obecny closed 1 year ago

obecny commented 1 year ago

Describe the bug eslint complains about solid/reactivity

To Reproduce Have this code, why is it causing a warning about solid/reactivity (check constructor). Is the warning invalid ? As for this case imho it should not cause it. In the end it is working as expected.

import { render } from "solid-js/web";
import { Accessor, createEffect, createSignal, Setter } from "solid-js";

class Foo {
  public a: Accessor<string>;
  private _setA: Setter<string>;

  constructor() {
    // warning solid/reactivity
    [this.a, this._setA] = createSignal('no value');
  }

  public setA(value: number): void {
    this._setA(`value: ${value}`);
  }
}

function Counter() {
  const [count, setCount] = createSignal(0);
  const increment = () => setCount(count() + 1);
  const foo1 = new Foo();

  createEffect(() => {
    foo1.setA(count());
  });

  return (
    <button type="button" onClick={increment}>
      {count()} - {foo1.a()}
    </button>
  );
}

render(() => <Counter />, document.getElementById("app")!);

Expected behavior no eslint error

Screenshots

Environment (please complete the following information): "eslint": "~8.17.0", "solid-js": "^1.5.5" osx Monterey

Additional context

joshwilsonvu commented 1 year ago

Hi, thanks for the report. Since Solid examples in the docs use functions for composition instead of classes, and functions are more feasible to analyze, the solid/reactivity rule doesn't support static analysis for classes. Implementing similar behavior using a function should work:

function createFoo() {
  const [a, _setA] = createSignal('no value');
  const setA = (value: number) => {
    _setA(`value: ${value}`);
  };
  return [a, setA];
}

As an aside, for this minimal example, you might be better off with a derived signal. Something like this:

function Counter() {
  const [count, setCount] = createSignal(0);
  const increment = () => setCount(count() + 1);
  const formattedCount = () => `value: ${count}`;

  return (
    <button type="button" onClick={increment}>
      {count()} - {formattedCount()}
    </button>
  );
}
philippe-wm commented 1 year ago

@joshwilsonvu thanks for the suggestion - it's certainly possible to use an intermediary function to get around the lint rule.

It is very possible, not knowing the linter internals, that it is very wrong to think of this problem as "just" allowing deconstructing a signal as [this.x, this.setX] = createSignal() in addition to const [x, setX] = createSignal()...