From f6dc75c9decb10bb6b2036fb279d7813707494db Mon Sep 17 00:00:00 2001 From: Ghislain B Date: Fri, 22 Dec 2023 16:01:29 -0500 Subject: [PATCH] chore(utils): remove unnecessary `objectAssignAndExtend` method (#1280) * chore(utils): remove unnecessary `objectAssignAndExtend` method - we can use `node-extend` which does the job and already has plenty of unit tests --- .../src/slickRowDetailView.spec.ts | 3 +- .../src/slickRowDetailView.ts | 12 +-- packages/utils/src/__tests__/utils.spec.ts | 90 ------------------- packages/utils/src/nodeExtend.ts | 2 +- packages/utils/src/utils.ts | 51 +++-------- 5 files changed, 19 insertions(+), 139 deletions(-) diff --git a/packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts b/packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts index 7d60ea34e..11af9230c 100644 --- a/packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts +++ b/packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts @@ -86,10 +86,11 @@ describe('SlickRowDetailView plugin', () => { it('should be able to change plugin options and "collapseAll" be called when "singleRowExpand" is enabled', () => { const collapseAllSpy = jest.spyOn(plugin, 'collapseAll'); const mockOptions = { - columnId: 'selector', + columnId: '_detail_selector', cssClass: 'some-detailView-toggle', expandedClass: 'some-class', collapsedClass: 'some-collapsed-class', + field: '_detail_selector', keyPrefix: '::', loadOnce: true, collapseAllOnSort: true, diff --git a/packages/row-detail-view-plugin/src/slickRowDetailView.ts b/packages/row-detail-view-plugin/src/slickRowDetailView.ts index 91afb2f91..96693ca47 100644 --- a/packages/row-detail-view-plugin/src/slickRowDetailView.ts +++ b/packages/row-detail-view-plugin/src/slickRowDetailView.ts @@ -20,7 +20,7 @@ import type { UsabilityOverrideFn, } from '@slickgrid-universal/common'; import { createDomElement, SlickEvent, SlickEventHandler, } from '@slickgrid-universal/common'; -import { objectAssignAndExtend } from '@slickgrid-universal/utils'; +import { extend } from '@slickgrid-universal/utils'; /** * A plugin to add Row Detail Panel View (for example providing order detail info when clicking on the order row in the grid) @@ -140,7 +140,7 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV this._grid = grid; this._gridUid = grid.getUID(); if (!this._addonOptions) { - this._addonOptions = objectAssignAndExtend(this.gridOptions.rowDetailView, this._defaults) as RowDetailView; + this._addonOptions = extend(true, {}, this._defaults, this.gridOptions.rowDetailView) as RowDetailView; } this._keyPrefix = this._addonOptions?.keyPrefix || '_'; @@ -200,7 +200,7 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV throw new Error('[Slickgrid-Universal] The Row Detail View requires options to be passed via the "rowDetailView" property of the Grid Options'); } - this._addonOptions = objectAssignAndExtend(gridOptions.rowDetailView, this._defaults) as RowDetailView; + this._addonOptions = extend(true, {}, this._defaults, gridOptions.rowDetailView) as RowDetailView; // user could override the expandable icon logic from within the options or after instantiating the plugin if (typeof this._addonOptions.expandableOverride === 'function') { @@ -239,7 +239,7 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV /** set or change some of the plugin options */ setOptions(options: Partial) { - this._addonOptions = objectAssignAndExtend(options, this._addonOptions); + this._addonOptions = extend(true, {}, this._addonOptions, options) as RowDetailView; if (this._addonOptions?.singleRowExpand) { this.collapseAll(); } @@ -248,9 +248,9 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV /** Collapse all of the open items */ collapseAll() { this.dataView.beginUpdate(); - for (const expandedRow of this._expandedRows) { + this._expandedRows.forEach(expandedRow => { this.collapseDetailView(expandedRow, true); - } + }); this.dataView.endUpdate(); } diff --git a/packages/utils/src/__tests__/utils.spec.ts b/packages/utils/src/__tests__/utils.spec.ts index acd4759d3..1ceaf627e 100644 --- a/packages/utils/src/__tests__/utils.spec.ts +++ b/packages/utils/src/__tests__/utils.spec.ts @@ -6,7 +6,6 @@ import { arrayRemoveItemByIndex, deepCopy, deepMerge, - objectAssignAndExtend, emptyObject, hasData, isDefined, @@ -418,95 +417,6 @@ describe('Service/Utilies', () => { }); }); - describe('objectAssignAndExtend method', () => { - it('should return undefined when both inputs are undefined', () => { - const obj1 = undefined; - const obj2 = null; - const output = objectAssignAndExtend(obj1, obj2); - expect(output).toEqual(undefined); - }); - - it('should merge object even when 1st input is undefined because 2nd input is an object', () => { - const input1 = undefined; - const input2 = { firstName: 'John' }; - const output = objectAssignAndExtend(input1, input2); - expect(output).toEqual({ firstName: 'John' }); - }); - - it('should merge object even when 1st input is undefined because 2nd input is an object', () => { - const input1 = { firstName: 'John' }; - const input2 = undefined; - const output = objectAssignAndExtend(input1, input2); - expect(output).toEqual({ firstName: 'John' }); - }); - - it('should provide empty object as input and expect output object to include 2nd object', () => { - const input1 = {}; - const input2 = { firstName: 'John' }; - const output = objectAssignAndExtend(input1, input2); - expect(output).toEqual({ firstName: 'John' }); - }); - - it('should not overwrite property when already filled with a value', () => { - const input1 = { firstName: 'Jane' }; - const input2 = { firstName: { name: 'John' } }; - const output = objectAssignAndExtend(input1, input2); - expect(output).toEqual({ firstName: 'Jane' }); - }); - - it('should provide input object with undefined property and expect output object to return merged object from 2nd object when that one is filled', () => { - const input1 = { firstName: undefined }; - const input2 = { firstName: {} }; - const output = objectAssignAndExtend(input1, input2); - expect(output).toEqual({ firstName: {} }); - }); - - it('should provide input object with undefined property and not expect it to overwrite property since it already has a value', () => { - const input1 = { firstName: { name: 'John' } }; - const input2 = { firstName: undefined }; - const output = objectAssignAndExtend(input1, input2); - expect(output).toEqual({ firstName: { name: 'John' } }); - }); - - it('should merge 2 objects and expect objects to be merged with both side', () => { - const input1 = { a: 1, b: 1, c: { x: 1, y: 1 }, d: [1, 1] }; - const input2 = { b: 2, c: { y: 2, z: 2 }, d: [2, 2], e: 2 }; - - const output = objectAssignAndExtend(input1, input2); - expect(output).toEqual({ - a: 1, b: 1, c: { x: 1, y: 1, z: 2 }, - d: [1, 1], - e: 2 - }); - }); - - it('should merge 3 objects and expect objects to be merged with both side', () => { - const input1 = { a: 1, b: 1, c: { x: 1, y: 1 }, d: [1, 1] }; - const input2 = { b: 2, c: { y: 2, z: 2 } }; - const input3 = { d: [2, 2], e: 2 }; - - const output = objectAssignAndExtend(input1, input2, input3); - expect(output).toEqual({ - a: 1, b: 1, c: { x: 1, y: 1, z: 2 }, - d: [1, 1], - e: 2 - }); - }); - - it('should merge 3 objects, by calling objectAssignAndExtend 2 times, and expect objects to be merged with both side', () => { - const input1 = { a: 1, b: 1, c: { x: 1, y: 1 }, d: [1, 1] }; - const input2 = { b: 2, c: { y: 2, z: 2 } }; - const input3 = { d: [2, 2], e: 2 }; - - const output = objectAssignAndExtend(objectAssignAndExtend(input1, input2), input3); - expect(output).toEqual({ - a: 1, b: 1, c: { x: 1, y: 1, z: 2 }, - d: [1, 1], - e: 2 - }); - }); - }); - describe('emptyObject method', () => { it('should empty all object properties', () => { const obj = { firstName: 'John', address: { zip: 123456, streetNumber: '123 Belleville Blvd' } }; diff --git a/packages/utils/src/nodeExtend.ts b/packages/utils/src/nodeExtend.ts index f51b44129..824aca78b 100644 --- a/packages/utils/src/nodeExtend.ts +++ b/packages/utils/src/nodeExtend.ts @@ -58,7 +58,7 @@ const setProperty = function setProperty(target: any, options: any) { }; // Return undefined instead of __proto__ if '__proto__' is not an own property -const getProperty = function getProperty(obj: any, name: any) { +const getProperty = function getProperty(obj: any, name: string) { if (name === '__proto__') { if (!hasOwn.call(obj, name)) { return void 0; diff --git a/packages/utils/src/utils.ts b/packages/utils/src/utils.ts index f02c524b8..d6c23f5d9 100644 --- a/packages/utils/src/utils.ts +++ b/packages/utils/src/utils.ts @@ -57,11 +57,11 @@ export function deepCopy(objectOrArray: any | any[]): any | any[] { // Loop through each item in the original // Recursively copy it's value and add to the clone - for (const key in objectOrArray) { + Object.keys(objectOrArray).forEach(key => { if (Object.prototype.hasOwnProperty.call(objectOrArray, key)) { (clone as any)[key] = deepCopy(objectOrArray[key]); } - } + }); return clone; }; @@ -104,7 +104,7 @@ export function deepMerge(target: any, ...sources: any[]): any { target = (!isObject(target) && isObject(source)) ? {} : target; if (isObject(target) && isObject(source)) { - for (const prop in source) { + Object.keys(source).forEach(prop => { if (source.hasOwnProperty(prop)) { if (prop in target) { // handling merging of two properties with equal names @@ -128,53 +128,22 @@ export function deepMerge(target: any, ...sources: any[]): any { target[prop] = source[prop]; } } - } + }); } return deepMerge(target, ...sources); } -/** - * This method is similar to `Object.assign` with the exception that it will also extend the object properties when filled. - * There's also a distinction with extend vs merge, we are only extending when the property is not filled (if it is filled then it remains untouched and will not be merged) - * It also applies the change directly on the target object which mutates the original object. - * For example using these 2 objects: obj1 = { a: 1, b: { c: 2, d: 3 }} and obj2 = { b: { d: 2, e: 3}}: - * - Object.assign(obj1, obj2) => { a: 1, b: { e: 4 }} - * - objectAssignAndExtend(obj1, obj2) => { a: 1, b: { c: 2, d: 3, e: 4 } - * @param {Object} target - the target object — what to apply the sources properties and mutate into - * @param {Object} sources - the source object(s) — objects containing the properties you want to apply. - * @returns {Object} The target object. - */ -export function objectAssignAndExtend(target: any, ...sources: any): any { - if (!sources.length || sources[0] === undefined) { - return target; - } - const source = sources.shift(); - - // when target is not an object but source is an object, then we'll assign as object - target = (!isObject(target) && isObject(source)) ? {} : target; - - if (isObject(target) && isObject(source)) { - for (const key of Object.keys(source)) { - if (typeof source[key] === 'object' && source[key] !== null) { - objectAssignAndExtend(target[key], source[key]); - } - if ((target[key] === null || target[key] === undefined) && source[key] !== null && source[key] !== undefined) { - target[key] = source[key]; - } - } - } - return objectAssignAndExtend(target, ...sources); -} - /** * Empty an object properties by looping through them all and deleting them * @param obj - input object */ export function emptyObject(obj: any) { - for (const key in obj) { - if (obj.hasOwnProperty(key)) { - delete obj[key]; - } + if (isObject(obj)) { + Object.keys(obj).forEach(key => { + if (obj.hasOwnProperty(key)) { + delete obj[key]; + } + }); } obj = null; obj = {};