Skip to content

Conversation

@reekystive
Copy link

@reekystive reekystive commented Oct 23, 2025

Summary

This PR fixes an asymmetric compatibility issue in useMediaQuery where the addEventListener compatibility check was removed during subscription but still present during unsubscription, causing errors in older Safari versions.

The recent commit 4880540 removed the addEventListener / addListener fallback logic in createQueryEntry but kept it in queryUnsubscribe, creating an inconsistency. This causes issues in Safari versions that don't support addEventListener on MediaQueryList (Safari < 14).

Changes

  • Restored the addEventListener / addListener compatibility check in createQueryEntry to match the unsubscription logic in queryUnsubscribe
  • Ensures symmetric handling of event listeners for both subscription and unsubscription

Problem

In older Safari versions, MediaQueryList only supports the deprecated addListener / removeListener methods. The code was:

  • Using addEventListener unconditionally when subscribing (line 20)
  • Using conditional removeEventListener / removeListener when unsubscribing (lines 52-56)

This mismatch meant that in older Safari:

  1. Subscription would fail trying to call non-existent addEventListener
  2. Even if it didn't fail, the listener wouldn't be properly registered with addListener
  3. Unsubscription would try to call removeListener on a listener that was never added

@reekystive reekystive changed the title fix(useMediaQuery): restore addEventListener compatibility check for symmetry Fix compatibility issue for useMediaQuery Oct 23, 2025
@xobotyi xobotyi self-assigned this Jan 11, 2026
@xobotyi xobotyi requested a review from Copilot January 11, 2026 16:44
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a compatibility issue in the useMediaQuery hook where event listener subscription and unsubscription were handled asymmetrically, causing errors in Safari versions < 14 that only support the deprecated addListener/removeListener methods.

Changes:

  • Restored the addEventListener/addListener compatibility check in the createQueryEntry function to match the existing compatibility logic in queryUnsubscribe
  • Ensures symmetric handling of event listeners for both subscription and unsubscription operations

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

Comment on lines +20 to +24
if (mql.addEventListener) {
mql.addEventListener('change', listener, {passive: true});
} else {
mql.addListener(listener);
}
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

Consider adding test coverage for the legacy addListener/removeListener compatibility path. The current tests only mock addEventListener and removeEventListener. Adding a test that mocks a MediaQueryList without addEventListener would ensure this compatibility code path is tested.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants