solidjs-community / eslint-plugin-solid

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

Regression in reactivity tracking #62

Closed philippe-wm closed 1 year ago

philippe-wm commented 1 year ago

Describe the bug

A reactivity pattern which was valid in 0.7.1 is not allowed now.

To Reproduce

export const SomeProvider: ParentComponent<{ adapter: ISomeAdapter }> = (props) => {
  return (
    <SomeContext.Provider value={createSomeService(props.adapter)}>
      {props.children}
    </SomeContext.Provider>
  );
};

Linter error:

The reactive variable 'props.adapter' should be used within JSX

Expected behavior

The prop is indeed used within JSX :)

Screenshots

Environment (please complete the following information):

Additional context

joshwilsonvu commented 1 year ago

Yes, this is intentional—see this section of the docs:

The value passed to provider is passed to useContext as is. That means wrapping as a reactive expression will not work. You should pass in Signals and Stores directly instead of accessing them in the JSX.

The rule is warning you that the property access on props is not tracked in the value prop. Wrapping it in a function/passing a signal directly does not trigger the warning, and neither will the following:

  return (
    <SomeContext.Provider value={createSomeService(() => props.adapter)}>
      {props.children}
    </SomeContext.Provider>
  );

Alternatively, if you're sure the adapter prop won't change, you can disable the rule here.

philippe-wm commented 1 year ago

Ouch, so we were actually breaking reactivity! Maybe the reported error message could be improved?

joshwilsonvu commented 1 year ago

For sure! But with the rule's current architecture, that and some other things aren't easy to do. Some issues are tagged with reactivity-v2, where an improved architecture will make them possible to address, and this is one of those. So give it some time 🙂