feat(core): improve error message for BrowserOnly; better docs (#6291)

* feat(core): improve error message for BrowserOnly; better docs

* oops

* oops

* docs

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'},
+    },
+  },
 };

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": {

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 <BrowserOnly> must be a "render function", e.g. <BrowserOnly>{() => <span>{window.location.href}</span>}</BrowserOnly>.
+Current type: ${isValidElement(children) ? 'React element' : typeof children}`);
+    }
     return <>{children()}</>;
   }
 

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(
+        <BrowserOnly>
+          {/* @ts-expect-error test */}
+          <span>{window.location.href}</span>
+        </BrowserOnly>,
+      );
+    }).toThrowErrorMatchingInlineSnapshot(`
+      "Docusaurus error: The children of <BrowserOnly> must be a \\"render function\\", e.g. <BrowserOnly>{() => <span>{window.location.href}</span>}</BrowserOnly>.
+      Current type: React element"
+    `);
+  });
+  test('Should reject string children', () => {
+    expect(() => {
+      renderer.create(
+        // @ts-expect-error test
+        <BrowserOnly> </BrowserOnly>,
+      );
+    }).toThrowErrorMatchingInlineSnapshot(`
+      "Docusaurus error: The children of <BrowserOnly> must be a \\"render function\\", e.g. <BrowserOnly>{() => <span>{window.location.href}</span>}</BrowserOnly>.
+      Current type: string"
+    `);
+  });
+  test('Should accept valid children', () => {
+    expect(
+      renderer
+        .create(
+          <BrowserOnly fallback={<span>Loading</span>}>
+            {() => <span>{window.location.href}</span>}
+          </BrowserOnly>,
+        )
+        .toJSON(),
+    ).toMatchInlineSnapshot(`
+      <span>
+        https://docusaurus.io
+      </span>
+    `);
+  });
+});

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 `<BrowserOnly>` 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 (
+    <BrowserOnly>
+      {/* highlight-start */}
+      {/* DON'T DO THIS - doesn't actually work */}
+      <span>page url = {window.location.href}</span>
+      {/* highlight-end */}
+    </BrowserOnly>
+  );
+};
+```
+
+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/>` {#interpolate}
 
 A simple interpolation component for text containing dynamic placeholders.