solid icon indicating copy to clipboard operation
solid copied to clipboard

`createMemo` custom comparator only fired when signal has observers

Open kaisermann opened this issue 3 years ago • 1 comments

Describe the bug

This came from a discussion in the #reactivity channel:

This looks like it could be interpreted as a bug. It's fine to optimize for the "no observers" case (and avoid the call to writeSignal), but that else clause should check the equals function if there is one. Although in that case it might be just as well to call writeSignal... Maybe worth raising as a GitHub issue.

While creating a custom signal that uses createMemo and testing it, I noticed that the returned value of the memo, with a custom equals comparator, would vary depending on if the signal had any observers or not. I understand that not running it for something that has no "observers" must be an optimization (which makes total sense). In my head, however, every time something is returned by a createMemo, it should run through the equality check, no 🤔 ?


Original message:

If one wants to create a test for a custom signal composed of createMemo + a custom equals function, is there a way to trigger the equality comparison without having to create an observer for that signal?

Example of the test I'm trying to

createRoot(() => {
  const [source, setSource] = createSignal<number[]>([1, 2, 3], {
    name: "source",
  });

  // uses createMemo + custom equals comparator
  const list = createList(source);
  // createComputed(list)

  const valueA = list();

  setSource([1, 2, 3]);

  const valueB = list();

  expect(list()).toBe(valueA);
  expect(list()).toBe(valueB);
});

The basic idea is that createList will memoize a list and only return a new list if any of its items have changed. So I'm trying to compare the first [1,2,3] with the second one sent via setSource. If I uncomment the // createComputed(list) line, the custom equals runs and the test passes. If I don't create this "extra" observer, the equals won't be executed and one array just overrides the other and the test fails.

Your Example Website or App

https://playground.solidjs.com/?hash=2023177129&version=1.4.1

Steps to Reproduce the Bug or Issue

  1. Create a signal with an empty object
  2. Memoize that signal with a custom comparator that logs --EQUALS--
  3. Change the value of the first signal for something else
  4. Nothing is logged
  5. Add a createComputed that subscribes to the memoized signal
  6. Change the value of the first signal again
  7. --EQUALS-- is logged
import { createSignal, createMemo, createComputed } from "solid-js";

const [source, setSource] = createSignal({});

const list = createMemo(source, undefined, {
  equals: (a, b) => {
    console.log('--EQUALS--')
    return false
  },
});

// uncomment this
// createComputed(list);

setSource({});

REPL

Expected behavior

As a user, I expected my createMemo custom comparator to be used regardless of the number of subscribers, as the signal may be used directly by a non-tracking scope (such as for testing).

Screenshots or Videos

No response

Platform

  • OS: macOS
  • Browser: Chrome

Additional context

No response

kaisermann avatar Jun 17 '22 09:06 kaisermann

Personally I found this as a perf optimization. The reason it doesn't run the equals function is because createMemo already decides beforehand if it should notify its observers, and if it has, it would decide again based on if the value it is about to emit is "equal" to the previous value. I don't think it's desirable to run the equals function at all if there's no observers to begin with. I might be wrong unless there's a use-case outside testing.

lxsmnsyc avatar Jun 17 '22 11:06 lxsmnsyc