From 5a5f6289b0e124408b71e89c2b90e8167602577b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=8D=A1=E8=89=B2?= <cipchk@qq.com>
Date: Fri, 26 Oct 2018 23:25:53 +0800
Subject: [PATCH] perf(module:upload): optimize invalid parameter warning
 (#2363)

close #2322
---
 components/upload/nz-upload-btn.component.ts | 37 +++++++++++---------
 components/upload/upload.spec.ts             | 22 ++++++++++--
 2 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/components/upload/nz-upload-btn.component.ts b/components/upload/nz-upload-btn.component.ts
index 00916a4ac1..8fe80df857 100644
--- a/components/upload/nz-upload-btn.component.ts
+++ b/components/upload/nz-upload-btn.component.ts
@@ -9,8 +9,6 @@ import {
   OnDestroy,
   OnInit,
   Optional,
-  SimpleChange,
-  SimpleChanges,
   ViewChild
 } from '@angular/core';
 import { Observable, Subscription } from 'rxjs';
@@ -179,10 +177,10 @@ export class NzUploadBtnComponent implements OnInit, OnChanges, OnDestroy {
     const { uid } = file;
     let { data, headers } = opt;
     if (typeof data === 'function') {
-      data = data(file);
+      data = (data as (file: UploadFile) => {})(file);
     }
     if (typeof headers === 'function') {
-      headers = headers(file);
+      headers = (headers as (file: UploadFile) => {})(file);
     }
     const args: UploadXHRArgs = {
       action         : opt.action,
@@ -195,15 +193,19 @@ export class NzUploadBtnComponent implements OnInit, OnChanges, OnDestroy {
         opt.onProgress(e, file);
       } : null,
       onSuccess      : (ret, xhr) => {
-        delete this.reqs[ uid ];
+        this.clean(uid);
         opt.onSuccess(ret, file, xhr);
       },
       onError        : (xhr) => {
-        delete this.reqs[ uid ];
+        this.clean(uid);
         opt.onError(xhr, file);
       }
     };
-    this.reqs[ uid ] = (opt.customRequest || this.xhr).call(this, args);
+    const req$ = (opt.customRequest || this.xhr).call(this, args);
+    if (!(req$ instanceof Subscription)) {
+      console.warn(`Must return Subscription type in '[nzCustomRequest]' property`);
+    }
+    this.reqs[ uid ] = req$;
     opt.onStart(file);
   }
 
@@ -245,18 +247,19 @@ export class NzUploadBtnComponent implements OnInit, OnChanges, OnDestroy {
     });
   }
 
+  private clean(uid: string): void {
+    const req$ = this.reqs[ uid ];
+    if (req$ instanceof Subscription) {
+      req$.unsubscribe();
+    }
+    delete this.reqs[ uid ];
+  }
+
   abort(file?: UploadFile): void {
     if (file) {
-      const uid = file && file.uid;
-      if (this.reqs[ uid ]) {
-        this.reqs[ uid ].unsubscribe();
-        delete this.reqs[ uid ];
-      }
+      this.clean(file && file.uid);
     } else {
-      Object.keys(this.reqs).forEach((uid) => {
-        this.reqs[ uid ].unsubscribe();
-        delete this.reqs[ uid ];
-      });
+      Object.keys(this.reqs).forEach((uid) => this.clean(uid));
     }
   }
 
@@ -285,7 +288,7 @@ export class NzUploadBtnComponent implements OnInit, OnChanges, OnDestroy {
     this.setClassMap();
   }
 
-  ngOnChanges(changes: { [P in keyof this]?: SimpleChange } & SimpleChanges): void {
+  ngOnChanges(): void {
     if (this.inited) {
       this.setClassMap();
     }
diff --git a/components/upload/upload.spec.ts b/components/upload/upload.spec.ts
index fee0740ddc..c781271b4d 100644
--- a/components/upload/upload.spec.ts
+++ b/components/upload/upload.spec.ts
@@ -15,7 +15,7 @@ import { NzIconModule } from '../icon/nz-icon.module';
 import { NzProgressModule } from '../progress/nz-progress.module';
 import { NzToolTipModule } from '../tooltip/nz-tooltip.module';
 
-import { ShowUploadListInterface, UploadChangeParam, UploadFile, UploadFilter, UploadListType, UploadType, ZipButtonOptions } from './interface';
+import { ShowUploadListInterface, UploadChangeParam, UploadFile, UploadFilter, UploadListType, UploadType, UploadXHRArgs, ZipButtonOptions } from './interface';
 import { NzUploadBtnComponent } from './nz-upload-btn.component';
 import { NzUploadListComponent } from './nz-upload-list.component';
 import { NzUploadComponent } from './nz-upload.component';
@@ -503,6 +503,15 @@ describe('upload', () => {
       });
     });
 
+    describe('[test boundary]', () => {
+      it('clean a not exists request', () => {
+        instance.comp.upload.reqs.test = null;
+        instance.show = false;
+        fixture.detectChanges();
+        expect(true).toBe(true);
+      });
+    });
+
     class NzUploadPageObject {
       private files: any;
       constructor() {
@@ -966,13 +975,21 @@ describe('upload', () => {
         comp.onChange(PNGSMALL as any);
         expect(comp.options.customRequest).toHaveBeenCalled();
       });
+
+      it('should be warn "Must return Subscription type in [nzCustomRequest] property"', () => {
+        let warnMsg = '';
+        console.warn = jasmine.createSpy().and.callFake(res => warnMsg = res);
+        comp.options.customRequest = ((item: UploadXHRArgs) => { }) as any;
+        comp.onChange(PNGSMALL as any);
+        expect(warnMsg).toContain(`Must return Subscription type`);
+      });
     });
   });
 });
 
 @Component({
   template: `
-  <nz-upload #upload
+  <nz-upload #upload *ngIf="show"
     [nzType]="nzType"
     [nzLimit]="nzLimit"
     [nzSize]="nzSize"
@@ -1005,6 +1022,7 @@ describe('upload', () => {
 })
 class TestUploadComponent {
   @ViewChild('upload') comp: NzUploadComponent;
+  show = true;
   nzType: UploadType = 'select';
   nzLimit = 0;
   nzSize = 0;