solidjs-community / eslint-plugin-solid

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

Lint warning when using props in solid-styled-components #29

Closed kavsingh closed 1 year ago

kavsingh commented 2 years ago

When accessing props via solid-styled-components, lint warnings are shown:

The reactive variable 'props.theme?.colors.surface[0]' should be used within JSX, a tracked scope (like createEffect), or inside an event handler function. Details: https://github.com/joshwilsonvu/eslint-plugin-solid/blob/main/docs/reactivity.md.eslint[solid/reactivity](https://github.com/joshwilsonvu/eslint-plugin-solid/blob/main/docs/reactivity.md)

**Screenshots**

in VSCode

image

via eslint .

image

To Reproduce Have put together a small repro here: https://github.com/kavsingh/solid-eslint-styled-repro

essentially:

Expected behavior Would be great to use props without lint warnings, or having to disable reactivity rules inline

Desktop (please complete the following information):

Additional context Realise this is not vanilla solid, this should probably have been an enhancement / feature request but could not change the label after creating the issue. I can make a new feature request issue if it works better. Is there a way to to use solid-styled-components without producing lint warnings?

Thank you! 🙏

joshwilsonvu commented 2 years ago

Thanks for the issue! Integrating better with community projects is something that I want to do with the reactivity rule now that it's maturing. Currently, there's a part of the rule that's essentially a series of if-statements that look for cases to turn this warning off, and to fix your issue I'd add a case for expressions in styled.* tagged template literals.

However, I don't really want to "bless" certain libraries with special cases while the ecosystem is still maturing. I thought about trying to accept custom cases from libraries, but that's a big ask.

But I think there's a quick solution that will fix your issue while remaining neutral to specific libraries—allowing reactivity in expressions in any tagged template literal where the expression is an inline function. That's cases like:

styled.div<{ opacity: number }>`
  padding: ${(props) => props.theme?.spacing[1]};
`;
css`color: ${props => props.color}`;
html`<div>${props => props.name}</div>`;
log`hello world! ${function(otherProps) { return otherProps.greeting; }}`;
css`color: ${() => signal()}`;
(but not these) ```jsx // too easy to forget to call `signal` when the template tag expects a value, not a function css`color: ${signal}`; // no declared intent to pass the function, not the result of the function call const f = a => signal() + a; css`color: ${f}`; ```

Any concerns? This will probably come out in v0.7.2, I'll close this when it's published.

kavsingh commented 2 years ago

that sounds great, thank you! 💯 agree about not making specific cases for specific libraries, so what you've suggested sounds like a good way forward 🙏 👍

joshwilsonvu commented 1 year ago

This should be fixed in v0.7.2, thanks for your patience!

zjullion commented 1 year ago

Just FYI - I'm getting these warnings in 0.7.4 with solid-styled-components version 0.28.5.