Rule for calling signals outside of implicit call sites

[thislooksfun]

Describe the need Solid's signal accessors are functions, not values. Much of the time using them as values instead of calling them will be caught by TypeScript, but there are some cases that won't be, for example:

function SampleComponent() {
  const [name, setName] = createSignal("John");
  const greeting = createMemo(() => `Hello ${name}`);
  return (
    <span>{greeting}, how are you?</span>
  );
}

The above code will render Hello function readSignal() { [native code] }, how are you?, and will not be reactive since name wasn't actually called inside the createMemo().

This is a problem for anyone coming over from React-flavored components (React, Preact, etc.) where the result of a state / memo hook is a value. Having a rule that catches this misuse is a must-have for me personally to confidently code in Solid.

Suggested Solution Have a rule that enforces that signals are actually called in places they expect to be. This will have to account for edge cases such as createMemo(() => ({ someKey: someSignalAccessor })), and it should have a setting for whether or not to allow the JSX shorthand (i.e. whether or not to allow using the accessor as a value inside the JSX block, like the {greeting} above).

Possible Alternatives Unfortunately the only alternative I can think of is to not have this rule and instead test everything (which is not a bad idea in general, but it's a bad assumption to make that every change will be properly tested).

Additional context Nothing that I can think of at the moment.

[joshwilsonvu]

Hi, thanks for the issue. You're right that eslint-plugin-solid is a good place to catch spots where signals should be called but aren't. There is progress on this already, in solid/reactivity:

(I'm amazed that {greeting} even displays any text, since it should be called here; Solid must be calling it automatically but I don't want to rely on that.)

Unfortunately, it doesn't catch where name should be called at the moment. I want to tweak your suggestion a bit and only pick certain cases where we know for sure that a signal should be called while leaving uncertain cases alone. That's the other way around from explicitly trying to allow all the cases where a signal doesn't have to be called. It's better to warn too little than too much.

Luckily, this case is easy to fix—if a signal is used in a template literal expression, it has to be called. There's a few other cases like this off the top of my head, like arithmetic/string concatenation and object/array indexing. I've added a commit to handle these, and will release it in a new version soon. Hope that helps!

[thislooksfun]

(I'm amazed that {greeting} even displays any text, since it should be called here; Solid must be calling it automatically but I don't want to rely on that.)

It does indeed. {count} and {count()} produce identical code. Specifically it seems that the compiler produces a binding statement (insert(_el$, signal)) with max(n-1, 0) sets of parens, so {count} and {count()} both produce insert(_el$, count). Not 100% sure what the reasoning for this is, but that's what it does. Poking around in the playground is kinda fun.

It's better to warn too little than too much.

I personally disagree. Obviously ideally it would warn exactly the right amount of the time, but if I have to choose one or the other I prefer to have too many warnings and dial them back by adding // eslint-disable-line (and similar) directives. That being said, the only cases that really matter to me are the ones that the TS compiler doesn't catch, and the only ones I can think of are string operations and passing to the TSX template itself.

[joshwilsonvu]

That's an interesting sentiment, and I'm sure it would lead to more correct code if you took the time to evaluate and disable all of the false positives. From an ecosystem perspective though, I still think a policy like that would damage solid/reactivity's credibility, leading to people disabling the rule altogether because of nuisance warnings—I've seen that happen a few times when the rule was less mature than it is now. Maybe one day I'll add an option to err on the side of too many warnings.

Anyway, the fix I mentioned is now out in v0.7.1. Thanks again for the report!