[Kropiunig]

Summary

compat's maxBy/minBy diverge from lodash (and from their own sibling max/min) when the iteratee yields null or undefined for elements — e.g. a missing/optional key.

They only skip NaN:

if (Number.isNaN(current)) {
  continue;
}

but max/min skip current == null (and symbols) as well. Two problems follow:

1. max/min is initialised to undefined and reused as the "not yet seen" sentinel (max === undefined). An undefined iteratee result sets the sentinel back to undefined, so it never flips and every later element overwrites the result — the function returns the last element instead of undefined.
2. A null result is coerced to 0 by the > / < comparison, so it can be picked as the extremum.

Reproduction

import { maxBy, minBy } from 'es-toolkit/compat';
import _ from 'lodash';

maxBy([{ a: 1 }, { a: 2 }], 'b');        // es-toolkit: { a: 2 }   lodash: undefined
maxBy([{ a: null }, { a: null }], 'a');  // es-toolkit: { a: null } lodash: undefined
minBy([5, undefined, 3, null], x => x);  // es-toolkit: null        lodash: 3

Fix

Mirror the guard already used by max/min so null, undefined, and symbol values are skipped. With those values skipped, the existing === undefined sentinel becomes reliable again (it can only ever be set from a real comparable value).

if (current == null || Number.isNaN(current) || typeof current === 'symbol') {
  continue;
}

Verification

• All existing maxBy/minBy specs still pass (Date, ±Infinity, string iteratees, NaN skipping, single/large arrays).
• Added regression tests covering missing keys and mixed null/undefined/numeric values; they fail on main and pass with the fix.
• Differentially fuzz-tested the patched functions against lodash over several thousand combinations of undefined/null/NaN/±Infinity/number/string values (property, function, and identity iteratees) with zero divergences.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
es-toolkit	Ready	Preview, Comment	Jul 22, 2026 9:34am

[Copilot]

Pull request overview

This PR fixes Lodash-compatibility issues in es-toolkit/compat’s maxBy/minBy when an iteratee produces null or undefined, aligning their skip-logic with the existing compat max/min guard (and Lodash’s typical extremum behavior).

Changes:

• Update maxBy/minBy to skip null, undefined, NaN, and symbol iteratee results before comparing.
• Add regression tests ensuring nullish iteratee results don’t reset the “unset” sentinel state and don’t get coerced into comparable numbers.
• Add tests asserting undefined is returned when no comparable iteratee output exists.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
src/compat/math/minBy.ts	Skips null/undefined/NaN/symbol iteratee outputs before updating the current minimum.
src/compat/math/minBy.spec.ts	Adds regression coverage for nullish iteratee outputs and “no comparable values” returning undefined.
src/compat/math/maxBy.ts	Skips null/undefined/NaN/symbol iteratee outputs before updating the current maximum.
src/compat/math/maxBy.spec.ts	Adds regression coverage for nullish iteratee outputs and “no comparable values” returning undefined.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Antoliny0919]

Nice catch @Kropiunig !

It looks like there were a few compatibility behaviors that were missing!

The current implementation already looks really good, but I'd like to suggest a few improvements.

[Antoliny0919]

In Lodash, an error is only thrown when a Symbol appears after the first element. My guess is that this is actually an implementation bug in Lodash.

const sym = Symbol('x');
console.log(lodash.maxBy([{ a: 1 }, { a: sym }], 'a'));  // Error !!!

While compat should match Lodash's behavior as closely as possible, do you think it would be reasonable to make an exception for this case 🤔

[Antoliny0919]

It would be great to add some test cases for Symbol as well! (same as maxBy)

[Antoliny0919]

Suggested change

	expect(maxBy([{ a: undefined }, { a: 5 }, { a: null }], 'a')).toEqual({ a: 5 });
	expect(maxBy([{ a: undefined }, { a: -5 }, { a: null }], 'a')).toEqual({ a: -5 });

This test case covers the current change well.
However, it also passes on the previous implementation.

I think that's because it compares positive numbers with null and undefined.
What do you think about using negative numbers instead?
With negative values, the previous implementation would behave differently, so selecting the negative number would verify that the fix is actually working.

[Kropiunig]

Thanks for the careful review!

Applied your negative-value suggestion to the maxBy test — with a negative maximum, null (coerced to 0) would wrongly win on the old code, so the assertion now genuinely fails before the fix and passes after. For minBy I kept positive values, since that is the mirror case where 0 wrongly wins as the minimum; that test already failed on the old code. I also added Symbol coverage to both specs (I confirmed each new test fails on the pre-fix guard and passes now).

On the Symbol design question: I left the current skip-and-do-not-throw behavior in place. lodash only throws when a Symbol appears after the first element, which is position-dependent and looks like a bug, and never throwing feels safer for compat consumers who pass in mixed data. That said, this is your call — happy to switch to strict lodash parity and replicate the throw if you would rather match it exactly.

[raon0211]

Thanks for the catch! Let's stick with the current implementation for simplicity.