diff --git a/README.md b/README.md index db84925f..9e2f399f 100644 --- a/README.md +++ b/README.md @@ -74,52 +74,53 @@ The package includes the following rules. πŸ’­ Requires [type information](https://typescript-eslint.io/linting/typed-linting).\ ❌ Deprecated. -| NameΒ Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β  | Description | πŸ’Ό | πŸ”§ | πŸ’‘ | πŸ’­ | ❌ | -| :--------------------------------------------------------------------- | :-------------------------------------------------------------------------------------------------------- | :--- | :- | :- | :- | :- | -| [ban-observables](docs/rules/ban-observables.md) | Disallow banned observable creators. | | | | | | -| [ban-operators](docs/rules/ban-operators.md) | Disallow banned operators. | | | | πŸ’­ | | -| [finnish](docs/rules/finnish.md) | Enforce Finnish notation. | | | | πŸ’­ | | -| [just](docs/rules/just.md) | Require the use of `just` instead of `of`. | | πŸ”§ | | | | -| [macro](docs/rules/macro.md) | Require the use of the RxJS Tools Babel macro. | | πŸ”§ | | | ❌ | -| [no-async-subscribe](docs/rules/no-async-subscribe.md) | Disallow passing `async` functions to `subscribe`. | βœ… πŸ”’ | | | πŸ’­ | | -| [no-compat](docs/rules/no-compat.md) | Disallow the `rxjs-compat` package. | | | | | ❌ | -| [no-connectable](docs/rules/no-connectable.md) | Disallow operators that return connectable observables. | | | | πŸ’­ | | -| [no-create](docs/rules/no-create.md) | Disallow the static `Observable.create` function. | βœ… πŸ”’ | | | πŸ’­ | | -| [no-cyclic-action](docs/rules/no-cyclic-action.md) | Disallow cyclic actions in effects and epics. | | | | πŸ’­ | | -| [no-explicit-generics](docs/rules/no-explicit-generics.md) | Disallow unnecessary explicit generic type arguments. | | | | | | -| [no-exposed-subjects](docs/rules/no-exposed-subjects.md) | Disallow public and protected subjects. | πŸ”’ | | | πŸ’­ | | -| [no-finnish](docs/rules/no-finnish.md) | Disallow Finnish notation. | | | | πŸ’­ | | -| [no-floating-observables](docs/rules/no-floating-observables.md) | Require Observables to be handled appropriately. | πŸ”’ | | | πŸ’­ | | -| [no-ignored-default-value](docs/rules/no-ignored-default-value.md) | Disallow using `firstValueFrom`, `lastValueFrom`, `first`, and `last` without specifying a default value. | πŸ”’ | | | πŸ’­ | | -| [no-ignored-error](docs/rules/no-ignored-error.md) | Disallow calling `subscribe` without specifying an error handler. | πŸ”’ | | | πŸ’­ | | -| [no-ignored-notifier](docs/rules/no-ignored-notifier.md) | Disallow observables not composed from the `repeatWhen` or `retryWhen` notifier. | βœ… πŸ”’ | | | πŸ’­ | | -| [no-ignored-replay-buffer](docs/rules/no-ignored-replay-buffer.md) | Disallow using `ReplaySubject`, `publishReplay` or `shareReplay` without specifying the buffer size. | βœ… πŸ”’ | | | | | -| [no-ignored-subscribe](docs/rules/no-ignored-subscribe.md) | Disallow calling `subscribe` without specifying arguments. | | | | πŸ’­ | | -| [no-ignored-subscription](docs/rules/no-ignored-subscription.md) | Disallow ignoring the subscription returned by `subscribe`. | | | | πŸ’­ | | -| [no-ignored-takewhile-value](docs/rules/no-ignored-takewhile-value.md) | Disallow ignoring the value within `takeWhile`. | βœ… πŸ”’ | | | | | -| [no-implicit-any-catch](docs/rules/no-implicit-any-catch.md) | Disallow implicit `any` error parameters in `catchError` operators. | βœ… πŸ”’ | πŸ”§ | πŸ’‘ | πŸ’­ | | -| [no-index](docs/rules/no-index.md) | Disallow importing index modules. | βœ… πŸ”’ | | | | | -| [no-internal](docs/rules/no-internal.md) | Disallow importing internal modules. | βœ… πŸ”’ | πŸ”§ | πŸ’‘ | | | -| [no-misused-observables](docs/rules/no-misused-observables.md) | Disallow Observables in places not designed to handle them. | πŸ”’ | | | πŸ’­ | | -| [no-nested-subscribe](docs/rules/no-nested-subscribe.md) | Disallow calling `subscribe` within a `subscribe` callback. | βœ… πŸ”’ | | | πŸ’­ | | -| [no-redundant-notify](docs/rules/no-redundant-notify.md) | Disallow sending redundant notifications from completed or errored observables. | βœ… πŸ”’ | | | πŸ’­ | | -| [no-sharereplay](docs/rules/no-sharereplay.md) | Disallow unsafe `shareReplay` usage. | βœ… πŸ”’ | | | | | -| [no-subclass](docs/rules/no-subclass.md) | Disallow subclassing RxJS classes. | πŸ”’ | | | πŸ’­ | | -| [no-subject-unsubscribe](docs/rules/no-subject-unsubscribe.md) | Disallow calling the `unsubscribe` method of subjects. | βœ… πŸ”’ | | | πŸ’­ | | -| [no-subject-value](docs/rules/no-subject-value.md) | Disallow accessing the `value` property of a `BehaviorSubject` instance. | | | | πŸ’­ | | -| [no-subscribe-handlers](docs/rules/no-subscribe-handlers.md) | Disallow passing handlers to `subscribe`. | | | | πŸ’­ | | -| [no-subscribe-in-pipe](docs/rules/no-subscribe-in-pipe.md) | Disallow calling of `subscribe` within any RxJS operator inside a `pipe`. | βœ… πŸ”’ | | | πŸ’­ | | -| [no-tap](docs/rules/no-tap.md) | Disallow the `tap` operator. | | | | | ❌ | -| [no-topromise](docs/rules/no-topromise.md) | Disallow use of the `toPromise` method. | βœ… πŸ”’ | | πŸ’‘ | πŸ’­ | | -| [no-unbound-methods](docs/rules/no-unbound-methods.md) | Disallow passing unbound methods. | βœ… πŸ”’ | | | πŸ’­ | | -| [no-unsafe-catch](docs/rules/no-unsafe-catch.md) | Disallow unsafe `catchError` usage in effects and epics. | | | | πŸ’­ | | -| [no-unsafe-first](docs/rules/no-unsafe-first.md) | Disallow unsafe `first`/`take` usage in effects and epics. | | | | πŸ’­ | | -| [no-unsafe-subject-next](docs/rules/no-unsafe-subject-next.md) | Disallow unsafe optional `next` calls. | βœ… πŸ”’ | | | πŸ’­ | | -| [no-unsafe-switchmap](docs/rules/no-unsafe-switchmap.md) | Disallow unsafe `switchMap` usage in effects and epics. | | | | πŸ’­ | | -| [no-unsafe-takeuntil](docs/rules/no-unsafe-takeuntil.md) | Disallow applying operators after `takeUntil`. | βœ… πŸ”’ | | | πŸ’­ | | -| [prefer-observer](docs/rules/prefer-observer.md) | Disallow passing separate handlers to `subscribe` and `tap`. | βœ… πŸ”’ | πŸ”§ | πŸ’‘ | πŸ’­ | | -| [prefer-root-operators](docs/rules/prefer-root-operators.md) | Disallow importing operators from `rxjs/operators`. | βœ… πŸ”’ | πŸ”§ | πŸ’‘ | | | -| [suffix-subjects](docs/rules/suffix-subjects.md) | Enforce the use of a suffix in subject identifiers. | | | | πŸ’­ | | -| [throw-error](docs/rules/throw-error.md) | Enforce passing only `Error` values to `throwError` or `Subject.error`. | βœ… πŸ”’ | | | πŸ’­ | | +| NameΒ Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β  | Description | πŸ’Ό | πŸ”§ | πŸ’‘ | πŸ’­ | ❌ | +| :------------------------------------------------------------------------------- | :-------------------------------------------------------------------------------------------------------- | :--- | :- | :- | :- | :- | +| [ban-observables](docs/rules/ban-observables.md) | Disallow banned observable creators. | | | | | | +| [ban-operators](docs/rules/ban-operators.md) | Disallow banned operators. | | | | πŸ’­ | | +| [finnish](docs/rules/finnish.md) | Enforce Finnish notation. | | | | πŸ’­ | | +| [just](docs/rules/just.md) | Require the use of `just` instead of `of`. | | πŸ”§ | | | | +| [macro](docs/rules/macro.md) | Require the use of the RxJS Tools Babel macro. | | πŸ”§ | | | ❌ | +| [no-async-subscribe](docs/rules/no-async-subscribe.md) | Disallow passing `async` functions to `subscribe`. | βœ… πŸ”’ | | | πŸ’­ | | +| [no-compat](docs/rules/no-compat.md) | Disallow the `rxjs-compat` package. | | | | | ❌ | +| [no-connectable](docs/rules/no-connectable.md) | Disallow operators that return connectable observables. | | | | πŸ’­ | | +| [no-create](docs/rules/no-create.md) | Disallow the static `Observable.create` function. | βœ… πŸ”’ | | | πŸ’­ | | +| [no-cyclic-action](docs/rules/no-cyclic-action.md) | Disallow cyclic actions in effects and epics. | | | | πŸ’­ | | +| [no-explicit-generics](docs/rules/no-explicit-generics.md) | Disallow unnecessary explicit generic type arguments. | | | | | | +| [no-exposed-subjects](docs/rules/no-exposed-subjects.md) | Disallow public and protected subjects. | πŸ”’ | | | πŸ’­ | | +| [no-finnish](docs/rules/no-finnish.md) | Disallow Finnish notation. | | | | πŸ’­ | | +| [no-floating-observables](docs/rules/no-floating-observables.md) | Require Observables to be handled appropriately. | πŸ”’ | | | πŸ’­ | | +| [no-ignored-default-value](docs/rules/no-ignored-default-value.md) | Disallow using `firstValueFrom`, `lastValueFrom`, `first`, and `last` without specifying a default value. | πŸ”’ | | | πŸ’­ | | +| [no-ignored-error](docs/rules/no-ignored-error.md) | Disallow calling `subscribe` without specifying an error handler. | πŸ”’ | | | πŸ’­ | | +| [no-ignored-notifier](docs/rules/no-ignored-notifier.md) | Disallow observables not composed from the `repeatWhen` or `retryWhen` notifier. | βœ… πŸ”’ | | | πŸ’­ | | +| [no-ignored-replay-buffer](docs/rules/no-ignored-replay-buffer.md) | Disallow using `ReplaySubject`, `publishReplay` or `shareReplay` without specifying the buffer size. | βœ… πŸ”’ | | | | | +| [no-ignored-subscribe](docs/rules/no-ignored-subscribe.md) | Disallow calling `subscribe` without specifying arguments. | | | | πŸ’­ | | +| [no-ignored-subscription](docs/rules/no-ignored-subscription.md) | Disallow ignoring the subscription returned by `subscribe`. | | | | πŸ’­ | | +| [no-ignored-takewhile-value](docs/rules/no-ignored-takewhile-value.md) | Disallow ignoring the value within `takeWhile`. | βœ… πŸ”’ | | | | | +| [no-implicit-any-catch](docs/rules/no-implicit-any-catch.md) | Disallow implicit `any` error parameters in `catchError` operators. | βœ… πŸ”’ | πŸ”§ | πŸ’‘ | πŸ’­ | | +| [no-index](docs/rules/no-index.md) | Disallow importing index modules. | βœ… πŸ”’ | | | | | +| [no-internal](docs/rules/no-internal.md) | Disallow importing internal modules. | βœ… πŸ”’ | πŸ”§ | πŸ’‘ | | | +| [no-misused-observables](docs/rules/no-misused-observables.md) | Disallow Observables in places not designed to handle them. | πŸ”’ | | | πŸ’­ | | +| [no-nested-subscribe](docs/rules/no-nested-subscribe.md) | Disallow calling `subscribe` within a `subscribe` callback. | βœ… πŸ”’ | | | πŸ’­ | | +| [no-redundant-notify](docs/rules/no-redundant-notify.md) | Disallow sending redundant notifications from completed or errored observables. | βœ… πŸ”’ | | | πŸ’­ | | +| [no-sharereplay](docs/rules/no-sharereplay.md) | Disallow unsafe `shareReplay` usage. | βœ… πŸ”’ | | | | | +| [no-sharereplay-before-takeuntil](docs/rules/no-sharereplay-before-takeuntil.md) | Disallow using `shareReplay({ refCount: false })` before `takeUntil`. | | | | | | +| [no-subclass](docs/rules/no-subclass.md) | Disallow subclassing RxJS classes. | πŸ”’ | | | πŸ’­ | | +| [no-subject-unsubscribe](docs/rules/no-subject-unsubscribe.md) | Disallow calling the `unsubscribe` method of subjects. | βœ… πŸ”’ | | | πŸ’­ | | +| [no-subject-value](docs/rules/no-subject-value.md) | Disallow accessing the `value` property of a `BehaviorSubject` instance. | | | | πŸ’­ | | +| [no-subscribe-handlers](docs/rules/no-subscribe-handlers.md) | Disallow passing handlers to `subscribe`. | | | | πŸ’­ | | +| [no-subscribe-in-pipe](docs/rules/no-subscribe-in-pipe.md) | Disallow calling of `subscribe` within any RxJS operator inside a `pipe`. | βœ… πŸ”’ | | | πŸ’­ | | +| [no-tap](docs/rules/no-tap.md) | Disallow the `tap` operator. | | | | | ❌ | +| [no-topromise](docs/rules/no-topromise.md) | Disallow use of the `toPromise` method. | βœ… πŸ”’ | | πŸ’‘ | πŸ’­ | | +| [no-unbound-methods](docs/rules/no-unbound-methods.md) | Disallow passing unbound methods. | βœ… πŸ”’ | | | πŸ’­ | | +| [no-unsafe-catch](docs/rules/no-unsafe-catch.md) | Disallow unsafe `catchError` usage in effects and epics. | | | | πŸ’­ | | +| [no-unsafe-first](docs/rules/no-unsafe-first.md) | Disallow unsafe `first`/`take` usage in effects and epics. | | | | πŸ’­ | | +| [no-unsafe-subject-next](docs/rules/no-unsafe-subject-next.md) | Disallow unsafe optional `next` calls. | βœ… πŸ”’ | | | πŸ’­ | | +| [no-unsafe-switchmap](docs/rules/no-unsafe-switchmap.md) | Disallow unsafe `switchMap` usage in effects and epics. | | | | πŸ’­ | | +| [no-unsafe-takeuntil](docs/rules/no-unsafe-takeuntil.md) | Disallow applying operators after `takeUntil`. | βœ… πŸ”’ | | | πŸ’­ | | +| [prefer-observer](docs/rules/prefer-observer.md) | Disallow passing separate handlers to `subscribe` and `tap`. | βœ… πŸ”’ | πŸ”§ | πŸ’‘ | πŸ’­ | | +| [prefer-root-operators](docs/rules/prefer-root-operators.md) | Disallow importing operators from `rxjs/operators`. | βœ… πŸ”’ | πŸ”§ | πŸ’‘ | | | +| [suffix-subjects](docs/rules/suffix-subjects.md) | Enforce the use of a suffix in subject identifiers. | | | | πŸ’­ | | +| [throw-error](docs/rules/throw-error.md) | Enforce passing only `Error` values to `throwError` or `Subject.error`. | βœ… πŸ”’ | | | πŸ’­ | | diff --git a/docs/rules/no-sharereplay-before-takeuntil.md b/docs/rules/no-sharereplay-before-takeuntil.md new file mode 100644 index 00000000..3fb4a322 --- /dev/null +++ b/docs/rules/no-sharereplay-before-takeuntil.md @@ -0,0 +1,40 @@ +# Disallow using `shareReplay({ refCount: false })` before `takeUntil` (`rxjs-x/no-sharereplay-before-takeuntil`) + + + +This rule effects failures if the `shareReplay` operator is used without its reference counting behavior +and placed in an observable composition before `takeUntil`. + +## Rule details + +Examples of **incorrect** code for this rule: + +```ts +source.pipe( + shareReplay(), + takeUntil(notifier), +); +``` + +Examples of **correct** code for this rule: + +```ts +source.pipe( + shareReplay({ refCount: true }), + takeUntil(notifier), +); +``` + +## When Not To Use It + +If you are confident your project uses `shareReplay` and `takeUntil` correctly, +then you may not need this rule. + +## Further reading + +- [Avoiding takeUntil leaks#An update](https://ncjamieson.com/avoiding-takeuntil-leaks/#an-update) + +## Resources + +- [Rule source](/src/rules/no-sharereplay-before-takeuntil.ts) +- [Test source](/tests/rules/no-sharereplay-before-takeuntil.test.ts) diff --git a/src/index.ts b/src/index.ts index cf693fa2..bf6b1d95 100644 --- a/src/index.ts +++ b/src/index.ts @@ -31,6 +31,7 @@ import { noMisusedObservablesRule } from './rules/no-misused-observables'; import { noNestedSubscribeRule } from './rules/no-nested-subscribe'; import { noRedundantNotifyRule } from './rules/no-redundant-notify'; import { noSharereplayRule } from './rules/no-sharereplay'; +import { noSharereplayBeforeTakeuntilRule } from './rules/no-sharereplay-before-takeuntil'; import { noSubclassRule } from './rules/no-subclass'; import { noSubjectUnsubscribeRule } from './rules/no-subject-unsubscribe'; import { noSubjectValueRule } from './rules/no-subject-value'; @@ -79,6 +80,7 @@ const plugin = { 'no-misused-observables': noMisusedObservablesRule, 'no-nested-subscribe': noNestedSubscribeRule, 'no-redundant-notify': noRedundantNotifyRule, + 'no-sharereplay-before-takeuntil': noSharereplayBeforeTakeuntilRule, 'no-sharereplay': noSharereplayRule, 'no-subclass': noSubclassRule, 'no-subject-unsubscribe': noSubjectUnsubscribeRule, diff --git a/src/rules/no-sharereplay-before-takeuntil.ts b/src/rules/no-sharereplay-before-takeuntil.ts new file mode 100644 index 00000000..692370f2 --- /dev/null +++ b/src/rules/no-sharereplay-before-takeuntil.ts @@ -0,0 +1,72 @@ +import { TSESTree as es } from '@typescript-eslint/utils'; +import { isCallExpression, isIdentifier, isLiteral, isMemberExpression, isObjectExpression, isProperty } from '../etc'; +import { ruleCreator } from '../utils'; + +export const noSharereplayBeforeTakeuntilRule = ruleCreator({ + defaultOptions: [], + meta: { + docs: { + description: 'Disallow using `shareReplay({ refCount: false })` before `takeUntil`.', + }, + messages: { + forbidden: 'shareReplay before takeUntil is forbidden unless \'refCount: true\' is specified.', + }, + schema: [], + type: 'problem', + }, + name: 'no-sharereplay-before-takeuntil', + create: (context) => { + return { + 'CallExpression[callee.name="shareReplay"]': (node: es.CallExpression) => { + const pipeCallExpression = node.parent as es.CallExpression; + if ( + !pipeCallExpression.arguments + || !isMemberExpression(pipeCallExpression.callee) + || !isIdentifier(pipeCallExpression.callee.property) + || pipeCallExpression.callee.property.name !== 'pipe' + ) { + return; + } + + const takeUntilIndex = pipeCallExpression.arguments.findIndex(arg => + isCallExpression(arg) + && isIdentifier(arg.callee) + && arg.callee.name === 'takeUntil', + ); + + if (takeUntilIndex === -1) { + return; + } + + const shareReplayConfig = node.arguments[0]; + if ( + !shareReplayConfig + || !isObjectExpression(shareReplayConfig) + ) { + // refCount defaults to false if no config is provided. + context.report({ + messageId: 'forbidden', + node: node.callee, + }); + return; + } + + const refCountElement = shareReplayConfig.properties + .filter(isProperty) + .find(prop => + isIdentifier(prop.key) + && prop.key.name === 'refCount'); + if ( + !refCountElement + || (isLiteral(refCountElement.value) + && refCountElement.value.value === false) + ) { + context.report({ + messageId: 'forbidden', + node: node.callee, + }); + } + }, + }; + }, +}); diff --git a/tests/rules/no-sharereplay-before-takeuntil.test.ts b/tests/rules/no-sharereplay-before-takeuntil.test.ts new file mode 100644 index 00000000..7d012512 --- /dev/null +++ b/tests/rules/no-sharereplay-before-takeuntil.test.ts @@ -0,0 +1,54 @@ +import { stripIndent } from 'common-tags'; +import { noSharereplayBeforeTakeuntilRule } from '../../src/rules/no-sharereplay-before-takeuntil'; +import { fromFixture } from '../etc'; +import { ruleTester } from '../rule-tester'; + +ruleTester({ types: false }).run('no-sharereplay-before-takeuntil', noSharereplayBeforeTakeuntilRule, { + valid: [ + stripIndent` + // with refCount true + import { of, takeUntil, shareReplay } from "rxjs"; + + const a = of("a"); + const b = of("b"); + + const t = true as const; + a.pipe(shareReplay({ refCount: t }), takeUntil(b)); + `, + stripIndent` + // unrelated + import { of, takeUntil, toArray } from "rxjs"; + + const a = of("a"); + const b = of("b"); + + a.pipe(takeUntil(b), toArray()); + `, + ], + invalid: [ + fromFixture( + stripIndent` + // without config + import { of, takeUntil, shareReplay } from "rxjs"; + + const a = of("a"); + const b = of("b"); + + a.pipe(shareReplay(), takeUntil(b)); + ~~~~~~~~~~~ [forbidden] + `, + ), + fromFixture( + stripIndent` + // with refCount false + import { of, takeUntil, shareReplay } from "rxjs"; + + const a = of("a"); + const b = of("b"); + + a.pipe(shareReplay({ refCount: false }), takeUntil(b)); + ~~~~~~~~~~~ [forbidden] + `, + ), + ], +});