-
Notifications
You must be signed in to change notification settings - Fork 3
No nested pipe similar to no-nested-subscribe #134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| # Avoid nested `pipe` calls (`no-nested-pipe`) | ||
|
|
||
| This rule effects failures if `pipe` is called within a `pipe` handler. | ||
|
|
||
| ## Rule details | ||
|
|
||
| Examples of **incorrect** code for this rule: | ||
|
|
||
| ```ts | ||
| import { switchMap, map, of } from 'rxjs'; | ||
|
|
||
| of('searchText1', 'searchText2') | ||
| .pipe( | ||
| switchMap(searchText => { | ||
| return callSearchAPI(searchText).pipe( | ||
| map(response => { | ||
| console.log(response); | ||
| return 'final ' + response; | ||
| // considering more lines here | ||
| }) | ||
| ); | ||
| }) | ||
| ) | ||
| .subscribe(value => console.log(value)); | ||
|
|
||
| function callSearchAPI(searchText) { | ||
| return of('new' + searchText); | ||
| } | ||
|
|
||
|
|
||
| ``` | ||
|
|
||
| Examples of **correct** code for this rule: | ||
|
|
||
| ```ts | ||
| import { switchMap, map, of } from 'rxjs'; | ||
|
|
||
| of('searchText1', 'searchText2') | ||
| .pipe( | ||
| switchMap(searchText => { | ||
| return callSearchAPI(searchText); | ||
| }) | ||
| ) | ||
| .subscribe(value => console.log(value)); | ||
|
|
||
| function callSearchAPI(searchText) { | ||
| return of('new' + searchText).pipe( | ||
| map(response => { | ||
| console.log(response); | ||
| return 'final ' + response; | ||
| // considering more lines here | ||
| }) | ||
| ); | ||
| } | ||
|
|
||
| ``` | ||
|
|
||
| ## Options | ||
|
|
||
| This rule has no options. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| /** | ||
| * @license Use of this source code is governed by an MIT-style license that | ||
| * can be found in the LICENSE file at https://github.com/cartant/eslint-plugin-rxjs | ||
| */ | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This license header can be removed (and in the test file too). |
||
|
|
||
| import { TSESTree as es } from "@typescript-eslint/experimental-utils"; | ||
|
Check failure on line 6 in source/rules/no-nested-pipe.ts
|
||
| import { getParent, getTypeServices } from "eslint-etc"; | ||
|
Check failure on line 7 in source/rules/no-nested-pipe.ts
|
||
| import { ruleCreator } from "../utils"; | ||
|
Check failure on line 8 in source/rules/no-nested-pipe.ts
|
||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we've upgraded typescript-eslint and dropped the dependency on eslint-etc, you'll need slightly separate imports: import { TSESTree as es } from '@typescript-eslint/utils';
import { getTypeServices } from '../etc';
...And instead of |
||
|
|
||
| const rule = ruleCreator({ | ||
| defaultOptions: [], | ||
| meta: { | ||
| docs: { | ||
| description: "Forbids the calling of `pipe` within a `pipe` callback.", | ||
| recommended: "error", | ||
| }, | ||
| fixable: undefined, | ||
| hasSuggestions: false, | ||
| messages: { | ||
| forbidden: "Nested pipe calls are forbidden.", | ||
| }, | ||
| schema: [], | ||
| type: "problem", | ||
| }, | ||
| name: "no-nested-pipe", | ||
| create: (context) => { | ||
|
Check failure on line 26 in source/rules/no-nested-pipe.ts
|
||
| const { couldBeObservable, couldBeType } = getTypeServices(context); | ||
| const argumentsMap = new WeakMap<es.Node, void>(); | ||
| return { | ||
| [`CallExpression > MemberExpression[property.name='pipe']`]: ( | ||
| node: es.MemberExpression | ||
| ) => { | ||
| if ( | ||
| !couldBeObservable(node.object) && | ||
| !couldBeType(node.object, "Pipeable") | ||
| ) { | ||
| return; | ||
| } | ||
| const callExpression = getParent(node) as es.CallExpression; | ||
| let parent = getParent(callExpression); | ||
| while (parent) { | ||
| if (argumentsMap.has(parent)) { | ||
| context.report({ | ||
| messageId: "forbidden", | ||
| node: node.property, | ||
| }); | ||
| return; | ||
| } | ||
| parent = getParent(parent); | ||
| } | ||
| for (const arg of callExpression.arguments) { | ||
| argumentsMap.set(arg); | ||
| } | ||
| }, | ||
| }; | ||
| }, | ||
| }); | ||
|
|
||
| export = rule; | ||
|
Check failure on line 59 in source/rules/no-nested-pipe.ts
|
||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're exporting rules slightly different to support ESM too. Do: export const noNestedPipeRule = ruleCreator({
... |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| /** | ||
| * @license Use of this source code is governed by an MIT-style license that | ||
| * can be found in the LICENSE file at https://github.com/cartant/eslint-plugin-rxjs | ||
| */ | ||
|
|
||
| import { stripIndent } from "common-tags"; | ||
| import { fromFixture } from "eslint-etc"; | ||
|
Check failure on line 7 in tests/rules/no-nested-pipe.ts
|
||
| import rule = require("../../source/rules/no-nested-pipe"); | ||
|
Check failure on line 8 in tests/rules/no-nested-pipe.ts
|
||
| import { ruleTester } from "../utils"; | ||
|
Check failure on line 9 in tests/rules/no-nested-pipe.ts
|
||
|
|
||
| ruleTester({ types: true }).run("no-nested-pipe", rule, { | ||
| valid: [ | ||
| stripIndent` | ||
| // not nested in pipe | ||
| import { Observable,of,switchMap } from "rxjs"; | ||
| of(47).pipe(switchMap(value => { | ||
| console.log('new' ,value); | ||
| })).subscribe(value => { | ||
| console.log(value); | ||
| }) | ||
| `, | ||
| stripIndent` | ||
| // not nested in pipe | ||
| import { Observable,switchMap } from "rxjs"; | ||
| of(47).pipe(switchMap(value => { | ||
| return someFunction(value) | ||
| })).subscribe(value => { | ||
| console.log(value); | ||
| }); | ||
| function someFunction(someParam: Observable<any>): Observable<any> { | ||
| return of(43).pipe( | ||
| switchMap(value => {value + someParam}) | ||
| ) | ||
| } | ||
| `, | ||
| stripIndent` | ||
| // not nested in pipe using function move to separate function | ||
| import { Observable,switchMap } from "rxjs"; | ||
| of(47).pipe(switchMap(value => { | ||
| return someFunction1(value) | ||
| }), | ||
| switchMap(value => { | ||
| return someFunction2(value) | ||
| }) | ||
| ).subscribe(value => { | ||
| console.log(value); | ||
| }); | ||
| function someFunction1(someParam: Observable<any>): Observable<any> { | ||
| return of(43).pipe( | ||
| switchMap(value => {value + someParam}) | ||
| ) | ||
| }; | ||
| function someFunction2(someParam: Observable<any>): Observable<any> { | ||
| return of(43).pipe( | ||
| switchMap(value => {value + someParam}) | ||
| ) | ||
| } | ||
| `, | ||
| ], | ||
| invalid: [ | ||
| fromFixture( | ||
| stripIndent` | ||
| // nested in pipe | ||
| import { of,switchMap,tap } from "rxjs"; | ||
| of("foo").pipe( | ||
| switchMap(value => { | ||
| return of("bar").pipe(tap(value => { console.log(value)}) | ||
| ~~~~ [forbidden] | ||
| )}) | ||
| ).subscribe(value => { | ||
| console.log(value); | ||
| }); | ||
| ` | ||
| ), | ||
| fromFixture( | ||
| stripIndent` | ||
| // nested in pipe with parallel pipes | ||
| import { of,switchMap,tap } from "rxjs"; | ||
| of("foo").pipe( | ||
| switchMap(value => { | ||
| return of("bar").pipe(tap(value => { console.log(value)}) | ||
| ~~~~ [forbidden] | ||
| )}), | ||
| switchMap(value => { | ||
| return of("bar").pipe(tap(value => { console.log(value)}) | ||
| ~~~~ [forbidden] | ||
| )}) | ||
| ).subscribe(value => { | ||
| console.log(value); | ||
| }); | ||
| ` | ||
| ), | ||
| ], | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll need to add the rule to
index.ts(we have a unit test to ensure it gets added).