solidjs-community / eslint-plugin-solid

Solid-specific linting rules for ESLint.
MIT License
209 stars 24 forks source link

solid/reactivity is false positive for custom element event handlers & callbacks in context updates #95

Closed ivan-lednev closed 1 year ago

ivan-lednev commented 1 year ago

Hi! First of all, thank you for a great plugin! I'm new to Solid.js, so I'm not even sure if it's a bug.

Describe the bug It looks like the plugin gives a false positive warning in two cases:

  1. When I use reactive variables in event handlers in custom elements
  2. When I use reactive variables in callbacks for updating store values

Here is a code example:

import { createContext, createSignal, useContext } from "solid-js";

function CustomButton(props) {
  return <button onClick={props.onClick}>{props.label}</button>;
}

const CustomContext = createContext();

function CustomProvider(props) {
  return <CustomContext.Provider value={{}}>{props.children}</CustomContext.Provider>;
}

function App() {
  const [counter, setCounter] = createSignal(0);
  const [, setContextValue] = useContext(CustomContext);

  return (
    <>
      <CustomButton
        // ESLint: This function should be passed to a tracked scope (like createEffect) or an event handler because
        // it contains reactivity, or else changes will be ignored.
        onClick={() => {
          setCounter(counter() + 1);
        }}
      >
        Click me!
      </CustomButton>

      <button
        // ESLint: This function should be passed to a tracked scope (like createEffect) or an event handler because
        // it contains reactivity, or else changes will be ignored.
        onClick={() => setContextValue("key", (prev) => prev + counter())}
      >
        Click me!
      </button>
    </>
  );
}
g-mero commented 1 year ago

same problem

joshwilsonvu commented 1 year ago

What version of the plugin were you using, or were you using the playground? There was an issue that caused warnings like these that has now been fixed. When I load your example now, I see only two warnings.

  return <button onClick={props.onClick}>{props.label}</button>;

This one is valid—see reasoning here.

        onClick={() => setContextValue("key", (prev) => prev + counter())}

This one occurs because the linter doesn't know that setContextValue is supposed to be a store setter, because it's an arbitrary variable taken from a context. So, it doesn't know that the updater function is "special" and runs synchronously. I'll consider allowing updater functions to be passed to any set* function call, but for now this is working as expected.

Thanks for filing an issue and don't hesitate to ask if anything else comes up!