S
SolidJS•2mo ago
snorbi

Solving "computations created outside ..." using `createMemo() ?`

I got the following warning because I used props.values in my event handling code:
computations created outside a createRoot or render will never be disposed
I have read the docs and some additional info (like https://github.com/solidjs/solid/issues/2130). The warning is gone when I simply wrap the props.values access in a createMemo(): 
const values = createMemo(() => props.values)
const values = createMemo(() => props.values)
My question is: is it a good solution? If yes, then why the props accessors are not functions themselves (like props.values()instead of the current props.values) so they would always return a "fresh" value? Thanks.
11 Replies
peerreynders
peerreynders•2mo ago
because I used props.values in my event handling code
Event handlers aren't tracked; so something in the code is preventing the linter from recognizing it as event handler code.
My question is: is it a good solution?
The typical countermeasure is untrack
why the props accessors are not functions themselves
They are getters, so in a sense they are functions … In this instance the error is more about the fact that reactive values are being accessed in a non-reactive context, rendering them in fact non-reactive. untrack clearly communicates the intent that this is a non-reactive (non-subscribing) access. That said, untrack shouldn't be necessary in an event handler.
peerreynders
peerreynders•2mo ago
Building a Reactive Library from Scratch can help to build a workable mental model.
DEV Community
Building a Reactive Library from Scratch
In the previous article A Hands-on Introduction to Fine-Grained Reactivity I explain the concepts...
Andreas Roth
Andreas Roth•2mo ago
The initial questions seems fishy or incomplete... The warning is that computations cannot be CREATED outside a reactive context. Accessing memos or signal fields should never CREATE a new computation, just read the current value, and this is totally fine to do outside of a reactive context. So you should trace the path from props.values through the parent components to the source to see what is happening... My guess is that you have defined a getter somewhere, that creates a memo or something like that
snorbi
snorbiOP•2mo ago
untrack() does not solve the issue, the warning still arises... So I must do something incorrectly, as @Andreas Roth suggested 😦 The code is something like this: 
export type Binding<T> = {
    get: () => T
    set(value: T): void
}

export function createBinding<T>(getter: () => T, setter: (value: T) => void): Binding<T | null> {
    return { get: getter, set: setter }
}

...

export type ComboboxProps<T extends NonNullable<any>> = JSX.SelectHTMLAttributes<HTMLSelectElement> & {
    binding: Binding<T | null>
    values: T[]
}

...

export function Combobox<T extends NonNullable<any>>(props: ComboboxProps<T>): JSX.Element {
    ...

    let ref: HTMLSelectElement | undefined

    const values = () => props.values // <-- using `createMemo(() => props.values)` solves the warning

    const handleChange = () => {
        const value = ref!.value
        props.binding.set(value != null && value != "" ? values().filter(e => keyProvider(e) == value)[0] : null) // <-- The warning is issued at this line; it is interesting that `props.binding` does not cause a warning
    }

    onMount(() => ref!.addEventListener("input", handleChange))
    onCleanup(() => ref!.removeEventListener("input", handleChange))

    return (
        <select ref={ref} ...>
            ...
        </select>
    );
}
export type Binding<T> = {
    get: () => T
    set(value: T): void
}

export function createBinding<T>(getter: () => T, setter: (value: T) => void): Binding<T | null> {
    return { get: getter, set: setter }
}

...

export type ComboboxProps<T extends NonNullable<any>> = JSX.SelectHTMLAttributes<HTMLSelectElement> & {
    binding: Binding<T | null>
    values: T[]
}

...

export function Combobox<T extends NonNullable<any>>(props: ComboboxProps<T>): JSX.Element {
    ...

    let ref: HTMLSelectElement | undefined

    const values = () => props.values // <-- using `createMemo(() => props.values)` solves the warning

    const handleChange = () => {
        const value = ref!.value
        props.binding.set(value != null && value != "" ? values().filter(e => keyProvider(e) == value)[0] : null) // <-- The warning is issued at this line; it is interesting that `props.binding` does not cause a warning
    }

    onMount(() => ref!.addEventListener("input", handleChange))
    onCleanup(() => ref!.removeEventListener("input", handleChange))

    return (
        <select ref={ref} ...>
            ...
        </select>
    );
}
And the usage is something like: 
function CheckoutPage() {
  ...
  const [selectedPaymentOption, setSelectedPaymentOption] = createSignal<PaymentOption | null>(null)
  ...
  return (
    ...
      <Combobox binding={createBinding(selectedPaymentOption, setSelectedPaymentOption)} values={<<a bit more complex filtering/mapping based on a Signal>>} ... />
    ...
  )
function CheckoutPage() {
  ...
  const [selectedPaymentOption, setSelectedPaymentOption] = createSignal<PaymentOption | null>(null)
  ...
  return (
    ...
      <Combobox binding={createBinding(selectedPaymentOption, setSelectedPaymentOption)} values={<<a bit more complex filtering/mapping based on a Signal>>} ... />
    ...
  )
Thanks. Ps.: I'm relatively new in both TypeScript/Javascript and SolidJS
bigmistqke
bigmistqke•2mo ago
(tip: you can get syntax highlighting with ``tsx) > If yes, then why the props accessors are not functions themselves (like props.values()instead of the current props.values) so they would always return a "fresh" value? the error you are getting is not about props.values` being incorrect values, but it's a warning about a possible memory leak.
peerreynders
peerreynders•2mo ago
A simple attempt at a reproduction doesn't yield a similar warning https://playground.solidjs.com/anonymous/5d394788-c3d4-40c9-adf4-90e1d3b9fb95 Which makes me wonder whether whatever is stored in <<a bit more complex filtering/mapping based on a Signal>> has something to do with it.
Solid Playground
Quickly discover what the solid compiler will generate from your JSX template
snorbi
snorbiOP•2mo ago
I have created a complete example: https://playground.solidjs.com/anonymous/e0fc8d1e-cfc8-4774-9d5e-ef9068494c00 For some reason I cannot run it in the playground (it is the first time I use it 🥲 ) but I hope it helps (I'm unable to include here because it is too long). Running it locally on my PC, first I select a value from the first dropdown, then the warning is issued when I select a value from the second dropdown. Thanks again for looking into it.
peerreynders
peerreynders•2mo ago
https://playground.solidjs.com/anonymous/7c94dfdb-612c-449c-ae5a-f434b508c574 FYI: While your language of choice may use null, the JS-language equivalent is undefined. null was adopted in JS to communicate "absence of an object"; then the DOM API started using null because the spec was "language agnostic" and used null (presumably from Java) in the text. https://github.com/sindresorhus/meta/discussions/7 Typically I find it a lot less verbose to stick with undefined and convert null to undefined where ever it surfaces. The DOM API also often uses an empty string to convey "no value". In most cases you want to use strict equality rather than non-strict equality. The only real use for non-strict equality is that undefined == null is true (while undefined === null is false). Once you standardize on using undefined the legitimate use cases for non-strict equality are drastically reduced. Example: https://playground.solidjs.com/anonymous/9f61bda6-04c8-4e2a-9c45-b392e727d733
Solid Playground
Quickly discover what the solid compiler will generate from your JSX template
GitHub
Intent to stop using null in my JS code · sindresorhus meta · Dis...
I intend to stop using null in my code, in favor of undefined. There are many reasons for doing so. Feedback wanted Tweet: https://twitter.com/sindresorhus/status/1114894287387258886 Background htt...
MDN Web Docs
Strict equality (===) - JavaScript | MDN
The strict equality (===) operator checks whether its two operands are equal, returning a Boolean result. Unlike the equality operator, the strict equality operator always considers operands of different types to be different.
mdynnl
mdynnl•2mo ago
createMemo is the correct solution in this case. what's happening is we compile ternaries and friends into memo which get buried under a getter, so accessing it outside of the graph will create computations outside of the graph. https://playground.solidjs.com/anonymous/ab0ebe4f-f90f-46d9-9be9-9e31cbb38b85 the playground by default uses a prod build from esm.sh so you need to pull the dev version with ?dev query to get the warnings though
snorbi
snorbiOP•2mo ago
Thanks @peerreynders , I refactored my code to use undefined instead of null. I have a Java/Kotlin background, so I used null out of reflex 🙂 Thanks @mdynnl for looking into it. I don't understand the "ternaries" part deep enough but it is good to hear that createMemo() is the good solution here 🙂 (Although it does not seem to solve the warning in case of your example, it solves it in my code.)
mdynnl
mdynnl•2mo ago
it's actually pretty simple if you look at the output tab Given a ternary cond() ? foo() : bar(), we wrap the condition part cond() with a memo like memo(() => !!cond()) and then use it, so that the whole computation doesn't re-run if the truthy-ness didn't change

Did you find this page helpful?