Skip to content

Commit

Permalink
fix(useMediaQuery): fix state stuck for concurrent mode (#866)
Browse files Browse the repository at this point in the history
In concurrent mode along with strict mode - synchronous mql state fetch
leads to state stuck. The only way to fix that behaviour is to switch
back to fully asynchronous code.

Upsides - way simpler code, and reduced amount of it.

Due to changes `useScreenOrientation` also affected.

fix #849

BREAKING CHANGE: `useMediaQuery` and `useScreenOrientation` are
asynchronous now and yields `undefined` at very first render, but
updates to actual value right after.
  • Loading branch information
xobotyi authored Jul 4, 2022
1 parent d9c4c6d commit 75db2b5
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 55 deletions.
12 changes: 8 additions & 4 deletions src/useMediaQuery/__docs__/example.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,20 @@ export const Example: React.FC = () => {
<br />
<br />
<div>
Small device (<code>max-width : 768px</code>): {isSmallDevice ? 'yes' : 'no'}
Small device (<code>max-width : 768px</code>):{' '}
{isSmallDevice === undefined ? 'unknown' : isSmallDevice ? 'yes' : 'no'}
</div>
<div>
Medium device (<code>max-width : 992px</code>): {isMediumDevice ? 'yes' : 'no'}
Medium device (<code>max-width : 992px</code>):{' '}
{isMediumDevice === undefined ? 'unknown' : isMediumDevice ? 'yes' : 'no'}
</div>
<div>
Large device (<code>max-width : 1200px</code>): {isLargeDevice ? 'yes' : 'no'}
Large device (<code>max-width : 1200px</code>):{' '}
{isLargeDevice === undefined ? 'unknown' : isLargeDevice ? 'yes' : 'no'}
</div>
<div>
Extra large device (<code>min-width : 1201px</code>): {isExtraLargeDevice ? 'yes' : 'no'}
Extra large device (<code>min-width : 1201px</code>):{' '}
{isExtraLargeDevice === undefined ? 'unknown' : isExtraLargeDevice ? 'yes' : 'no'}
</div>
</div>
);
Expand Down
6 changes: 2 additions & 4 deletions src/useMediaQuery/__docs__/story.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Tracks the state of CSS media query.
## Reference

```ts
export function useMediaQuery(query: string, matchOnMount?: boolean): boolean | undefined;
export function useMediaQuery(query: string): boolean | undefined;
```

#### Importing
Expand All @@ -32,10 +32,8 @@ export function useMediaQuery(query: string, matchOnMount?: boolean): boolean |
#### Arguments

- **query** _`string`_ - CSS media query to track.
- **matchOnMount** _`boolean`_ - whether hook state should be fetched during effects stage instead
of synchronous fetch. _Set this parameter to `true` for SSR use-cases._

#### Return

`boolean` - `true` in case document matches media query, `false` otherwise.
`undefined` - in case `matchOnMount` parameter set to `true` and effect stage is not done yet.
`undefined` - initially, when media query isn't matched yet.
8 changes: 1 addition & 7 deletions src/useMediaQuery/__tests__/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,8 @@ describe('useMediaQuery', () => {

it('should return undefined on first render', () => {
const { result } = renderHook(() => useMediaQuery('max-width : 768px'));
expect(result.all.length).toBe(1);
expect(result.current).toBe(false);
});

it('should return undefined on first render and `matchOnMount` set to false', () => {
const { result } = renderHook(() => useMediaQuery('max-width : 768px', true));
expect(result.all[0]).toBeUndefined();
expect(result.all.length).toBe(2);
expect(result.all[0]).toBe(undefined);
expect(result.current).toBe(false);
});

Expand Down
47 changes: 9 additions & 38 deletions src/useMediaQuery/useMediaQuery.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import { Dispatch, useCallback, useEffect, useRef } from 'react';
import { usePrevious, useRerender } from '..';
import { isBrowser } from '../util/const';
import { Dispatch, useEffect } from 'react';
import { useSafeState } from '..';

const queriesMap = new Map<
string,
{ mql: MediaQueryList; dispatchers: Set<Dispatch<boolean>>; listener: () => void }
>();

type QueryStateSetter = (matches: boolean, initial?: boolean) => void;
type QueryStateSetter = (matches: boolean) => void;

const querySubscribe = (query: string, setState: QueryStateSetter) => {
let entry = queriesMap.get(query);
Expand All @@ -31,7 +30,7 @@ const querySubscribe = (query: string, setState: QueryStateSetter) => {
}

entry.dispatchers.add(setState);
setState(entry.mql.matches, true);
setState(entry.mql.matches);
};

const queryUnsubscribe = (query: string, setState: QueryStateSetter): void => {
Expand All @@ -52,50 +51,22 @@ const queryUnsubscribe = (query: string, setState: QueryStateSetter): void => {
}
};

export function useMediaQuery(query: string, matchOnMount?: false): boolean;
export function useMediaQuery(query: string, matchOnMount: true): boolean | undefined;

/**
* Tracks the state of CSS media query.
*
* Defaults to false in SSR environments
* Return undefined initially and later receives correct value.
*
* @param query CSS media query to track.
* @param matchOnMount whether hook state should be fetched during effects stage instead of
* synchronous fetch. Set this parameter to `true` for SSR use-cases.
*/
export function useMediaQuery(query: string, matchOnMount?: boolean): boolean | undefined {
const rerender = useRerender();
const previousQuery = usePrevious(query);

const state = useRef<boolean | undefined>();

const setState = useCallback(
(matches: boolean, initial?: boolean) => {
if (state.current !== matches) {
state.current = matches;

if (!initial || matchOnMount) rerender();
}
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[matchOnMount]
);
export function useMediaQuery(query: string): boolean | undefined {
const [state, setState] = useSafeState<boolean>();

// do synchronous subscription only for case we are in browser and mount match required
if (!matchOnMount && isBrowser && previousQuery !== query) {
querySubscribe(query, setState);
}

// otherwise, match should happen in effect stage
useEffect(() => {
if (matchOnMount) {
querySubscribe(query, setState);
}
querySubscribe(query, setState);

return () => queryUnsubscribe(query, setState);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [query]);

return state.current;
return state;
}
6 changes: 4 additions & 2 deletions src/useScreenOrientation/useScreenOrientation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ export type ScreenOrientation = 'portrait' | 'landscape';
* As `Screen Orientation API` is still experimental and not supported by Safari, this
* hook uses CSS3 `orientation` media-query to check screen orientation.
*/
export function useScreenOrientation(): ScreenOrientation {
return useMediaQuery('(orientation: portrait)') ? 'portrait' : 'landscape';
export function useScreenOrientation(): ScreenOrientation | undefined {
const matches = useMediaQuery('(orientation: portrait)');

return typeof matches === 'undefined' ? undefined : matches ? 'portrait' : 'landscape';
}

0 comments on commit 75db2b5

Please sign in to comment.