From a27e6cc024b44df9d4b25b3d5df814be73645602 Mon Sep 17 00:00:00 2001 From: "CALABRO Raphael (UA 2118)" Date: Wed, 23 May 2018 11:47:43 +0200 Subject: [PATCH] Upgraded to rxjs6 syntax and removed lodash Fixed "compare" method for null and undefined values Using const and let when possible Using triple equals to avoid comparison errors Fixed unit test by casting "bulb" to allow assignation to SortOrder type Removed lodash dependency and updated dependencies versions. Updated systemjs and karma configurations Fixed code style. Upgraded version number to 2.0.0 Using SortOrder as argument to better convey the possible values Added unit tests for new code and fixed onSortChange emissions Updated NodeJS version for Travis --- .travis.yml | 2 +- examples/systemjs/app/data-filter.pipe.ts | 3 +- karma.conf.js | 7 -- package.json | 40 +++++------ src/BootstrapPaginator.ts | 3 +- src/DataTable.spec.ts | 86 +++++++++++++++-------- src/DataTable.ts | 79 +++++++++++++-------- systemjs.config.js | 9 ++- 8 files changed, 132 insertions(+), 97 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1afc66f..a9ef856 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,6 @@ language: node_js node_js: -- '5' +- '8' before_install: - npm install -g codeclimate-test-reporter script: npm test -- --browsers PhantomJS diff --git a/examples/systemjs/app/data-filter.pipe.ts b/examples/systemjs/app/data-filter.pipe.ts index b165732..0469335 100644 --- a/examples/systemjs/app/data-filter.pipe.ts +++ b/examples/systemjs/app/data-filter.pipe.ts @@ -1,4 +1,3 @@ -import * as _ from "lodash"; import {Pipe, PipeTransform} from "@angular/core"; @Pipe({ @@ -8,7 +7,7 @@ export class DataFilterPipe implements PipeTransform { transform(array: any[], query: string): any { if (query) { - return _.filter(array, row=>row.name.indexOf(query) > -1); + return array.filter(row=>row.name.indexOf(query) > -1); } return array; } diff --git a/karma.conf.js b/karma.conf.js index 4ad00c8..ccd7f22 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -68,13 +68,6 @@ module.exports = function (config) { watched: false }, - // Lodash - { - pattern: 'node_modules/lodash/lodash.js', - included: false, - watched: false - }, - // The testing library { pattern: 'systemjs.config.js', diff --git a/package.json b/package.json index fe69d9f..a865ebc 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "angular2-datatable", - "version": "0.6.0", + "version": "2.0.0", "description": "DataTable component for Angular2 framework", "main": "index.js", "scripts": { @@ -19,8 +19,7 @@ }, "keywords": [ "angular", - "angularjs", - "angular2", + "angular6", "ng", "ng2", "table", @@ -35,12 +34,13 @@ }, "homepage": "https://github.com/mariuszfoltak/angular2-datatable#readme", "devDependencies": { - "@angular/core": "^2.1.0", - "@angular/compiler": "^2.1.0", - "@angular/common": "^2.1.0", - "@angular/platform-browser": "^2.1.0", - "@angular/platform-browser-dynamic": "^2.1.0", - "@angular/compiler-cli": "^2.1.0", + "@angular/common": "^6.0.0", + "@angular/compiler": "^6.0.0", + "@angular/compiler-cli": "^6.0.0", + "@angular/core": "^6.0.0", + "@angular/platform-browser": "^6.0.0", + "@angular/platform-browser-dynamic": "^6.0.0", + "@types/jasmine": "^2.5.35", "core-js": "^2.4.1", "http-server": "^0.9.0", "jasmine-core": "^2.4.1", @@ -49,24 +49,18 @@ "karma-coverage": "^1.1.1", "karma-jasmine": "^1.0.2", "karma-phantomjs-launcher": "^1.0.2", - "lodash": "^4.0.0", "phantomjs-prebuilt": "^2.1.7", "remap-istanbul": "^0.7.0", - "rxjs": "5.0.0-beta.12", - "systemjs": "^0.19.39", - "typescript": "~2.1.0", - "zone.js": "^0.6.25", "rimraf": "^2.5.4", - "@types/lodash": "ts2.0", - "@types/jasmine": "^2.5.35" - }, - "dependencies": { - "lodash": "^4.0.0" + "rxjs": "^6.0.0", + "systemjs": "^0.19.47", + "typescript": "~2.7.2", + "zone.js": "^0.6.25" }, "peerDependencies": { - "@angular/core": "^2.0.0", - "@angular/common": "^2.0.0", - "@angular/platform-browser": "^2.0.0", - "rxjs": "^5.0.0-beta.12" + "@angular/core": "^6.0.0", + "@angular/common": "^6.0.0", + "@angular/platform-browser": "^6.0.0", + "rxjs": "^6.0.0" } } diff --git a/src/BootstrapPaginator.ts b/src/BootstrapPaginator.ts index 435ce0e..dccd42a 100644 --- a/src/BootstrapPaginator.ts +++ b/src/BootstrapPaginator.ts @@ -1,6 +1,5 @@ import {Component, Input, OnChanges} from "@angular/core"; import {DataTable} from "./DataTable"; -import * as _ from "lodash"; @Component({ selector: "mfBootstrapPaginator", @@ -57,7 +56,7 @@ export class BootstrapPaginator implements OnChanges { ngOnChanges(changes: any): any { if (changes.rowsOnPageSet) { - this.minRowsOnPage = _.min(this.rowsOnPageSet) + this.minRowsOnPage = this.rowsOnPageSet.reduce((previous, current) => current < previous ? current : previous); } } } \ No newline at end of file diff --git a/src/DataTable.spec.ts b/src/DataTable.spec.ts index b58a52d..3b00332 100644 --- a/src/DataTable.spec.ts +++ b/src/DataTable.spec.ts @@ -3,7 +3,8 @@ import {SimpleChange, Component} from "@angular/core"; import {DataTable, PageEvent, SortEvent} from "./DataTable"; import {TestBed, ComponentFixture} from "@angular/core/testing"; import {By} from "@angular/platform-browser"; -import * as _ from "lodash"; +import {switchMap} from 'rxjs/operators'; +import {range} from "rxjs"; @Component({ template: `
` @@ -29,13 +30,13 @@ describe("DataTable directive tests", ()=> { {id: 5, name: 'Ðrone'}, {id: 4, name: 'Ananas'} ]; - datatable.ngOnChanges({inputData: new SimpleChange(null, datatable.inputData)}); + datatable.ngOnChanges({inputData: new SimpleChange(null, datatable.inputData, false)}); }); describe("initializing", ()=> { it("data should be empty array if inputData is undefined or null", () => { - datatable.ngOnChanges({inputData: new SimpleChange(null, null)}); + datatable.ngOnChanges({inputData: new SimpleChange(null, null, false)}); datatable.ngDoCheck(); expect(datatable.data).toEqual([]); }); @@ -108,12 +109,19 @@ describe("DataTable directive tests", ()=> { }); datatable.rowsOnPage = 3; - datatable.ngOnChanges({rowsOnPage: new SimpleChange(2, 3)}); + datatable.ngOnChanges({rowsOnPage: new SimpleChange(2, 3, false)}); datatable.ngDoCheck(); expect(datatable.data).toEqual([{id: 3, name: 'banana'}, {id: 1, name: 'Duck'}, {id: 2, name: 'ącki'}]); - - }); + + it("should emit a dataLength of 0 when inputData is null or undefined", (done) => { + datatable.onPageChange.subscribe((pageOptions: PageEvent)=> { + expect(pageOptions.dataLength).toEqual(0); + done(); + }); + datatable.inputData = null; + datatable.setPage(2, 3); + }) }); describe("sorting", ()=> { @@ -128,8 +136,8 @@ describe("DataTable directive tests", ()=> { datatable.sortBy = "id"; datatable.sortOrder = "asc"; datatable.ngOnChanges({ - sortBy: new SimpleChange(null, datatable.sortBy), - sortOrder: new SimpleChange(null, datatable.sortOrder) + sortBy: new SimpleChange(null, datatable.sortBy, false), + sortOrder: new SimpleChange(null, datatable.sortOrder, false) }); datatable.ngDoCheck(); expect(datatable.data).toEqual([ @@ -151,8 +159,8 @@ describe("DataTable directive tests", ()=> { datatable.sortBy = "id"; datatable.sortOrder = "desc"; datatable.ngOnChanges({ - sortBy: new SimpleChange(null, datatable.sortBy), - sortOrder: new SimpleChange(null, datatable.sortOrder) + sortBy: new SimpleChange(null, datatable.sortBy, false), + sortOrder: new SimpleChange(null, datatable.sortOrder, false) }); datatable.ngDoCheck(); @@ -167,7 +175,7 @@ describe("DataTable directive tests", ()=> { datatable.ngDoCheck(); datatable.sortBy = "id"; datatable.ngOnChanges({ - sortBy: new SimpleChange(null, datatable.sortBy) + sortBy: new SimpleChange(null, datatable.sortBy, false) }); datatable.ngDoCheck(); expect(datatable.sortOrder).toEqual("asc"); @@ -181,10 +189,10 @@ describe("DataTable directive tests", ()=> { }); datatable.ngDoCheck(); datatable.sortBy = "id"; - datatable.sortOrder = "bulb"; + datatable.sortOrder = "bulb" as any; datatable.ngOnChanges({ - sortBy: new SimpleChange(null, datatable.sortBy), - sortOrder: new SimpleChange(null, datatable.sortOrder) + sortBy: new SimpleChange(null, datatable.sortBy, false), + sortOrder: new SimpleChange(null, datatable.sortOrder, false) }); datatable.ngDoCheck(); expect(datatable.sortOrder).toEqual("asc"); @@ -197,6 +205,20 @@ describe("DataTable directive tests", ()=> { ]); }); + it("should set sortOrder to 'asc' if setSort is given something else than 'asc' or 'desc'", () => { + datatable.setSort("id", "bulb" as any); + expect(datatable.getSort()).toEqual({sortBy: "id", sortOrder: "asc"}); + datatable.ngDoCheck(); + expect(datatable.sortOrder).toEqual("asc"); + expect(datatable.data).toEqual([ + {id: 1, name: 'Duck'}, + {id: 2, name: 'ącki'}, + {id: 3, name: 'banana'}, + {id: 4, name: 'Ananas'}, + {id: 5, name: 'Ðrone'} + ]); + }); + it("shouldn't change order when only order provided", (done)=> { done(); datatable.onSortChange.subscribe(()=> { @@ -204,17 +226,19 @@ describe("DataTable directive tests", ()=> { }); datatable.ngDoCheck(); datatable.sortOrder = "desc"; - datatable.ngOnChanges({sortOrder: new SimpleChange(null, datatable.sortOrder)}); + datatable.ngOnChanges({sortOrder: new SimpleChange(null, datatable.sortOrder, false)}); datatable.ngDoCheck(); expect(datatable.data).toEqual(datatable.inputData); }); it("should call output event when sorting changed", (done)=> { datatable.ngDoCheck(); - datatable.sortByChange.switchMap((sortBy: string)=> { - expect(sortBy).toEqual("id"); - return datatable.sortOrderChange; - }).subscribe((sortOrder: string)=> { + datatable.sortByChange.pipe( + switchMap((sortBy: string)=> { + expect(sortBy).toEqual("id"); + return datatable.sortOrderChange; + }) + ).subscribe((sortOrder: string)=> { expect(sortOrder).toEqual("desc"); done(); }); @@ -228,8 +252,8 @@ describe("DataTable directive tests", ()=> { done.fail("Shouldn't call sortOrderChange"); }); done(); - datatable.sortOrder = "bulb"; - datatable.ngOnChanges({sortOrder: new SimpleChange(null, datatable.sortOrder)}); + datatable.sortOrder = "bulb" as any; + datatable.ngOnChanges({sortOrder: new SimpleChange(null, datatable.sortOrder, false)}); datatable.ngDoCheck(); }); // Wywołanie outputa gdy zmiana z innej strony @@ -272,14 +296,16 @@ describe("DataTable directive tests", ()=> { {name: 'Claire', age: 9}, {name: 'Anna', age: 34}, {name: 'Claire', age: 16}, + {name: 'Anna', age: 12}, {name: 'Claire', age: 7}, {name: 'Anna', age: 12} ]; - datatable.ngOnChanges({inputData: new SimpleChange(datatable.inputData, newData)}); + datatable.ngOnChanges({inputData: new SimpleChange(datatable.inputData, newData, false)}); datatable.setSort(['name', 'age'], "asc"); datatable.ngDoCheck(); expect(datatable.data).toEqual([ + {name: 'Anna', age: 12}, {name: 'Anna', age: 12}, {name: 'Anna', age: 34}, {name: 'Claire', age: 7}, @@ -297,7 +323,7 @@ describe("DataTable directive tests", ()=> { {name: 'Claire', city: {zip: '11111'}}, {name: 'Anna', city: {zip: '21111'}} ]; - datatable.ngOnChanges({inputData: new SimpleChange(datatable.inputData, newData)}); + datatable.ngOnChanges({inputData: new SimpleChange(datatable.inputData, newData, false)}); datatable.setSort("city.zip", "asc"); datatable.ngDoCheck(); @@ -315,7 +341,7 @@ describe("DataTable directive tests", ()=> { describe("data change", ()=> { it("should refresh data when inputData change", ()=> { let newData = [{id: 5, name: 'Ðrone'}, {id: 4, name: 'Ananas'}]; - datatable.ngOnChanges({inputData: new SimpleChange(datatable.inputData, newData)}); + datatable.ngOnChanges({inputData: new SimpleChange(datatable.inputData, newData, false)}); datatable.ngDoCheck(); expect(datatable.data).toEqual([{id: 5, name: 'Ðrone'}, {id: 4, name: 'Ananas'}]); }); @@ -347,7 +373,7 @@ describe("DataTable directive tests", ()=> { done(); }); let newData = [{id: 5, name: 'Ðrone'}, {id: 4, name: 'Ananas'}]; - datatable.ngOnChanges({inputData: new SimpleChange(datatable.inputData, newData)}); + datatable.ngOnChanges({inputData: new SimpleChange(datatable.inputData, newData, false)}); datatable.ngDoCheck(); }); @@ -375,7 +401,7 @@ describe("DataTable directive tests", ()=> { expect(opt.rowsOnPage).toEqual(2); done(); }); - _.times(3, ()=>datatable.inputData.pop()); + range(0, 3).forEach(()=>datatable.inputData.pop()); datatable.ngDoCheck(); }); @@ -384,7 +410,7 @@ describe("DataTable directive tests", ()=> { datatable.ngDoCheck(); let newData = [{id: 5, name: 'Ðrone'}, {id: 4, name: 'Ananas'}]; - datatable.ngOnChanges({inputData: new SimpleChange(datatable.inputData, newData)}); + datatable.ngOnChanges({inputData: new SimpleChange(datatable.inputData, newData, false)}); datatable.ngDoCheck(); expect(datatable.data).toEqual(newData); }); @@ -406,7 +432,7 @@ describe("DataTable directive tests", ()=> { datatable.ngDoCheck(); let newData = [{id: 5, name: 'Ðrone'}, {id: 1, name: 'Duck'}, {id: 4, name: 'Ananas'}]; - datatable.ngOnChanges({inputData: new SimpleChange(datatable.inputData, newData)}); + datatable.ngOnChanges({inputData: new SimpleChange(datatable.inputData, newData, false)}); datatable.ngDoCheck(); expect(datatable.data).toEqual([{id: 1, name: 'Duck'}]); }); @@ -436,7 +462,7 @@ describe("DataTable directive tests", ()=> { datatable.ngDoCheck(); let newData = []; - datatable.ngOnChanges({inputData: new SimpleChange(datatable.inputData, newData)}); + datatable.ngOnChanges({inputData: new SimpleChange(datatable.inputData, newData, false)}); datatable.ngDoCheck(); expect(datatable.activePage).toEqual(1); }); @@ -445,7 +471,7 @@ describe("DataTable directive tests", ()=> { datatable.setPage(2, 1); datatable.ngDoCheck(); - _.times(5, ()=>datatable.inputData.pop()); + range(0, 5).forEach(()=>datatable.inputData.pop()); datatable.ngDoCheck(); expect(datatable.inputData.length).toEqual(0); expect(datatable.activePage).toEqual(1); diff --git a/src/DataTable.ts b/src/DataTable.ts index 1232241..3e8efc6 100644 --- a/src/DataTable.ts +++ b/src/DataTable.ts @@ -2,12 +2,11 @@ import { Directive, Input, EventEmitter, SimpleChange, OnChanges, DoCheck, IterableDiffers, IterableDiffer, Output } from "@angular/core"; -import * as _ from "lodash"; -import {ReplaySubject} from "rxjs/Rx"; +import { ReplaySubject } from "rxjs"; export interface SortEvent { sortBy: string|string[]; - sortOrder: string + sortOrder: SortOrder } export interface PageEvent { @@ -20,17 +19,19 @@ export interface DataEvent { length: number; } +export type SortOrder = "asc" | "desc"; + @Directive({ - selector: 'table[mfData]', - exportAs: 'mfDataTable' + selector: "table[mfData]", + exportAs: "mfDataTable" }) export class DataTable implements OnChanges, DoCheck { - private diff: IterableDiffer; + private diff: IterableDiffer; @Input("mfData") public inputData: any[] = []; @Input("mfSortBy") public sortBy: string|string[] = ""; - @Input("mfSortOrder") public sortOrder = "asc"; + @Input("mfSortOrder") public sortOrder: SortOrder = "asc"; @Output("mfSortByChange") public sortByChange = new EventEmitter(); @Output("mfSortOrderChange") public sortOrderChange = new EventEmitter(); @@ -52,12 +53,12 @@ export class DataTable implements OnChanges, DoCheck { return {sortBy: this.sortBy, sortOrder: this.sortOrder}; } - public setSort(sortBy: string|string[], sortOrder: string): void { + public setSort(sortBy: string|string[], sortOrder: SortOrder): void { if (this.sortBy !== sortBy || this.sortOrder !== sortOrder) { this.sortBy = sortBy; - this.sortOrder = _.includes(["asc","desc"], sortOrder) ? sortOrder : "asc"; + this.sortOrder = ["asc","desc"].indexOf(sortOrder) >= 0 ? sortOrder : "asc"; this.mustRecalculateData = true; - this.onSortChange.next({sortBy: sortBy, sortOrder: sortOrder}); + this.onSortChange.next({sortBy: this.sortBy, sortOrder: this.sortOrder}); this.sortByChange.emit(this.sortBy); this.sortOrderChange.emit(this.sortOrder); } @@ -81,13 +82,13 @@ export class DataTable implements OnChanges, DoCheck { } private calculateNewActivePage(previousRowsOnPage: number, currentRowsOnPage: number): number { - let firstRowOnPage = (this.activePage - 1) * previousRowsOnPage + 1; - let newActivePage = Math.ceil(firstRowOnPage / currentRowsOnPage); + const firstRowOnPage = (this.activePage - 1) * previousRowsOnPage + 1; + const newActivePage = Math.ceil(firstRowOnPage / currentRowsOnPage); return newActivePage; } private recalculatePage() { - let lastPage = Math.ceil(this.inputData.length / this.rowsOnPage); + const lastPage = Math.ceil(this.inputData.length / this.rowsOnPage); this.activePage = lastPage < this.activePage ? lastPage : this.activePage; this.activePage = this.activePage || 1; @@ -105,7 +106,7 @@ export class DataTable implements OnChanges, DoCheck { this.mustRecalculateData = true; } if (changes["sortBy"] || changes["sortOrder"]) { - if (!_.includes(["asc", "desc"], this.sortOrder)) { + if (["asc", "desc"].indexOf(this.sortOrder) < 0) { console.warn("angular2-datatable: value for input mfSortOrder must be one of ['asc', 'desc'], but is:", this.sortOrder); this.sortOrder = "asc"; } @@ -122,7 +123,7 @@ export class DataTable implements OnChanges, DoCheck { } public ngDoCheck(): any { - let changes = this.diff.diff(this.inputData); + const changes = this.diff.diff(this.inputData); if (changes) { this.recalculatePage(); this.mustRecalculateData = true; @@ -134,33 +135,51 @@ export class DataTable implements OnChanges, DoCheck { } private fillData(): void { - this.activePage = this.activePage; - this.rowsOnPage = this.rowsOnPage; - - let offset = (this.activePage - 1) * this.rowsOnPage; + const offset = (this.activePage - 1) * this.rowsOnPage; let data = this.inputData; - var sortBy = this.sortBy; - if (typeof sortBy === 'string' || sortBy instanceof String) { - data = _.orderBy(data, this.caseInsensitiveIteratee(sortBy), [this.sortOrder]); - } else { - data = _.orderBy(data, sortBy, [this.sortOrder]); - } - data = _.slice(data, offset, offset + this.rowsOnPage); + data = [...data].sort(this.sorter(this.sortBy, this.sortOrder)); + data = data.slice(offset, offset + this.rowsOnPage); this.data = data; } private caseInsensitiveIteratee(sortBy: string) { return (row: any): any => { - var value = row; - for (let sortByProperty of sortBy.split('.')) { - if(value) { + let value = row; + for (const sortByProperty of sortBy.split(".")) { + if (value) { value = value[sortByProperty]; } } - if (value && typeof value === 'string' || value instanceof String) { + if (value && typeof value === "string" || value instanceof String) { return value.toLowerCase(); } return value; }; } + + private compare(left: any, right: any): number { + return left === right ? 0 : left == null || left > right ? 1 : -1; + } + + private sorter(sortBy: string | string[], sortOrder: string): (left: T, right: T) => number { + const order = sortOrder === "desc" ? -1 : 1; + if (typeof sortBy === "string" || sortBy instanceof String) { + const iteratee = this.caseInsensitiveIteratee(sortBy as string); + return (left, right) => { + return this.compare(iteratee(left), iteratee(right)) * order; + }; + } else { + const iteratees = sortBy.map(entry => this.caseInsensitiveIteratee(entry)); + return (left, right) => { + for (const iteratee of iteratees) { + const comparison = this.compare(iteratee(left), iteratee(right)) * order; + if (comparison !== 0) { + return comparison; + } + } + return 0; + }; + } + } + } \ No newline at end of file diff --git a/systemjs.config.js b/systemjs.config.js index a9338a2..be9a292 100644 --- a/systemjs.config.js +++ b/systemjs.config.js @@ -22,7 +22,7 @@ // other libraries 'rxjs': 'npm:rxjs', - 'lodash': 'npm:lodash/lodash.js' + 'rxjs/operators': 'npm:rxjs/operators', }, // packages tells the System loader how to load when no filename and/or no extension packages: { @@ -30,7 +30,12 @@ defaultExtension: 'js' }, rxjs: { - defaultExtension: 'js' + defaultExtension: 'js', + main: 'index.js' + }, + 'rxjs/operators': { + defaultExtension: 'js', + main: 'index.js' } } });