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
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
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)) { |
|
Nice to see lodash go. Three things from our side before this lands:
CI is green and the rest of the replacements checked out faithful (random, isEmpty at the live call sites, the resource-name param lookup is actually stricter than lodash get in a good way). |
2 participants
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