Currently the following results in Observable<never>, which is very incorrect:
defer(() => undefined);
I would expect a TypeScript error there. As the developer should be returning some form of ObservableInput from that function.
But the implementation accounts for factories that return a falsy - i.e. non-ObservableInput - value:
https://github.com/ReactiveX/rxjs/blob/master/src/internal/observable/defer.ts#L64
Why?
I literally have no idea. haha. A late night? It's clearly incorrect. defer(() => undefined) has no real meaning. Even in terms like defer(() => { if (test) { return something$ } }), it's not so hard to return EMPTY or of() there, if that's what you really wanted.
Here's a concrete reason I want it changed:
I was refactoring some code, and I had some defer calls, where I tried to put something ahead of the returned value:
Before, it looked like this. And everything was fine in my IDE because I was returning something that was the shape expected by the return type: Observable<VerySpecificThing[]>. Like so:
function getVerySpecificThings(...args: any[]): Observable<VerySpecificThing[]>) {
return defer(() =>
from(dataFetch(calculateThing(args[0]))).pipe(
/* a bunch of things to get the right shape */
/* error handling etc */
)
);
}
After I refactored to add something before the defer, I messed it up, but it all still passed, because it was returning an Observable<never>:
function getVerySpecificThings(...args: any[]): Observable<VerySpecificThing[]>) {
return defer(() => {
const startTIme = Date.now();
from(dataFetch(calculateThing(args[0]))).pipe(
/* a bunch of things to get the right shape */
/* error handling etc */
)
});
}
Notice I missed the return in front of the from there? No bueno. It actually took me a bit to see the problem, because there were no type errors and no runtime errors. Better types would have caught that. Also, if the runtime wasn't doing something stupid there it would have caught that.
I think we need to fix this. It's a footgun, IMO.
Most helpful comment
Here's a concrete reason I want it changed:
I was refactoring some code, and I had some
defercalls, where I tried to put something ahead of the returned value:Before, it looked like this. And everything was fine in my IDE because I was returning something that was the shape expected by the return type:
Observable<VerySpecificThing[]>. Like so:After I refactored to add something before the defer, I messed it up, but it all still passed, because it was returning an
Observable<never>:Notice I missed the
returnin front of thefromthere? No bueno. It actually took me a bit to see the problem, because there were no type errors and no runtime errors. Better types would have caught that. Also, if the runtime wasn't doing something stupid there it would have caught that.I think we need to fix this. It's a footgun, IMO.