refactor: remove lodash dependency#331
Conversation
### Description Removes lodash and @types/lodash as direct dependencies and replaces all usages with modern native ES6+ features and the existing ts-deepmerge library. ### Scenarios Tested - Compiled and ran the unit and integration tests: npm test - Validated with ESLint checks: npm run lint
There was a problem hiding this comment.
Code Review
This pull request removes the dependency on lodash by replacing its utility functions with native ES6+ features, custom lightweight helpers, and ts-deepmerge. The review feedback highlights a few areas of improvement for these custom helpers: updating isPlainObject to support objects created with Object.create(null) for full Lodash compatibility, simplifying mapValues using Object.entries(), and making the in operator check in wrapV1 safer against truthy primitives to prevent runtime TypeErrors.
| function isPlainObject(value: any): boolean { | ||
| return ( | ||
| typeof value === 'object' && | ||
| value !== null && | ||
| Object.getPrototypeOf(value) === Object.prototype | ||
| ); | ||
| } |
There was a problem hiding this comment.
The custom isPlainObject implementation does not handle objects created with Object.create(null) (which have a null prototype). In Lodash, _.isPlainObject returns true for such objects.
To ensure full compatibility with Lodash's behavior and prevent unexpected serialization failures when users pass clean dictionary objects (created via Object.create(null)), update the helper to also check for a null prototype.
| function isPlainObject(value: any): boolean { | |
| return ( | |
| typeof value === 'object' && | |
| value !== null && | |
| Object.getPrototypeOf(value) === Object.prototype | |
| ); | |
| } | |
| function isPlainObject(value: any): boolean { | |
| if (typeof value !== 'object' || value === null) { | |
| return false; | |
| } | |
| const proto = Object.getPrototypeOf(value); | |
| return proto === null || proto === Object.prototype; | |
| } |
| function mapValues<T, U>( | ||
| obj: Record<string, T>, | ||
| fn: (val: T) => U | ||
| ): Record<string, U> { | ||
| const res: Record<string, U> = {}; | ||
| for (const key in obj) { | ||
| if (Object.prototype.hasOwnProperty.call(obj, key)) { | ||
| res[key] = fn(obj[key]); | ||
| } | ||
| } | ||
| return res; | ||
| } |
There was a problem hiding this comment.
Since the goal of this PR is to leverage modern ES6+ features, we can simplify the mapValues helper. Using Object.entries() is cleaner, more idiomatic, and automatically avoids iterating over inherited prototype properties, eliminating the need for an explicit hasOwnProperty check.
function mapValues<T, U>(
obj: Record<string, T>,
fn: (val: T) => U
): Record<string, U> {
const res: Record<string, U> = {};
for (const [key, val] of Object.entries(obj)) {
res[key] = fn(val);
}
return res;
}| cloudFunction: CloudFunction<T> | ||
| ): WrappedScheduledFunction | WrappedFunction<T, CloudFunction<T>> { | ||
| if (!has(cloudFunction, '__endpoint')) { | ||
| if (!cloudFunction || !('__endpoint' in cloudFunction)) { |
There was a problem hiding this comment.
If cloudFunction is a truthy primitive (e.g., a string or number passed in JavaScript), evaluating '__endpoint' in cloudFunction will throw a TypeError (e.g., Cannot use 'in' operator to search for '__endpoint' in ...) instead of throwing the intended descriptive error.
To make this check completely safe, ensure cloudFunction is an object or function before using the in operator.
| if (!cloudFunction || !('__endpoint' in cloudFunction)) { | |
| if (!cloudFunction || (typeof cloudFunction !== 'object' && typeof cloudFunction !== 'function') || !('__endpoint' in cloudFunction)) { |
Removes lodash and @types/lodash as direct dependencies and replaces all usages with modern native ES6+ features and the existing ts-deepmerge library.
Description
Removes lodash and @types/lodash as direct dependencies and replaces all usages with modern native ES6+ features and the existing ts-deepmerge library.
Scenarios Tested
npm testnpm run lint