Skip to content

Commit 636c922

Browse files
fix(no-misused-observables): false positive for JSX attributes (#239)
Missed corner case from the original implementation of this rule when imitating `no-misused-promises`. We didn't add unit tests for the non-void case, so this was triggering the rule: ```tsx import { Observable, of } from "rxjs"; interface Props { foo: () => Observable<number>; } declare function Component(props: Props): any; const _ = <Component foo={() => of(42)} />; ``` essentially blocking _any_ prop from ever being an Observable-returning function. We need to check if the contextual type of the prop is void-returning _and_ if the expression returns an Observable before reporting a violation.
1 parent 8e06ed9 commit 636c922

File tree

2 files changed

+59
-38
lines changed

2 files changed

+59
-38
lines changed

src/rules/no-misused-observables.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -180,11 +180,21 @@ export const noMisusedObservablesRule = ruleCreator({
180180
}
181181

182182
function checkJSXAttribute(node: es.JSXAttribute): void {
183-
if (!node.value || !isJSXExpressionContainer(node.value)) {
183+
if (
184+
node.value == null
185+
|| !isJSXExpressionContainer(node.value)
186+
) {
184187
return;
185188
}
186-
187-
if (couldReturnObservable(node.value.expression)) {
189+
const expressionContainer = esTreeNodeToTSNodeMap.get(
190+
node.value,
191+
);
192+
const contextualType = checker.getContextualType(expressionContainer);
193+
if (
194+
contextualType != null
195+
&& isVoidReturningFunctionType(contextualType)
196+
&& couldReturnObservable(node.value.expression)
197+
) {
188198
context.report({
189199
messageId: 'forbiddenVoidReturnAttribute',
190200
node: node.value,
@@ -336,8 +346,8 @@ export const noMisusedObservablesRule = ruleCreator({
336346
const tsNode = esTreeNodeToTSNodeMap.get(node);
337347
if (
338348
tsNode.initializer == null
339-
|| !node.init
340-
|| !node.id.typeAnnotation
349+
|| node.init == null
350+
|| node.id.typeAnnotation == null
341351
) {
342352
return;
343353
}

tests/rules/no-misused-observables.test.ts

Lines changed: 44 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -49,30 +49,43 @@ ruleTester({ types: true }).run('no-misused-observables', noMisusedObservablesRu
4949
{
5050
code: stripIndent`
5151
// void return attribute; explicitly allowed
52-
import { Observable, of } from "rxjs";
53-
import React, { FC } from "react";
52+
import { of } from "rxjs";
5453
55-
const Component: FC<{ foo: () => void }> = () => <div />;
56-
const App = () => {
57-
return (
58-
<Component foo={() => of(42)} />
59-
);
60-
};
54+
interface Props {
55+
foo: () => void;
56+
}
57+
declare function Component(props: Props): any;
58+
59+
const _ = <Component foo={() => of(42)} />;
6160
`,
6261
options: [{ checksVoidReturn: { attributes: false } }],
6362
languageOptions: { parserOptions: { ecmaFeatures: { jsx: true } } },
6463
},
64+
{
65+
code: stripIndent`
66+
// void return attribute; not void
67+
import { Observable, of } from "rxjs";
68+
69+
interface Props {
70+
foo: () => Observable<number>;
71+
}
72+
declare function Component(props: Props): any;
73+
74+
const _ = <Component foo={() => of(42)} />;
75+
`,
76+
languageOptions: { parserOptions: { ecmaFeatures: { jsx: true } } },
77+
},
6578
{
6679
code: stripIndent`
6780
// void return attribute; unrelated
68-
import React, { FC } from "react";
6981
70-
const Component: FC<{ foo: () => void, bar: boolean }> = () => <div />;
71-
const App = () => {
72-
return (
73-
<Component foo={() => 42} bar />
74-
);
75-
};
82+
interface Props {
83+
foo: () => void;
84+
bar: boolean;
85+
}
86+
declare function Component(props: Props): any;
87+
88+
const _ = <Component foo={() => 42} bar />;
7689
`,
7790
languageOptions: { parserOptions: { ecmaFeatures: { jsx: true } } },
7891
},
@@ -339,15 +352,14 @@ ruleTester({ types: true }).run('no-misused-observables', noMisusedObservablesRu
339352
stripIndent`
340353
// void return attribute; block body
341354
import { Observable, of } from "rxjs";
342-
import React, { FC } from "react";
343-
344-
const Component: FC<{ foo: () => void }> = () => <div />;
345-
const App = () => {
346-
return (
347-
<Component foo={(): Observable<number> => { return of(42); }} />
348-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [forbiddenVoidReturnAttribute]
349-
);
350-
};
355+
356+
interface Props {
357+
foo: () => void;
358+
}
359+
declare function Component(props: Props): any;
360+
361+
const _ = <Component foo={(): Observable<number> => { return of(42); }} />;
362+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [forbiddenVoidReturnAttribute]
351363
`,
352364
{
353365
languageOptions: { parserOptions: { ecmaFeatures: { jsx: true } } },
@@ -357,15 +369,14 @@ ruleTester({ types: true }).run('no-misused-observables', noMisusedObservablesRu
357369
stripIndent`
358370
// void return attribute; inline body
359371
import { Observable, of } from "rxjs";
360-
import React, { FC } from "react";
361-
362-
const Component: FC<{ foo: () => void }> = () => <div />;
363-
const App = () => {
364-
return (
365-
<Component foo={() => of(42)} />
366-
~~~~~~~~~~~~~~ [forbiddenVoidReturnAttribute]
367-
);
368-
};
372+
373+
interface Props {
374+
foo: () => void;
375+
}
376+
declare function Component(props: Props): any;
377+
378+
const _ = <Component foo={() => of(42)} />;
379+
~~~~~~~~~~~~~~ [forbiddenVoidReturnAttribute]
369380
`,
370381
{
371382
languageOptions: { parserOptions: { ecmaFeatures: { jsx: true } } },

0 commit comments

Comments
 (0)