Skip to content

Commit

Permalink
fix(eslint-plugin): avoid-mapping-selectors don't report on ThisExpre…
Browse files Browse the repository at this point in the history
…ssion (#3546)

Closes #3511
  • Loading branch information
timdeschryver authored Aug 28, 2022
1 parent 9768551 commit a28175c
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 15 deletions.
64 changes: 54 additions & 10 deletions modules/eslint-plugin/spec/rules/avoid-mapping-selectors.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,27 +118,71 @@ export class CollectionService {
import { Store } from '@ngrx/store'
class OkBecauseOfThis {
foo$ = this.store.select(selectIsAuthenticated).pipe(
storeSelect$ = this.store.select(selectIsAuthenticated).pipe(
take(1),
map((isAuthenticated: boolean) => (isAuthenticated ? true : this.router.parseUrl('/login'))),
)
pipeSelect$ = this.store.pipe(
select(selectIsAuthenticated),
take(1),
map((isAuthenticated: boolean) => (isAuthenticated ? true : this.router.parseUrl('/login'))),
)
constructor(private store: Store) {}
}`,
`
import { Store } from '@ngrx/store'
class OkBecauseOfEffect {
foo$ = createEffect(() => {
return this.store.select(selectObj).pipe(
map((obj) => obj.prop),
distinctUntilChanged(),
map(() => anAction())
)
})
constructor(private store: Store) {}
storeSelect$ = createEffect(() => {
return this.store.select(selectObj).pipe(
map((obj) => obj.prop),
distinctUntilChanged(),
map(() => anAction())
)
})
pipeSelect$ = createEffect(() => {
return this.store.pipe(
select(selectObj),
map((obj) => obj.prop),
distinctUntilChanged(),
map(() => anAction())
)
})
constructor(private store: Store) {}
}`,
// https://github.com/ngrx/platform/issues/3511
`
import { Store } from '@ngrx/store'
class OkBecauseOfEffect {
storeSelect$ = createEffect(() => {
return this.store.select(selectObj).pipe(
switchMap(() => this.service.something()),
map((obj) => obj.prop),
)
})
pipeSelect$ = createEffect(() => {
return this.store.pipe(
select(selectObj),
exhaustMap(() => this.service.something()),
map((obj) => obj.prop),
)
})
somethingElse$ = createEffect(() => {
return this.store.pipe(
select(selectObj),
customOperator(() => this.prop),
map((obj) => obj.prop),
)
})
constructor(private store: Store, private service: Service) {}
}`,
];

Expand Down
34 changes: 29 additions & 5 deletions modules/eslint-plugin/src/rules/store/avoid-mapping-selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { createRule } from '../../rule-creator';
import {
asPattern,
getNgRxStores,
isMemberExpression,
namedCallableExpression,
pipeExpression,
} from '../../utils';
Expand Down Expand Up @@ -59,17 +60,40 @@ export default createRule<Options, MessageIds>({
return false;
}

let pipeHasThisExpression = false;

const selectorQuery = `:matches(${selectSelector}, ${pipeWithSelectAndMapSelector})`;
return {
[`:matches(${selectSelector}, ${pipeWithSelectAndMapSelector}) > CallExpression[callee.name='map']:not(:has(ThisExpression))`](
[`${selectorQuery} > CallExpression:has(ThisExpression)`](
node: TSESTree.CallExpression
) {
pipeHasThisExpression = true;
},
[`${selectorQuery}[callee.property.name=pipe]:exit`](
node: TSESTree.CallExpression
) {
if (pipeHasThisExpression) {
pipeHasThisExpression = false;
return;
}

if (isInCreateEffect(node)) {
return;
}
context.report({
node,
messageId,
});

const operators = node.arguments;
const mapOperator = operators.find(
(operator) =>
isCallExpression(operator) &&
isIdentifier(operator.callee) &&
operator.callee.name === 'map'
);
if (mapOperator) {
context.report({
node: mapOperator,
messageId,
});
}
},
};
},
Expand Down

0 comments on commit a28175c

Please sign in to comment.