Skip to content

Commit

Permalink
fix(no-ignored-replay-buffer): check config bufferSize (#12)
Browse files Browse the repository at this point in the history
Fixes cartant#100

There's also a proposed fix PR in the upstream repo
cartant#114 but I avoided
looking at that solution to avoid any license issue, so this solution
might be different.

- Fix: if the `shareReplay` operator was passed an object config, then
require `bufferSize` to be in that object.
- Fix: the rule wasn't handling if `shareReplay` is imported under a
namespace.
- (Might need to review the entire project. Since rxjs now recommends
importing from "rxjs" instead of "rxjs/operators", there's a risk that
many rules also fail to account for that new paradigm. Note that this
rule is the only rule that tests namespace imports for all cases, so we
at least should expand that coverage/type of test.)
  • Loading branch information
JasonWeinzierl authored Nov 8, 2024
1 parent 6870c8a commit ef2f886
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 3 deletions.
10 changes: 10 additions & 0 deletions docs/rules/no-ignored-replay-buffer.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ import { ReplaySubject } from "rxjs";
const subject = new ReplaySubject<number>();
```

```ts
import { of, shareReplay } from "rxjs";
of(42).pipe(shareReplay({ refCount: true }));
```

Examples of **correct** code for this rule:

```ts
Expand All @@ -26,3 +31,8 @@ const subject = new ReplaySubject<number>(1);
import { ReplaySubject } from "rxjs";
const subject = new ReplaySubject<number>(Infinity);
```

```ts
import { of, shareReplay } from "rxjs";
of(42).pipe(shareReplay({ refCount: true, bufferSize: 1 }));
```
32 changes: 29 additions & 3 deletions src/rules/no-ignored-replay-buffer.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { TSESTree as es } from '@typescript-eslint/utils';
import { AST_NODE_TYPES, TSESTree as es } from '@typescript-eslint/utils';
import { ruleCreator } from '../utils';

export const noIgnoredReplayBufferRule = ruleCreator({
Expand All @@ -17,16 +17,35 @@ export const noIgnoredReplayBufferRule = ruleCreator({
},
name: 'no-ignored-replay-buffer',
create: (context) => {
function checkShareReplayConfig(
node: es.Identifier,
shareReplayConfigArg: es.ObjectExpression,
) {
if (!shareReplayConfigArg.properties.some(p => p.type === AST_NODE_TYPES.Property && p.key.type === AST_NODE_TYPES.Identifier && p.key.name === 'bufferSize')) {
context.report({
messageId: 'forbidden',
node,
});
}
}

function checkNode(
node: es.Node,
{ arguments: args }: { arguments: es.Node[] },
node: es.Identifier,
{ arguments: args }: es.NewExpression | es.CallExpression,
) {
if (!args || args.length === 0) {
context.report({
messageId: 'forbidden',
node,
});
}

if (node.name === 'shareReplay' && args?.length === 1) {
const arg = args[0];
if (arg.type === AST_NODE_TYPES.ObjectExpression) {
checkShareReplayConfig(node, arg);
}
}
}

return {
Expand All @@ -49,6 +68,13 @@ export const noIgnoredReplayBufferRule = ruleCreator({
const callExpression = node.parent as es.CallExpression;
checkNode(node, callExpression);
},
'CallExpression > MemberExpression > Identifier[name=/^(publishReplay|shareReplay)$/]': (
node: es.Identifier,
) => {
const memberExpression = node.parent as es.MemberExpression;
const callExpression = memberExpression.parent as es.CallExpression;
checkNode(node, callExpression);
},
};
},
});
31 changes: 31 additions & 0 deletions tests/rules/no-ignored-replay-buffer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ ruleTester({ types: false }).run('no-ignored-replay-buffer', noIgnoredReplayBuff
const a = of(42).pipe(shareReplay(1));
`,
fromFixture(
stripIndent`
// shareReplay with config not ignored
import { interval, shareReplay } from "rxjs";
interval(1000).pipe(shareReplay({ bufferSize: 1, refCount: true }));
`,
),
stripIndent`
// namespace ReplaySubject not ignored
import * as Rx from "rxjs";
Expand All @@ -47,6 +54,13 @@ ruleTester({ types: false }).run('no-ignored-replay-buffer', noIgnoredReplayBuff
const a = Rx.of(42).pipe(shareReplay(1));
`,
stripIndent`
// namespace shareReplay with config not ignored
import * as Rx from "rxjs";
import { shareReplay } from "rxjs/operators";
const a = Rx.of(42).pipe(shareReplay({ bufferSize: 1, refCount: true }));
`,
stripIndent`
// namespace class not ignored
import * as Rx from "rxjs";
Expand Down Expand Up @@ -91,6 +105,15 @@ ruleTester({ types: false }).run('no-ignored-replay-buffer', noIgnoredReplayBuff
~~~~~~~~~~~ [forbidden]
`,
),
fromFixture(
stripIndent`
// shareReplay with config ignored
import { of, shareReplay } from "rxjs";
const a = of(42).pipe(shareReplay({ refCount: true }));
~~~~~~~~~~~ [forbidden]
`,
),
fromFixture(
stripIndent`
// namespace ReplaySubject ignored
Expand Down Expand Up @@ -122,6 +145,14 @@ ruleTester({ types: false }).run('no-ignored-replay-buffer', noIgnoredReplayBuff
~~~~~~~~~~~ [forbidden]
`,
),
fromFixture(
stripIndent`
// namespace shareReplay with config ignored
import * as Rx from "rxjs";
const a = Rx.of(42).pipe(Rx.shareReplay({ refCount: true }));
~~~~~~~~~~~ [forbidden]
`,
),
fromFixture(
stripIndent`
// namespace class ignored
Expand Down

0 comments on commit ef2f886

Please sign in to comment.