[bugfix] previous value and has previous value of computed and refactor logic

[nank1ro]

Refactor the core of the library in order to fix issues with previousValue and hasPreviousValue of Computed and simplify the logic.

[codecov[bot]]

Codecov Report

:exclamation: No coverage uploaded for pull request base (dev@2f64d0d). Click here to learn what that means. The diff coverage is n/a.

@@          Coverage Diff           @@
##             dev      #45   +/-   ##
======================================
  Coverage       ?   97.32%           
======================================
  Files          ?       18           
  Lines          ?      710           
  Branches       ?        0           
======================================
  Hits           ?      691           
  Misses         ?       19           
  Partials       ?        0           

[nank1ro]

@manuel-plavsic

This is the only critical change I am not sure about https://github.com/nank1ro/solidart/pull/45/files#diff-ff1336af3256cbb826fb12484c57514aeec782f3a3fc8c6a0f8d4a77a5572fe0R210

If T is something nullable I may skip the value.

[nank1ro]

Yep unfortunately I just confirmed this bug with a new test:

    test('nullable derived signal', () async {
      final count = createSignal<int?>(0);
      final doubleCount = createComputed(() {
        if (count() == null) return null;
        return count()! * 2;
      });

      await pumpEventQueue();
      expect(doubleCount.value, 0);

      count.set(1);
      await pumpEventQueue();
      expect(doubleCount.value, 2);

      count.set(null);
      await pumpEventQueue();
      expect(doubleCount.value, null); // <-- FAILS HERE, IT'S 2 AND NOT NULL
    });

[manuel-plavsic]

Yes, it is because null is supposed to be a possible value in case T is nullable. I think that using T? is more pragmatic than using the wrapper class I suggested in a previous PR, however it makes these things a bit less intuitive (when designing the library).

I would change

if (changed && newValue != null) {
  _setValue(newValue);
}

to:

if (changed) {
   _setValue(newValue as T);
}

I haven't run the tests yet, but I think it should work.

[manuel-plavsic]

Actually, I am not so sure my suggestion works with non-nullable T.

[nank1ro]

I actually fixed with

    if (changed && newValue is T) {
      _setValue(newValue);
    }

This is safe, the new test passes! thank you so much for your suggestions 💙

[manuel-plavsic]

Yes, that code looks really good to me! Great that the test passes! I really like this library 💙