Skip to content

refactor: remove lodash dependency#331

Open
inlined wants to merge 1 commit into
firebase:masterfrom
inlined:remove-lodash-dependency
Open

refactor: remove lodash dependency#331
inlined wants to merge 1 commit into
firebase:masterfrom
inlined:remove-lodash-dependency

Conversation

@inlined

@inlined inlined commented Jul 8, 2026

Copy link
Copy Markdown
Member

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

  • Compiled and ran the unit and integration tests: npm test
  • Validated with ESLint checks: npm run lint

### 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 gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +26 to +32
function isPlainObject(value: any): boolean {
return (
typeof value === 'object' &&
value !== null &&
Object.getPrototypeOf(value) === Object.prototype
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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;
}

Comment on lines +34 to +45
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;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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;
}

Comment thread src/v1.ts
cloudFunction: CloudFunction<T>
): WrappedScheduledFunction | WrappedFunction<T, CloudFunction<T>> {
if (!has(cloudFunction, '__endpoint')) {
if (!cloudFunction || !('__endpoint' in cloudFunction)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
if (!cloudFunction || !('__endpoint' in cloudFunction)) {
if (!cloudFunction || (typeof cloudFunction !== 'object' && typeof cloudFunction !== 'function') || !('__endpoint' in cloudFunction)) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants