Skip to content

Commit

Permalink
style(eslint): apply specified code style changes (#138)
Browse files Browse the repository at this point in the history
Refs: https://babeljs.io/docs/en/assumptions#setspreadproperties

Fixes: #141
Fixes: #139

Co-authored-by: Brian Greig <brian@resurgencewebdesign.com>
Co-authored-by: septs <github@septs.pw>

Signed-off-by: Derek Lewis <DerekNonGeneric@inf.is>
  • Loading branch information
DerekNonGeneric committed Oct 17, 2022
1 parent dc92c4a commit 34fdb2a
Showing 1 changed file with 48 additions and 45 deletions.
93 changes: 48 additions & 45 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,14 @@ const _hasOwn = Object.prototype.hasOwnProperty;
* @returns {T}
* @template T
*/
export function map<T>(opt_initial: T | undefined) {
const obj = Object.create(null);
export const map = <T>(opt_initial: T | null | undefined): object => {
const object = Object.create(null);
if (opt_initial) {
Object.assign(obj, opt_initial);
Object.assign(object, opt_initial);
}
return obj;

// FIXME(@DerekNonGeneric): Should we be creating objects w/ null protos?
return { ...opt_initial };
}

/**
Expand All @@ -46,8 +48,8 @@ export function map<T>(opt_initial: T | undefined) {
* @returns {boolean}
* @template T
*/
export function hasOwn<T>(obj: T, key: string) {
return _hasOwn.call(obj, key);
export const hasOwn = <T>(object: T, key: string): boolean => {
return _hasOwn.call(object, key);
}

/**
Expand All @@ -57,42 +59,43 @@ export function hasOwn<T>(obj: T, key: string) {
* @param {string} key
* @returns {unknown}
*/
export function ownProperty(obj: Record<string, number | RegExp>, key: string) {
if (hasOwn(obj, key)) {
return obj[key];
} else {
return undefined;
}
export const ownProperty = (
object: Record<string, number | RegExp>,
key: string,
): unknown => {
return hasOwn(object, key) ? Reflect.get(object, key) : undefined;
}

interface ITargetSourceDepth {
t: Object;
s: Object;
/** @typedef {{t: object, s: object, d: number}} DeepMergeTuple */
type DeepMergeTuple = {
t: object;
s: object;
d: number;
}

/**
* Deep merges source into target.
*
* @param {!Object} target
* @param {!Object} source
* @param {number} depth The maximum merge depth. If exceeded, Object.assign
* will be used instead.
* @returns {!Object}
* @param {!object} target
* @param {!object} source
* @param {!number} depth The maximum merge depth. If exceeded, `Object.assign`
* will be used instead.
* @return {!object}
* @throws {Error} If source contains a circular reference.
* Note: Only nested objects are deep-merged, primitives and arrays are not.
*/
export function deepMerge(target: Object, source: Object, depth = 10): Object {
export const deepMerge = (target: object, source: object, depth = 10): object => {
// Keep track of seen objects to detect recursive references.
const seen: Array<Object> = [];
/** @type {!object[]} */
const seen: object[] = [];

/** @type {!Array<ITargetSourceDepth>} */
const queue: Array<ITargetSourceDepth> = [];
/** @type {!DeepMergeTuple[]} */
const queue: DeepMergeTuple[] = [];
queue.push({ t: target, s: source, d: 0 });

// BFS to ensure objects don't have recursive references at shallower depths.
while (queue.length > 0) {
const { t, s, d } = map(queue.shift());
const { t, s, d } = /** @type {!DeepMergeTuple} */ Object(queue.shift());
if (seen.includes(s)) {
throw new Error('Source object has a circular reference.');
}
Expand All @@ -104,19 +107,19 @@ export function deepMerge(target: Object, source: Object, depth = 10): Object {
Object.assign(t, s);
continue;
}
Object.keys(s).forEach((key) => {
const newValue = s[key];
for (const key of Object.keys(s)) {
const newValue = Reflect.get(s, key);
// Perform a deep merge IFF both target and source have the same key
// whose corresponding values are objects.
if (hasOwn(t, key)) {
const oldValue = t[key];
const oldValue = Reflect.get(t, key);
if (isObject(newValue) && isObject(oldValue)) {
queue.push({ t: oldValue, s: newValue, d: d + 1 });
return;
continue;
}
}
t[key] = newValue;
});
Reflect.set(t, key, newValue);
}
}
return target;
}
Expand All @@ -126,11 +129,11 @@ export function deepMerge(target: Object, source: Object, depth = 10): Object {
* @param {!Record<string, number | RegExp> | null | undefined} o2
* @returns {boolean}
*/
export function objectsEqualShallow(
export const objectsEqualShallow = (
o1: Record<string, number | RegExp> | null | undefined,
o2: Record<string, number | RegExp> | null | undefined
): boolean {
if (o1 === null || o2 === null) {
): boolean => {
if (o1 == undefined || o2 == undefined) {

This comment has been minimized.

Copy link
@DerekNonGeneric

DerekNonGeneric Oct 18, 2022

Author Member

This change might actually be incorrect. The way we had it before (in the previous revision as is evident by this diff), it would check whether the object was null or undefined due to the looseness of the double equals comparison, so the only thing this may have accomplished was silencing some lint rule that disallowed the use of null in our codebase, which i am not yet convinced would be included as a guideline in our coding standard.

What would have been best here — to satisfy the lint warning about not being allowed to use double equals signs (something i tend to agree with) — we should have been able to use something like isNullOrUndefined as seen in Node.js Utils module (unfortunately deprecated).

Refs: https://nodejs.org/api/util.html#utilisnullorundefinedobject

Few JS features draw more ire in the broader JS community than the == operator, generally referred to as the "loose equality" operator. The majority of all writing and public discourse on JS condemns this operator as poorly designed and dangerous/bug-ridden when used in JS programs. Even the creator of the language himself, Brendan Eich, has lamented how it was designed as a big mistake.

Refs: https://github.com/getify/You-Dont-Know-JS/blob/2nd-ed/get-started/ch2.md#coercive-comparisons

See: #140

This comment has been minimized.

Copy link
@DerekNonGeneric

DerekNonGeneric Nov 5, 2022

Author Member

[…] so the only thing this may have accomplished was silencing some lint rule that disallowed the use of null in our codebase, which i am not yet convinced would be included as a guideline in our coding standard.

See: https://github.com/openinf/util-object/wiki/Avoid-all-uses-of-%60undefined%60

@ignoreintuition introduced this (well, we let him), but it seems like undefined is really risky to use and maybe should be outright disallowed in all our codebases? I would be inclined to at least have ESLint warning about it for now (maybe not erroring until we have a preferred replacement).

/cc @septs @ignoreintuition

This comment has been minimized.

Copy link
@DerekNonGeneric

DerekNonGeneric Nov 5, 2022

Author Member

I highly, highly recommend reading TS 4.8 release announcement, where this seems to be much improved (that's the latest minor version of TS and what most folks should be on (and we currently are on).

Specifically, check out #improved-intersection-reduction-union-compatibility-and-narrowing where they give us actual code as the resolution for our problem here.

This comment has been minimized.

Copy link
@DerekNonGeneric

DerekNonGeneric Nov 5, 2022

Author Member
if ((o1 === undefined || o1 === null) && (o2 === undefined || o2 === null)) {
  // Null is only equal to null, and undefined to undefined.
  return o1 === o2;
}

This comment has been minimized.

Copy link
@DerekNonGeneric

DerekNonGeneric Nov 5, 2022

Author Member
if (isNullish(o1) || isNullish(o2)) {
  // Null is only equal to null, and undefined to undefined.
  return o1 === o2;
}
// Null is only equal to null, and undefined to undefined.
return o1 === o2;
}
Expand All @@ -153,21 +156,21 @@ export function objectsEqualShallow(
* updates the object originally passed, and returns the value that was returned
* by the factory function.
*
* @param {T} obj
* @param {string} prop
* @param {T extends object} object
* @param {string} property
* @param {function(T, string):R} factory
* @returns {R}
* @template T,R
* @template P,T,R
*/
export function memo<T, P extends keyof T>(
obj: T,
prop: P,
factory: (arg0: T, arg1: P) => T[P]
): T[P] {
let result = obj[prop];
export const memo = <T extends object, P extends keyof T>(
object: T,
property: P,
factory: (argument0: T, argument1: P) => T[P],
): T[P] => {
let result = Reflect.get(object, property);
if (result === undefined) {
result = factory(obj, prop);
obj[prop] = result;
result = factory(object, property);
Reflect.set(object, property, result);
}
return result;
}

0 comments on commit 34fdb2a

Please sign in to comment.