Closed manuel-plavsic closed 1 year ago
:exclamation: No coverage uploaded for pull request base (
dev@928b5f1
). Click here to learn what that means. The diff coverage isn/a
.
@@ Coverage Diff @@
## dev #36 +/- ##
======================================
Coverage ? 97.72%
======================================
Files ? 17
Lines ? 704
Branches ? 0
======================================
Hits ? 688
Misses ? 16
Partials ? 0
Done :)
Thank you so much for your contributions, I really appreciate it! 👏
Btw, I think that we still need a getter, otherwise one could manually change the value of hasPreviousValue
, for example:
@override
void initState() {
super.initState();
counter = createSignal(0);
counter.hasPreviousValue = true;
}
I will make another PR soon if it is fine for you.
That is so good to hear, thanks! I really enjoy contributing to this amazing library! Btw, I sent you a DM on Twitter a few days ago ;)
You're right I hadn't thought about it that the value can be changed from outside.
Maybe you can put a getter in the ReadSignal
that returns false.
Then in the Signal
you may have a private _hasPreviousSignal
value and then override the getter returning this private variable.
I didn't get any DM on Twitter 😕 , this is my profile https://twitter.com/nank1ro
I don't know what is going on at Twitter... I can see the message... 😂 I will open a GitHub discussion instead.
I have a question regarding the next pull request: why should the getter in ReadSignal
return false instead of "correctly" returning true or false. Also, I don't understand why previousValue
is a getter that returns null. I see that it has something to do with it being a ReadSignal
and not a WriteSignal
, but the value held by the ReadSignal
can also change.
Edit: it is actually immutable. I confused Computed
with ReadSignal
, since createComputed
returns a ReadSignal
.
Don't worry I'm going to do the PR myself cause I have to release a new dev.
This PR adds the
hasPreviousValue
getter suggested in https://github.com/nank1ro/solidart/pull/27#issuecomment-1588990715.It is a bit unclear to me whether or not I should make a
reportObserved
call before returning_hasPreviousValue
(similarly to what thepreviousValue
getter does).