build: add linting around unsafe takeUntil usage (#19720)

Adds a lint rule to catch unsafe usages of `takeUntil`.

Author: crisbeto

package.json

@@ -59,6 +59,7 @@
     "core-js": "^2.6.9",
     "material-components-web": "7.0.0-canary.7461aad68.0",
     "rxjs": "^6.5.3",
+    "rxjs-tslint-rules": "^4.33.1",
     "systemjs": "0.19.43",
     "tslib": "^2.0.0",
     "zone.js": "~0.10.3"

src/cdk-experimental/column-resize/column-resize.ts

@@ -91,8 +91,8 @@ export abstract class ColumnResize implements AfterViewInit, OnDestroy {
         this.notifier.triggerResize.pipe(mapTo(undefined)),
         this.notifier.resizeCompleted.pipe(mapTo(undefined))
     ).pipe(
-        takeUntil(this.destroyed),
-        take(1),
+      take(1),
+      takeUntil(this.destroyed),
     ).subscribe(() => {
       this.setResized();
     });

src/cdk-experimental/column-resize/overlay-handle.ts

@@ -11,7 +11,7 @@ import {coerceCssPixelValue} from '@angular/cdk/coercion';
 import {Directionality} from '@angular/cdk/bidi';
 import {ESCAPE} from '@angular/cdk/keycodes';
 import {CdkColumnDef} from '@angular/cdk/table';
-import {fromEvent, Subject} from 'rxjs';
+import {fromEvent, Subject, merge} from 'rxjs';
 import {
   distinctUntilChanged,
   filter,
@@ -59,21 +59,19 @@ export abstract class ResizeOverlayHandle implements AfterViewInit, OnDestroy {
 
   private _listenForMouseEvents() {
     this.ngZone.runOutsideAngular(() => {
-      const takeUntilDestroyed = takeUntil<MouseEvent>(this.destroyed);
-
       fromEvent<MouseEvent>(this.elementRef.nativeElement!, 'mouseenter').pipe(
-          takeUntilDestroyed,
           mapTo(this.resizeRef.origin.nativeElement!),
+          takeUntil(this.destroyed),
       ).subscribe(cell => this.eventDispatcher.headerCellHovered.next(cell));
 
       fromEvent<MouseEvent>(this.elementRef.nativeElement!, 'mouseleave').pipe(
-          takeUntilDestroyed,
           map(event => event.relatedTarget &&
               _closest(event.relatedTarget as Element, HEADER_CELL_SELECTOR)),
+          takeUntil(this.destroyed)
       ).subscribe(cell => this.eventDispatcher.headerCellHovered.next(cell));
 
       fromEvent<MouseEvent>(this.elementRef.nativeElement!, 'mousedown')
-          .pipe(takeUntilDestroyed).subscribe(mousedownEvent => {
+          .pipe(takeUntil(this.destroyed)).subscribe(mousedownEvent => {
         this._dragStarted(mousedownEvent);
       });
     });
@@ -100,11 +98,11 @@ export abstract class ResizeOverlayHandle implements AfterViewInit, OnDestroy {
 
     this.updateResizeActive(true);
 
-    mouseup.pipe(takeUntil(escape), takeUntil(this.destroyed)).subscribe(({screenX}) => {
+    mouseup.pipe(takeUntil(merge(escape, this.destroyed))).subscribe(({screenX}) => {
       this._notifyResizeEnded(size, screenX !== startX);
     });
 
-    escape.pipe(takeUntil(mouseup), takeUntil(this.destroyed)).subscribe(() => {
+    escape.pipe(takeUntil(merge(mouseup, this.destroyed))).subscribe(() => {
       this._notifyResizeEnded(initialSize);
     });
 
@@ -113,9 +111,7 @@ export abstract class ResizeOverlayHandle implements AfterViewInit, OnDestroy {
         startWith(startX),
         distinctUntilChanged(),
         pairwise(),
-        takeUntil(mouseup),
-        takeUntil(escape),
-        takeUntil(this.destroyed),
+        takeUntil(merge(mouseup, escape, this.destroyed))
     ).subscribe(([prevX, currX]) => {
       let deltaX = currX - prevX;
 

src/cdk-experimental/popover-edit/table-directives.ts

@@ -107,9 +107,9 @@ export class CdkEditable implements AfterViewInit, OnDestroy {
           handler => element.addEventListener('focus', handler, true),
           handler => element.removeEventListener('focus', handler, true)
           ).pipe(
-              takeUntil(this.destroyed),
               toClosest(ROW_SELECTOR),
               share(),
+              takeUntil(this.destroyed),
               ).subscribe(this.editEventDispatcher.focused);
 
       merge(
@@ -119,22 +119,22 @@ export class CdkEditable implements AfterViewInit, OnDestroy {
         ),
         fromEvent<KeyboardEvent>(element, 'keydown').pipe(filter(event => event.key === 'Escape'))
       ).pipe(
-        takeUntil(this.destroyed),
         mapTo(null),
         share(),
+        takeUntil(this.destroyed),
       ).subscribe(this.editEventDispatcher.focused);
 
       // Keep track of rows within the table. This is used to know which rows with hover content
       // are first or last in the table. They are kept focusable in case focus enters from above
       // or below the table.
       this.ngZone.onStable.pipe(
-          takeUntil(this.destroyed),
           // Optimization: ignore dom changes while focus is within the table as we already
           // ensure that rows above and below the focused/active row are tabbable.
           withLatestFrom(this.editEventDispatcher.editingOrFocused),
           filter(([_, activeRow]) => activeRow == null),
           map(() => element.querySelectorAll(ROW_SELECTOR)),
           share(),
+          takeUntil(this.destroyed),
           ).subscribe(this.editEventDispatcher.allRows);
 
       fromEvent<KeyboardEvent>(element, 'keydown').pipe(
@@ -310,10 +310,9 @@ export class CdkPopoverEdit<C> implements AfterViewInit, OnDestroy {
     // scroll position and viewport size.
     merge(this.services.scrollDispatcher.scrolled(), this.services.viewportRuler.change())
         .pipe(
-            startWith(null),
-            takeUntil(this.overlayRef!.detachments()),
-            takeUntil(this.destroyed),
-            )
+          startWith(null),
+          takeUntil(merge(this.overlayRef!.detachments(), this.destroyed))
+        )
         .subscribe(() => {
           this._updateOverlaySize();
         });

tslint.json

@@ -1,4 +1,7 @@
 {
+  "extends": [
+    "rxjs-tslint-rules"
+  ],
   "rulesDirectory": [
     "./tools/tslint-rules/",
     "node_modules/vrsource-tslint-rules/rules",
@@ -107,6 +110,10 @@
     "template-no-negated-async": true,
     "use-lifecycle-interface": true,
 
+    // RxJS
+    "rxjs-no-unsafe-takeuntil": true,
+    "rxjs-no-unsafe-catch": true,
+
     // Custom Rules
     "ts-loader": true,
     "no-exposed-todo": true,

yarn.lock

@@ -1171,6 +1171,13 @@
   dependencies:
     "@types/node" ">= 8"
 
+"@phenomnomnominal/tsquery@^4.0.0":
+  version "4.1.0"
+  resolved "https://registry.yarnpkg.com/@phenomnomnominal/tsquery/-/tsquery-4.1.0.tgz#9c836d6db829b5127ccc1ffd8e4c2ad08d600071"
+  integrity sha512-+i1eqUJODVanUDuTdOPgnjErFg21DKGLstdRXp4LLGcSbO7c+3pwJPkmdSfbkh9gO6xaHJ/5ftSAMqEFJF5cGA==
+  dependencies:
+    esquery "^1.0.1"
+
 "@protobufjs/aspromise@^1.1.1", "@protobufjs/aspromise@^1.1.2":
   version "1.1.2"
   resolved "https://registry.yarnpkg.com/@protobufjs/aspromise/-/aspromise-1.1.2.tgz#9b8b0cc663d669a7d8f6f5d0893a14d348f30fbf"
@@ -3859,6 +3866,11 @@ decamelize@^1.1.0, decamelize@^1.1.1, decamelize@^1.1.2, decamelize@^1.2.0:
   resolved "https://registry.yarnpkg.com/decamelize/-/decamelize-1.2.0.tgz#f6534d15148269b20352e7bee26f501f9a191290"
   integrity sha1-9lNNFRSCabIDUue+4m9QH5oZEpA=
 
+decamelize@^4.0.0:
+  version "4.0.0"
+  resolved "https://registry.yarnpkg.com/decamelize/-/decamelize-4.0.0.tgz#aa472d7bf660eb15f3494efd531cab7f2a709837"
+  integrity sha512-9iE1PgSik9HeIIw2JO94IidnE3eBoQrFJ3w7sFuzSX4DpmZ3v5sZpUiV5Swcf6mQEF+Y0ru8Neo+p+nyh2J+hQ==
+
 decode-uri-component@^0.2.0:
   version "0.2.0"
   resolved "https://registry.yarnpkg.com/decode-uri-component/-/decode-uri-component-0.2.0.tgz#eb3913333458775cb84cd1a1fae062106bb87545"
@@ -4537,6 +4549,13 @@ esprima@^4.0.0, esprima@~4.0.0:
   resolved "https://registry.yarnpkg.com/esprima/-/esprima-4.0.1.tgz#13b04cdb3e6c5d19df91ab6987a8695619b0aa71"
   integrity sha512-eGuFFw7Upda+g4p+QHvnW0RyTX/SVeJBDM/gCtMARO0cLuT2HcEKnTPvhjV6aGeqrCB/sbNop0Kszm0jsaWU4A==
 
+esquery@^1.0.1:
+  version "1.3.1"
+  resolved "https://registry.yarnpkg.com/esquery/-/esquery-1.3.1.tgz#b78b5828aa8e214e29fb74c4d5b752e1c033da57"
+  integrity sha512-olpvt9QG0vniUBZspVRN6lwB7hOZoTRtT+jzR+tS4ffYx2mzbw+z0XCOk44aaLYKApNX5nMm+E+P6o25ip/DHQ==
+  dependencies:
+    estraverse "^5.1.0"
+
 estraverse@^1.9.1:
   version "1.9.3"
   resolved "https://registry.yarnpkg.com/estraverse/-/estraverse-1.9.3.tgz#af67f2dc922582415950926091a4005d29c9bb44"
@@ -4547,6 +4566,11 @@ estraverse@^4.1.0, estraverse@^4.2.0:
   resolved "https://registry.yarnpkg.com/estraverse/-/estraverse-4.2.0.tgz#0dee3fed31fcd469618ce7342099fc1afa0bdb13"
   integrity sha1-De4/7TH81GlhjOc0IJn8GvoL2xM=
 
+estraverse@^5.1.0:
+  version "5.1.0"
+  resolved "https://registry.yarnpkg.com/estraverse/-/estraverse-5.1.0.tgz#374309d39fd935ae500e7b92e8a6b4c720e59642"
+  integrity sha512-FyohXK+R0vE+y1nHLoBM7ZTyqRpqAlhdZHCWIWEviFLiGB8b04H6bQs8G+XTthacvT8VuwvteiP7RJSxMs8UEw==
+
 estree-walker@^0.6.1:
   version "0.6.1"
   resolved "https://registry.yarnpkg.com/estree-walker/-/estree-walker-0.6.1.tgz#53049143f40c6eb918b23671d1fe3219f3a1b362"
@@ -10318,6 +10342,19 @@ [email protected]:
   resolved "https://registry.yarnpkg.com/rx/-/rx-4.1.0.tgz#a5f13ff79ef3b740fe30aa803fb09f98805d4782"
   integrity sha1-pfE/957zt0D+MKqAP7CfmIBdR4I=
 
+rxjs-tslint-rules@^4.33.1:
+  version "4.33.1"
+  resolved "https://registry.yarnpkg.com/rxjs-tslint-rules/-/rxjs-tslint-rules-4.33.1.tgz#c94f9b4ccec4fcbb6697033899dfe1298056aebf"
+  integrity sha512-sFhSDmVzqf653/qbAnhDBZ2xMFpr4eek3e5jFNpfweCvRD5/Vb/vf1/c8LMsuj0wvA7ZU2Vl2E53nrAvxIdOig==
+  dependencies:
+    "@phenomnomnominal/tsquery" "^4.0.0"
+    decamelize "^4.0.0"
+    resolve "^1.4.0"
+    semver "^7.0.0"
+    tslib "^2.0.0"
+    tsutils "^3.0.0"
+    tsutils-etc "^1.2.2"
+
 [email protected]:
   version "6.5.5"
   resolved "https://registry.yarnpkg.com/rxjs/-/rxjs-6.5.5.tgz#c5c884e3094c8cfee31bf27eb87e54ccfc87f9ec"
@@ -10529,6 +10566,11 @@ semver@^6.3.0:
   resolved "https://registry.yarnpkg.com/semver/-/semver-6.3.0.tgz#ee0a64c8af5e8ceea67687b133761e1becbd1d3d"
   integrity sha512-b39TBaTSfV6yBrapU89p5fKekE2m/NwnDocOVruQFS1/veMgdzuPcnOM34M6CwxW8jH/lxEa5rBoDeUwu5HHTw==
 
+semver@^7.0.0:
+  version "7.3.2"
+  resolved "https://registry.yarnpkg.com/semver/-/semver-7.3.2.tgz#604962b052b81ed0786aae84389ffba70ffd3938"
+  integrity sha512-OrOb32TeeambH6UrhtShmF7CRDqhL6/5XpPNp2DuRH6+9QLw/orhp72j87v8Qa1ScDkvrrBNpZcDejAirJmfXQ==
+
 semver@~5.0.1:
   version "5.0.3"
   resolved "https://registry.yarnpkg.com/semver/-/semver-5.0.3.tgz#77466de589cd5d3c95f138aa78bc569a3cb5d27a"
@@ -11880,6 +11922,11 @@ tslint@~5.1.0:
     semver "^5.3.0"
     tsutils "^1.4.0"
 
+tsutils-etc@^1.2.2:
+  version "1.2.2"
+  resolved "https://registry.yarnpkg.com/tsutils-etc/-/tsutils-etc-1.2.2.tgz#cdeeb777574a5c1b15b27658cb8424f7f7139831"
+  integrity sha512-5g2cXpD1OoVc/MLZxh5PuHXhlnYQmuRiW66e1n91j+2J/Pw5lfmVcZAghoDVBdltDXGaCjy8ZttXaX2u/MjHgg==
+
 [email protected]:
   version "2.27.2"
   resolved "https://registry.yarnpkg.com/tsutils/-/tsutils-2.27.2.tgz#60ba88a23d6f785ec4b89c6e8179cac9b431f1c7"
@@ -11899,7 +11946,7 @@ tsutils@^2.29.0:
   dependencies:
     tslib "^1.8.1"
 
-tsutils@^3.17.1:
+tsutils@^3.0.0, tsutils@^3.17.1:
   version "3.17.1"
   resolved "https://registry.yarnpkg.com/tsutils/-/tsutils-3.17.1.tgz#ed719917f11ca0dee586272b2ac49e015a2dd759"
   integrity sha512-kzeQ5B8H3w60nFY2g8cJIuH7JDpsALXySGtwGJ0p2LSjLgay3NdIpqq5SoOBe46bKDW2iq25irHCr8wjomUS2g==