Skip to content

Commit

Permalink
fix: first render consistency of useBreakpointValue in SSR+CSR envs (#…
Browse files Browse the repository at this point in the history
…5651)

* test(media-query): fixed that the same environment as SSR could not be reproduced

* fix(media-query): don't ignore defaultBreakpoint when using SSR

* chore: add changeset

* test(media-query): add 2xl into test breakpoints

* fix: first render consistency in SSR+CSR envs

* fix: move default value for defaultBreakpoint to useBreakpoint

Co-authored-by: ishowta <ishowta@gmail.com>
  • Loading branch information
TimKolberger and ishowta committed Feb 24, 2022
1 parent 27eec8d commit d6bed34
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 31 deletions.
9 changes: 9 additions & 0 deletions .changeset/thick-swans-attend.md
@@ -0,0 +1,9 @@
---
"@chakra-ui/media-query": patch
---

- Fixed an issue that undefined is returned when calling the hook
`useBreakpoint` with `defaultValue` specified in SSR

- Fixed an issue where the value of `useBreakpointValue` in CSR did not match
SSR.
2 changes: 1 addition & 1 deletion packages/media-query/src/use-breakpoint-value.ts
Expand Up @@ -8,7 +8,7 @@ import { useBreakpoint } from "./use-breakpoint"
* provided responsive values object.
*
* @param values
* @param defaultBreakpoint default breakpoint name
* @param [defaultBreakpoint] default breakpoint name
* (in non-window environments like SSR)
*
* For SSR, you can use a package like [is-mobile](https://github.com/kaimallea/isMobile)
Expand Down
31 changes: 19 additions & 12 deletions packages/media-query/src/use-breakpoint.ts
Expand Up @@ -5,13 +5,15 @@ import { useTheme } from "@chakra-ui/system"
/**
* React hook used to get the current responsive media breakpoint.
*
* @param defaultBreakpoint default breakpoint name
* @param [defaultBreakpoint="base"] default breakpoint name
* (in non-window environments like SSR)
*
* For SSR, you can use a package like [is-mobile](https://github.com/kaimallea/isMobile)
* to get the default breakpoint value from the user-agent
*/
export function useBreakpoint(defaultBreakpoint?: string) {
export function useBreakpoint(
defaultBreakpoint = "base", // default value ensures SSR+CSR consistency
) {
const { __breakpoints } = useTheme()
const env = useEnvironment()

Expand All @@ -25,20 +27,25 @@ export function useBreakpoint(defaultBreakpoint?: string) {
)

const [currentBreakpoint, setCurrentBreakpoint] = React.useState(() => {
if (env.window.matchMedia) {
// set correct breakpoint on first render
const matchingBreakpointDetail = queries.find(
({ query }) => env.window.matchMedia(query).matches,
)
return matchingBreakpointDetail?.breakpoint
}

if (defaultBreakpoint) {
// use fallback if available
// use default breakpoint to ensure render consistency in SSR + CSR environments
// => first render on the client has to match the render on the server
const fallbackBreakpointDetail = queries.find(
({ breakpoint }) => breakpoint === defaultBreakpoint,
)
return fallbackBreakpointDetail?.breakpoint
if (fallbackBreakpointDetail) {
return fallbackBreakpointDetail.breakpoint
}
}

if (env.window.matchMedia) {
// set correct breakpoint on first render if no default breakpoint was provided
const matchingBreakpointDetail = queries.find(
({ query }) => env.window.matchMedia(query).matches,
)
if (matchingBreakpointDetail) {
return matchingBreakpointDetail.breakpoint
}
}

return undefined
Expand Down
9 changes: 6 additions & 3 deletions packages/media-query/tests/test-data.ts
@@ -1,3 +1,4 @@
import { extendTheme } from "@chakra-ui/react"
import { createBreakpoints } from "@chakra-ui/theme-tools"

export const breakpoints = createBreakpoints({
Expand All @@ -6,16 +7,18 @@ export const breakpoints = createBreakpoints({
md: "200px",
lg: "300px",
xl: "400px",
customBreakpoint: "500px",
"2xl": "500px",
customBreakpoint: "600px",
})

export const theme = { breakpoints }
export const theme = extendTheme({ breakpoints })

export const queries = {
base: "(min-width: 0px) and (max-width: 99px)",
sm: "(min-width: 100px) and (max-width: 199px)",
md: "(min-width: 200px) and (max-width: 299px)",
lg: "(min-width: 300px) and (max-width: 399px)",
xl: "(min-width: 400px) and (max-width: 499px)",
customBreakpoint: "(min-width: 500px)",
"2xl": "(min-width: 500px) and (max-width: 599px)",
customBreakpoint: "(min-width: 600px)",
}
15 changes: 12 additions & 3 deletions packages/media-query/tests/use-breakpoint-value-ssr.test.tsx
@@ -1,9 +1,18 @@
import React from "react"
import { renderToStaticMarkup } from "react-dom/server"
import { ThemeProvider } from "@chakra-ui/system"
import { ChakraProvider } from "@chakra-ui/react"
import { theme } from "./test-data"
import { useBreakpointValue } from "../src"

jest.mock("@chakra-ui/utils", () => ({
...jest.requireActual("@chakra-ui/utils"),
isBrowser: false,
}))

beforeEach(() => {
jest.resetAllMocks()
})

describe("with defaultBreakpoint", () => {
// To clean up erroneous console warnings from react, we temporarliy force
// useLayoutEffect to behave like useEffect. Since neither can run in our SSR
Expand Down Expand Up @@ -100,9 +109,9 @@ function ssrRenderWithDefaultBreakpoint(
defaultBreakpoint: string,
) {
return renderToStaticMarkup(
<ThemeProvider theme={theme}>
<ChakraProvider theme={theme}>
<TestComponent values={values} defaultBreakpoint={defaultBreakpoint} />
</ThemeProvider>,
</ChakraProvider>,
)
}

Expand Down
50 changes: 38 additions & 12 deletions packages/media-query/tests/use-breakpoint-value.test.tsx
Expand Up @@ -17,19 +17,20 @@ describe("with object", () => {
})

const values = {
base: "base",
sm: "sm",
md: "md",
lg: "lg",
xl: "xl",
customBreakpoint: "customBreakpoint",
base: "__base__",
sm: "__sm__",
md: "__md__",
lg: "__lg__",
xl: "__xl__",
"2xl": "__2xl__",
customBreakpoint: "__customBreakpoint__",
}

test("uses base value if smaller than sm", () => {
renderWithQuery(values, queries.base)

Object.keys(values).forEach((key) => {
if (key === "base") {
if (key === "__base__") {
expect(screen.getByText(key)).toBeInTheDocument()
} else {
expect(screen.queryByText(key)).not.toBeInTheDocument()
Expand All @@ -41,7 +42,7 @@ describe("with object", () => {
renderWithQuery(values, queries.sm)

Object.keys(values).forEach((key) => {
if (key === "sm") {
if (key === "__sm__") {
expect(screen.getByText(key)).toBeInTheDocument()
} else {
expect(screen.queryByText(key)).not.toBeInTheDocument()
Expand All @@ -53,7 +54,7 @@ describe("with object", () => {
renderWithQuery(values, queries.md)

Object.keys(values).forEach((key) => {
if (key === "md") {
if (key === "__md__") {
expect(screen.getByText(key)).toBeInTheDocument()
} else {
expect(screen.queryByText(key)).not.toBeInTheDocument()
Expand All @@ -65,7 +66,7 @@ describe("with object", () => {
renderWithQuery(values, queries.lg)

Object.keys(values).forEach((key) => {
if (key === "lg") {
if (key === "__lg__") {
expect(screen.getByText(key)).toBeInTheDocument()
} else {
expect(screen.queryByText(key)).not.toBeInTheDocument()
Expand All @@ -77,7 +78,19 @@ describe("with object", () => {
renderWithQuery(values, queries.xl)

Object.keys(values).forEach((key) => {
if (key === "xl") {
if (key === "__xl__") {
expect(screen.getByText(key)).toBeInTheDocument()
} else {
expect(screen.queryByText(key)).not.toBeInTheDocument()
}
})
})

test("2xl", () => {
renderWithQuery(values, queries.xl)

Object.keys(values).forEach((key) => {
if (key === "__2xl__") {
expect(screen.getByText(key)).toBeInTheDocument()
} else {
expect(screen.queryByText(key)).not.toBeInTheDocument()
Expand All @@ -89,7 +102,7 @@ describe("with object", () => {
renderWithQuery(values, queries.customBreakpoint)

Object.keys(values).forEach((key) => {
if (key === "customBreakpoint") {
if (key === "__customBreakpoint__") {
expect(screen.getByText(key)).toBeInTheDocument()
} else {
expect(screen.queryByText(key)).not.toBeInTheDocument()
Expand Down Expand Up @@ -118,6 +131,7 @@ describe("with array", () => {
"value2",
"value3",
"value4",
"value5",
"anotherValue",
"customBreakpoint",
]
Expand Down Expand Up @@ -166,6 +180,18 @@ describe("with array", () => {
test("xl", () => {
renderWithQuery(values, queries.xl)

values.forEach((value) => {
if (value === "value5") {
expect(screen.getByText(value)).toBeInTheDocument()
} else {
expect(screen.queryByText(value)).not.toBeInTheDocument()
}
})
})

test("2xl", () => {
renderWithQuery(values, queries["2xl"])

values.forEach((value) => {
if (value === "anotherValue") {
expect(screen.getByText(value)).toBeInTheDocument()
Expand Down

1 comment on commit d6bed34

@vercel
Copy link

@vercel vercel bot commented on d6bed34 Feb 24, 2022

Choose a reason for hiding this comment

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

Please sign in to comment.