memo in property getter

[elsassph]

In some cases, a call to memo is generated inside a property getter, which is very broken and will cause a leak if the property is read out of context:

Playground:
https://playground.solidjs.com/anonymous/c342a013-c31e-45a0-b75b-56f3b21e99df

It happens when using a ?: expression and "potentially reactive" condition and value, e.g.

condition() ? value() : undefined // bug
condition() ? 'value' : undefined // OK
condition ? value() : undefined // OK

[titoBouzout]

@elsassph can you please clarify? I am a bit confused about this issue, thanks!

[elsassph]

@titoBouzout tldr; the code is rewritten by the compiler to add a memo and this is questionable.

After discussion on Discord I've been told it's a feature, not a bug, but while I can understand the subtle potential effect of memoizing the condition, I think it is questionable to change the user code here.

• I understand it is valid when dealing with conditional children, to be safe against needless re-rendering,
• but applying that to all props doesn't seem documented and has been a source of serious confusion for us (why doesn't it happen when I move this code into a local function?),
• it is creating a temporary memo which will be thrown away every time the prop is read,
• unlike children or tag props (like div) there is no guarantee that the prop will be read in an effect / with context, which can cause mysterious reactivity leaks:

computations created outside a  `createRoot`  or `render` will never be disposed

[ryansolid]

This is one of my motivations for memos to be self releasable when created outside of roots. This is by far the most likely solve. Creating a few extra memos on access to be thrown away vs not guarding conditionals is hardly a decision.

The problem is any prop can be rendered... Ie props.children is the most common getter which means there is basically no chance that we won't be doing this sort of transformation. Unless we were to tell people they shouldn't use ternaries in JSX expressions which seems prohibitive.

I suppose it probably isn't called specifically in the documentation which is likely worth doing. But it is also a fairly rough heuristic. We can document that too. Basically it looks at the right side and decides if it is JSX Element or a function call that it could be considered expensive work worth memoizing. We could limit this to only JSX elements but I had my concerns especially early days when Components weren't special at all.

I think with self releasing Memos no one would really notice or care in this scenario.

[elsassph]

I'm not surprised to see an automatic memo for props.children but that was unintuitive that it happens for all props.

Not a bug then, but looking forward to self releasing memos...