New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(useMediaQuery): only add/remove event listeners on query change #3236
Conversation
4cf721b
to
3279cf1
Compare
packages/core/useMediaQuery/index.ts
Outdated
if (!isSupported.value) | ||
return | ||
|
||
cleanup() | ||
|
||
mediaQuery = window!.matchMedia(toRef(query).value) | ||
mediaQuery = window!.matchMedia(toValue(query)) | ||
matches.value = !!mediaQuery?.matches | ||
|
||
if (!mediaQuery) | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be possible to assert that if isSupported.value = true
, then window.matchMedia
always returns a MediaQueryList
unless there exists a noncompliant implementation that this aims to handle.
This should allow us to simplify
matches.value = !!mediaQuery?.matches
if (!mediaQuery)
return
to:
matches.value = mediaQuery.matches
Falsy checks are a remnant from previous versions of the code before isSupported was introduced, where code path existed that skipped calling window.matchMedia.
a01d4c0
to
330fd8b
Compare
I think there's a test failure right now caused by a |
Fixes "Error: Illegal characters in path." on Windows
Before submitting the PR, please make sure you do the following
fixes #123
).Description
This pull request changes
useMediaQuery
to only register event listeners when it is first created or when the reactive value for the passed inquery
changes in order to avoid re-creating the event listeners on everychange
event. This will avoid any excess overhead in the event handler itself.Fixes #3235
Additional context
In the process,
toRef(query).value
was changedtoValue(query)
. I am not sure nor haven't tried yet whether this preserves the reactivity insidewatchEffect
. If this works as intended, it should be a slight improvement in bundled code size. Update: Reactivity seems to be working fine.🤖 Generated by Copilot at 4cf721b
Enhance
useMediaQuery
to support getter functions and fix a bug. Refactor the code for better performance and readability.🤖 Generated by Copilot at 4cf721b
toRef
withtoValue
to supportMaybeRefOrGetter
type forquery
argument ofuseMediaQuery
(link, link)update
andcleanup
functions to improve performance, readability, and bug fix (link, link)watchEffect
call insidestopWatch
variable and pass it totryOnScopeDispose
to avoid unnecessary effects and ensure proper cleanup (link)