solidjs-community / eslint-plugin-solid

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

Linter miss reactivity-breaking pattern #61

Closed philippe-wm closed 1 year ago

philippe-wm commented 1 year ago

Describe the bug

It is possible to "cheat" the linter using an intermediary function.

To Reproduce

export const MyComp: Component<{ data: string }> = (props) => {
  const data = (): string => props.data;
  const oops = data();
  return <div>{oops}</div>;
}

Expected behavior

Linter should flag that we are breaking reactivity.

Screenshots

Environment (please complete the following information):

Additional context

joshwilsonvu commented 1 year ago

Nice catch! Here's the problem, if you're curious:

solid/reactivity uses a stack-based algorithm based on lexical scopes. It determines whether a function is a component, and therefore whether its props parameter is a reactive variable, based on whether the function contains JSX. The catch is that it only knows whether the function contains JSX on the way back up the AST, and when it reaches your component on the way back up, it has already finished analyzing the data function without knowing that props is reactive. So marking props as reactive then is too late.

There are some upcoming improvements that will bypass this subtle timing issue. In the meantime this could be addressed by marking any single parameter named props as reactive, before checking if the function has JSX.

philippe-wm commented 1 year ago

Thanks for the fix, and for the explanation 😄