Skip to content

fix(utils): compare Date values by time in deepEqualObj - #16749

Open
yfwmaniish wants to merge 1 commit into
medusajs:developfrom
yfwmaniish:fix-deepequalobj-date-compare
Open

fix(utils): compare Date values by time in deepEqualObj#16749
yfwmaniish wants to merge 1 commit into
medusajs:developfrom
yfwmaniish:fix-deepequalobj-date-compare

Conversation

@yfwmaniish

Copy link
Copy Markdown

What

deepEqualObj reports two different Date values as equal:

deepEqualObj(new Date(0), new Date(999999))                       // true ❌ (should be false)
deepEqualObj({ created_at: new Date(0) }, { created_at: new Date(1) }) // true ❌

Why

After the primitive/null guards, the function compares two objects purely by their own enumerable keys:

const obj1Keys = Object.keys(obj1)
const obj2Keys = Object.keys(obj2)
if (obj1Keys.length !== obj2Keys.length) return false

Date instances have no own enumerable keys, so for any two dates both key lists are [], the lengths match, the loop body never runs, and the function returns true. This matters in practice because Medusa entities carry Date fields everywhere (created_at, updated_at, …), so a change-detection comparison on such objects can silently miss a differing date.

Fix

Compare Date values by their time before falling through to the key-based comparison:

if (obj1 instanceof Date || obj2 instanceof Date) {
  return (
    obj1 instanceof Date &&
    obj2 instanceof Date &&
    obj1.getTime() === obj2.getTime()
  )
}

A Date compared to a non-Date object is now correctly unequal, and equal dates stay equal. Plain-object comparison is unchanged.

Tests

Added a deepEqualObj case covering equal/unequal dates, nested date fields, and Date-vs-plain-object. Verified all existing cases still pass.

Copilot AI lite review requested due to automatic review settings September 7, 2026 10:03
@yfwmaniish
yfwmaniish requested a review from a team as a code owner September 7, 2026 10:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@changeset-bot

changeset-bot Bot commented Sep 7, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5157edf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 83 packages
Name Type
@medusajs/utils Patch
@medusajs/event-bus-redis Patch
@medusajs/framework Patch
@medusajs/modules-sdk Patch
@medusajs/orchestration Patch
@medusajs/query Patch
@medusajs/workflows-sdk Patch
@medusajs/cli Patch
@medusajs/medusa-oas-cli Patch
@medusajs/medusa Patch
@medusajs/test-utils Patch
@medusajs/analytics Patch
@medusajs/api-key Patch
@medusajs/auth Patch
@medusajs/cache-inmemory Patch
@medusajs/cache-redis Patch
@medusajs/caching Patch
@medusajs/cart Patch
@medusajs/currency Patch
@medusajs/customer Patch
@medusajs/event-bus-local Patch
@medusajs/file Patch
@medusajs/fulfillment Patch
@medusajs/index Patch
@medusajs/inventory Patch
@medusajs/link-modules Patch
@medusajs/locking Patch
@medusajs/notification Patch
@medusajs/order Patch
@medusajs/payment Patch
@medusajs/pricing Patch
@medusajs/product Patch
@medusajs/promotion Patch
@medusajs/rbac Patch
@medusajs/region Patch
@medusajs/sales-channel Patch
@medusajs/search Patch
@medusajs/settings Patch
@medusajs/stock-location Patch
@medusajs/store Patch
@medusajs/tax Patch
@medusajs/translation Patch
@medusajs/user Patch
@medusajs/workflow-engine-inmemory Patch
@medusajs/workflow-engine-redis Patch
@medusajs/analytics-local Patch
@medusajs/analytics-posthog Patch
@medusajs/auth-emailpass Patch
@medusajs/auth-github Patch
@medusajs/auth-google Patch
@medusajs/auth-oidc Patch
@medusajs/caching-redis Patch
@medusajs/file-local Patch
@medusajs/file-s3 Patch
@medusajs/fulfillment-manual Patch
@medusajs/locking-postgres Patch
@medusajs/locking-redis Patch
@medusajs/notification-local Patch
@medusajs/notification-sendgrid Patch
@medusajs/payment-stripe Patch
@medusajs/search-postgres Patch
@medusajs/draft-order Patch
@medusajs/loyalty-plugin Patch
@medusajs/core-flows Patch
integration-tests-http Patch
@medusajs/oas-github-ci Patch
@medusajs/instantsearch-adapter Patch
@medusajs/js-sdk Patch
@medusajs/types Patch
create-medusa-app Patch
@medusajs/http-types-generator Patch
@medusajs/deps Patch
@medusajs/eslint-plugin Patch
@medusajs/telemetry Patch
@medusajs/admin-bundler Patch
@medusajs/admin-sdk Patch
@medusajs/admin-shared Patch
@medusajs/admin-vite-plugin Patch
@medusajs/dashboard Patch
@medusajs/icons Patch
@medusajs/toolbox Patch
@medusajs/ui-preset Patch
@medusajs/ui Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@medusa-os-bot

medusa-os-bot Bot commented Sep 7, 2026

Copy link
Copy Markdown

Thanks for the contribution! Initial automated review looks good.

The contributor fixes a real bug in the deepEqualObj utility: JavaScript Date instances expose no own enumerable keys, so the existing key-length/loop comparison treated any two Date values as equal regardless of their actual time. The fix inserts a Date guard before the key-based path, correctly delegating to getTime() comparison and returning false when exactly one argument is a Date. The logic is sound, all existing test cases still pass, and four new cases cover equal dates, unequal dates, dates nested in plain objects, and Date-vs-plain-object cross-type comparisons. Checklist: - Changeset included and correctly formatted as a patch for @medusajs/utils. - Tests added in packages/core/utils/src/common/tests/deep-equal-obj.spec.ts. - Conventions followed: no semicolons, double quotes, 2-space indent, no issue/PR references in code comments. - No security, performance, or correctness concerns. Note (non-blocking): the PR body uses the section headers "Fix" and "Tests" rather than the template's "How" and "Testing". The content is substantive and satisfies the intent of both sections. Additionally, contribution guidelines ask for a linked issue for non-trivial fixes; this PR lacks one, though the description is self-contained and documents the bug thoroughly.

Triggered by: new PR opened

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants