Skip to content

Commit

Permalink
chore(utils): remove unnecessary objectAssignAndExtend method (#1280)
Browse files Browse the repository at this point in the history 
* chore(utils): remove unnecessary `objectAssignAndExtend` method
- we can use `node-extend` which does the job and already has plenty of unit tests
  • Loading branch information
@ghiscoding
ghiscoding authored Dec 22, 2023
1 parent 9d60a9d commit f6dc75c
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 139 deletions.
3 changes: 2 additions & 1 deletion packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 6 additions & 6 deletions packages/row-detail-view-plugin/src/slickRowDetailView.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 || '_';

Expand Down Expand Up @@ -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') {
Expand Down Expand Up @@ -239,7 +239,7 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV

/** set or change some of the plugin options */
setOptions(options: Partial<RowDetailViewOption>) {
this._addonOptions = objectAssignAndExtend(options, this._addonOptions);
this._addonOptions = extend(true, {}, this._addonOptions, options) as RowDetailView;
if (this._addonOptions?.singleRowExpand) {
this.collapseAll();
}
Expand All @@ -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();
}

Expand Down
90 changes: 0 additions & 90 deletions packages/utils/src/__tests__/utils.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
arrayRemoveItemByIndex,
deepCopy,
deepMerge,
objectAssignAndExtend,
emptyObject,
hasData,
isDefined,
Expand Down Expand Up @@ -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' } };
Expand Down
2 changes: 1 addition & 1 deletion packages/utils/src/nodeExtend.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
51 changes: 10 additions & 41 deletions packages/utils/src/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down Expand Up @@ -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
Expand All @@ -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 = {};
Expand Down

0 comments on commit f6dc75c

Please sign in to comment.