Rxjs: defer(() => undefined) should be a TS error

Created on 19 May 2020  路  3Comments  路  Source: ReactiveX/rxjs

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.

bug

Most helpful comment

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.

All 3 comments

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.

Was this page helpful?
0 / 5 - 0 ratings