bug(`withResource` and Mutations): `hasValue()` does not narrow the respective `value` signal

[michael-small]

Regular resources have hasValue to narrow undefined out of the value

const value = someResource.value(); // string | undefined

if (someResource.hasValue()) {
    const valueNarrowed = someResource.value(); // string
}

withResource and the Mutations (httpMutation and rxMutation) both have a hasValue(), but upon doing some work recently, we noticed that the type narrowing does not work.

Until fix: use usual narrowing strategies that invoke the signal before trying to conditionally use them.

[jdegand]

The use of this should be looked at. It might evaluate to the store vs the signal. Also, some of the type guards seem to limit the parameter vs the signal itself.

[rainerhahnekamp]

Yes, that's a tricky one. @jdegand if you have some ideas, please let us know

[jdegand]

@rainerhahnekamp I'd search the Angular Repo for hasValue, etc. and look at some issues they have. There usually is a lot of information provided in issues that is not documented elsewhere. One thing that I have seen is the use of Exclude like hasValue(): this is ResourceRef<Exclude<T, undefined>>;

[rainerhahnekamp]

Yeah, I remember that I've tried that as well, but didn't work. The issue with the SignalStore is that the typing is quite complicated. But please, if you have a solution, don't hesitate an open a PR. Would be great!

[robmanganelly]

probably off topic, but this formula:

const value = someResource.value(); // string | undefined

if (someResource.hasValue()) {
    const valueNarrowed = someResource.value(); // string
}

is potentially unsafe

if you read the value before making sure it has one, it could throw.
I remember seeing such discussion in a thread in angular

One of the maintainers adviced to check hasValue before attempting to read the rs value

// T | undefined
const safeValue = computed(()=>rs.hasValue() ? rs.value() : undefined) 

// T
const safeValue = computed(()=>rs.hasValue() ? rs.value() : 'defaultValue' /* T */)

perhaps that formula can solve both the chance of throwing and the type issue,

Another thing I recall reading in angular is that type narrowing does not work in signals the same way it does in regular variables

Every time you read a signal, the type is inferred from the signal function, regardless of narrowing to previous reads

function isString(value: unknown): value is string {
  return typeof value === 'string';
}

// with regular variables
const someValue: unknown = 'foo'

if(isString(someValue)){ console.log(someValue.toUpperCase())}

// with signals , won't work

const anotherValue = signal<unknown>('foo')

if(isString(anotherValue())){ console.log(anotherValue().toUpperCase())} // should give type issues


// a workaround is reading before narrowing

const staticValue = anotherValue();

if(isString(staticValue)){ console.log(staticValue.toUpperCase())} // should NOT give type issues

This said, I don't know if we can call this a problem, or the problem really belongs to this project, even though we could certainly implement the mentioned workarounds here...
I'm willing to submit a PR if you see it appropriate.

[rainerhahnekamp]

Hey,

why do you see this as unsafe?

if (someResource.hasValue()) {
    const valueNarrowed = someResource.value(); // string
}

The hasValue is actually a type predicate (and has been implemented as one), which removes undefined from the resource's value. For some reason, TypeScript doesn't recognize it.

So I think we just have a problem with the types but not with the approach.

[robmanganelly]

Hi,

that part is ok. I was referring to the first line of the initial comment

const value = someResource.value(); // string | undefined

the block inside hasValue is safe

in the current state of the resource api, value is not "safe", in the sense it could throw if the resource does.

see angular/angular#62687 and angular/angular#62065

I can't tell if that will be the definitive behavior, but the advice from Matthieu Riegler at the time was to access the value inside a hasValue condition to ensure never throwing, something like this

// T
const safeValue = computed(()=>rs.hasValue() ? rs.value() : 'defaultValue' /* T */)

which could solve both the narrowing issue and prevent throwing on resource error

I apologize if the message lead to misunderstandings

[rainerhahnekamp]

Ah, yes, yes, accessing a value without proper check is always a high risk. I am not happy with the resource's error behavior, but it is what it is...

You can also use a linkedSignal if you want to return the previous value instead the default one, but in the end, we do have the problem with withResource, that hasValue doesn't work as it does for the normal Resource.