diff --git a/jest.config.mjs b/jest.config.mjs index 4d11b9bb17..a15052fea6 100644 --- a/jest.config.mjs +++ b/jest.config.mjs @@ -49,4 +49,9 @@ export default { '@docusaurus/plugin-content-docs/client': '@docusaurus/plugin-content-docs/lib/client/index.js', }, + globals: { + window: { + location: {href: 'https://docusaurus.io'}, + }, + }, }; diff --git a/packages/docusaurus/package.json b/packages/docusaurus/package.json index cacf7bcf56..9548ef4253 100644 --- a/packages/docusaurus/package.json +++ b/packages/docusaurus/package.json @@ -119,6 +119,7 @@ "@types/rtl-detect": "^1.0.0", "@types/serve-handler": "^6.1.1", "@types/webpack-bundle-analyzer": "^4.4.1", + "react-test-renderer": "^17.0.2", "tmp-promise": "^3.0.2" }, "peerDependencies": { diff --git a/packages/docusaurus/src/client/exports/BrowserOnly.tsx b/packages/docusaurus/src/client/exports/BrowserOnly.tsx index b1166a2957..b1beedb416 100644 --- a/packages/docusaurus/src/client/exports/BrowserOnly.tsx +++ b/packages/docusaurus/src/client/exports/BrowserOnly.tsx @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -import React from 'react'; +import React, {isValidElement} from 'react'; import useIsBrowser from '@docusaurus/useIsBrowser'; // Similar comp to the one described here: @@ -14,12 +14,19 @@ function BrowserOnly({ children, fallback, }: { - children?: () => JSX.Element; + children: () => JSX.Element; fallback?: JSX.Element; }): JSX.Element | null { const isBrowser = useIsBrowser(); - if (isBrowser && children != null) { + if (isBrowser) { + if ( + typeof children !== 'function' && + process.env.NODE_ENV === 'development' + ) { + throw new Error(`Docusaurus error: The children of must be a "render function", e.g. {() => {window.location.href}}. +Current type: ${isValidElement(children) ? 'React element' : typeof children}`); + } return <>{children()}; } diff --git a/packages/docusaurus/src/client/exports/__tests__/BrowserOnly.test.tsx b/packages/docusaurus/src/client/exports/__tests__/BrowserOnly.test.tsx new file mode 100644 index 0000000000..89161383b5 --- /dev/null +++ b/packages/docusaurus/src/client/exports/__tests__/BrowserOnly.test.tsx @@ -0,0 +1,55 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import React from 'react'; +import renderer from 'react-test-renderer'; +import BrowserOnly from '../BrowserOnly'; + +jest.mock('@docusaurus/useIsBrowser', () => () => true); + +describe('BrowserOnly', () => { + test('Should reject react element children', () => { + process.env.NODE_ENV = 'development'; + expect(() => { + renderer.create( + + {/* @ts-expect-error test */} + {window.location.href} + , + ); + }).toThrowErrorMatchingInlineSnapshot(` + "Docusaurus error: The children of must be a \\"render function\\", e.g. {() => {window.location.href}}. + Current type: React element" + `); + }); + test('Should reject string children', () => { + expect(() => { + renderer.create( + // @ts-expect-error test + , + ); + }).toThrowErrorMatchingInlineSnapshot(` + "Docusaurus error: The children of must be a \\"render function\\", e.g. {() => {window.location.href}}. + Current type: string" + `); + }); + test('Should accept valid children', () => { + expect( + renderer + .create( + Loading}> + {() => {window.location.href}} + , + ) + .toJSON(), + ).toMatchInlineSnapshot(` + + https://docusaurus.io + + `); + }); +}); diff --git a/website/docs/docusaurus-core.md b/website/docs/docusaurus-core.md index 6fec152f62..798fce738f 100644 --- a/website/docs/docusaurus-core.md +++ b/website/docs/docusaurus-core.md @@ -228,6 +228,29 @@ const MyComponent = (props) => { }; ``` +:::info Why do we use a "render function"? + +It's important to realize that the children of `` is not a JSX element, but a function that _returns_ an element. This is a design decision. Consider this code: + +```jsx +import BrowserOnly from '@docusaurus/BrowserOnly'; + +const MyComponent = () => { + return ( + + {/* highlight-start */} + {/* DON'T DO THIS - doesn't actually work */} + page url = {window.location.href} + {/* highlight-end */} + + ); +}; +``` + +While you may expect that `BrowserOnly` hides away the children during server-side rendering, it actually can't. When the React renderer tries to render this JSX tree, it does see the `{window.location.href}` variable as a node of this tree and tries to render it, although it's actually not used! Using a function ensures that we only let the renderer see the browser-only component when it's needed. + +::: + ### `` {#interpolate} A simple interpolation component for text containing dynamic placeholders.