Issue after upgrading to 0.7.2 from 0.7.0

[jochenjonc]

Hi,

The change in #185 gives another kind of false positive, which I think worked before.

When you have a function with multiple signatures, like this:

export interface Hook {
  onInit?: (field: Field) => void | ((field: Field) => Observable<any>);
}

It doesn't seem to be able to distinguish between the one that returns void and the one that returns the Observable.

When I reorder it, like this:

export interface Hook {
  onInit?: ((field: Field) => Observable<any>) | (field: Field) => void;
}

It does work (I don't have a linting error).

But the sample I give is not part of code that I have under control, it comes from the Formly library. See https://github.com/ngx-formly/ngx-formly/blob/main/src/core/src/lib/models/fieldconfig.ts#L249-L257

Would it be possible to handle such cases in eslint-plugin-rxjs-x?

[JasonWeinzierl]

Is this for the finnish rule? Could you provide an example of how you're implementing this interface (e.g. is your onInit returning an Observable or void)?

[JasonWeinzierl]

(Might be unrelated but documenting this here: seems there's no finnish test coverage for arrow function declarations. VariableDeclarator > Identifier to couldReturnObservable is always false. Not sure if this is missing test coverage or intentional behavior? This is reported in #66)

[jochenjonc]

It's not for the finnish rule, but for the no-misused-observables. And my onInit is returning an Observable. I'll try to create an example.

[JasonWeinzierl]

I tried reproducing this in the no-misused-observables tests, and for example, this doesn't error:

      import { Observable, of } from "rxjs";

      interface Hook {
        onInit?: (field: string) => void | ((field: string) => Observable<any>);
      }

      const hook: Hook = {
        onInit: field => of(field),
      };

Do you have a different example?
Also, could this be a different TypeScript version problem (or some other package)? We're running the unit tests on TypeScript 5.8.3; which versions are you using?

[jochenjonc]

Hi,

The scenario you are describing works, but this little twist doesn't:

      import { Observable, of } from "rxjs";

      export type FormlyHookFn = (field: FormlyFieldConfig) => void;

      interface Hook {
        onInit?: FormlyHookFn | ((field: string) => Observable<any>);
      }

      const hook: Hook = {
        onInit: field => of(field),
      };

Regards,

Jochen

[JasonWeinzierl]

Ah yep my example was missing parenthesis so it wasn't triggering the error.

Minimum steps to reproduce:

      import { Observable, of } from "rxjs";

      interface Hook {
        onInit?: ((field: string) => void) | ((field: string) => Observable<any>);
      }

      const hook: Hook = {
        onInit: field => of(field),
      };

[JasonWeinzierl]

Created #236 to fix, feel free to review.

Interestingly, this bug was only revealed by the 0.72 release (#185), but this bug would also have been triggered in <= 0.71 if you specified an explicit return type, e.g.

  onInit: (field: string): Observable<string> => of(field)

#185 only enhanced this rule to begin noticing implicit return types too.

[JasonWeinzierl]

The fix is available in v0.7.7