feat(v2): add a way to exclude components from build-time prerendering by lex111 · Pull Request #2323 · facebook/docusaurus

lex111 · 2020-02-26T00:40:00Z

Motivation

Resolve #2273.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Inside ClientOnly put component that used window object, eg.

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

docusaurus-bot · 2020-02-26T00:42:52Z

Deploy preview for docusaurus-2 ready!

Built with commit 63ee326

https://deploy-preview-2323--docusaurus-2.netlify.com

yangshun · 2020-02-26T00:48:45Z

+import React, {useEffect, useState} from 'react';
+
+const ClientOnly = ({children}) => {
+  const [isClient, setIsClient] = useState(false);


I think isComponentMounted will be a better name.

yangshun · 2020-02-26T01:01:25Z

+import React, {useEffect, useState} from 'react';
+
+const BrowserOnly = ({children}) => {
+  const [isClient, setIsClient] = useState(false);


Btw I'm wondering if we can use the ExecutionEnvironment.canUseDOM instead, then there's no need for a state right?

Why so? This state is needed.

What's the diff between doing your way and this:

function BrowserOnly({children}) => { return ExecutionEnvironment.canUseDOM && children; }

It seemed to me that all the work of this component is to use useEffect, isn't it?

Yes and I think we don't even need useEffect here. Can you try what I've proposed and see whether there's any difference? :/

It seems that none of the options actually works, it really puzzled me and in complete dismay 🤒

P.S. For the test, I used the @u-wave/react-vimeo component in the MDX file.

So even your current implementation doesn't work? Luckily we test out first lol. Maybe the component is running some side effects that relies on browser environments being present during import. In that case there's no way around it.

Not exactly, rather why something does not work, this component on some dependencies.

After some investigation I found out that build fails if a component using a window object is imported, how is this possible? Therefore the BrowserOnly component is useless here 🤷‍♂️

yangshun

I went to test this component out and it doesn't seem to work. I'm not sure why but I think we can't let the children be JSX, we have to let it be a render function.

To reproduce the bug, add

<BrowserOnly>{window.location.pathname}</BrowserOnly>

to any page and try to build it. It will fail.

But if you do this:

<BrowserOnly>{() => window.location.pathname}</BrowserOnly>

along with the suggested change of doing children() within BrowserOnly.js, it will be fine.

Will need to change the docs also and say the contents have to be a render function. Will also be good to have tests.

yangshun · 2020-02-29T06:21:06Z

+
+import React, {useEffect, useState} from 'react';
+
+const BrowserOnly = ({children}) => {


This also works and is shorter IMO.

function BrowserOnly({children}) { return ExecutionEnvironment.canUseDOM && children != null && <>{children()}</>; };

Let's use function declaration for functional components.

Thanks, although this does not solve the issue of importing components using window.

For example, if I just import this component in the MDX file, then I will have an error during the building that the window is not defined.

import {Replay} from 'vimond-replay';

In that case, users will have to lazy load the component after the component has mounted. There's no way we can handle that with the suggested component here.

yangshun · 2020-03-25T17:51:03Z

@lex111 shall we try to get this into the next release?

…o feat-exclude-ssr

lex111 · 2020-03-28T23:23:11Z

@yangshun done ✔️

yangshun

I will fix this if you don't get to it by tonight so that we can include this in the next release.

yangshun · 2020-03-29T04:14:37Z

+import BrowserOnly from '@docusaurus/BrowserOnly';
+
+<BrowserOnly>
+  {/* Something that should be excluded during build process prerendering. */}


We need to give an example to show that children should be a render function and not normal children.

Ah yes, done ✔️

facebook#2323) * feat(v2): add a way to exclude components from SSR within JSX * Update docusaurus-core.md * Update and rename ClientOnly.js to BrowserOnly.js * improve docs * Improve naming * Simplify realization * Fix code style * Fix docs * Update docusaurus-core.md Co-authored-by: Yangshun Tay <tay.yang.shun@gmail.com>

feat(v2): add a way to exclude components from SSR within JSX

c561a33

lex111 added the pr: new feature This PR adds a new API or behavior. label Feb 26, 2020

lex111 requested a review from yangshun February 26, 2020 00:40

lex111 requested a review from wgao19 as a code owner February 26, 2020 00:40

facebook-github-bot added the CLA Signed Signed Facebook CLA label Feb 26, 2020

yangshun changed the title ~~feat(v2): add a way to exclude components from SSR within JSX~~ feat(v2): add a way to exclude components from server prerendering within JSX Feb 26, 2020

yangshun added 3 commits February 26, 2020 08:55

Update docusaurus-core.md

c43b40b

Update and rename ClientOnly.js to BrowserOnly.js

b0a42d0

improve docs

6794da6

yangshun reviewed Feb 26, 2020

View reviewed changes

yangshun changed the title ~~feat(v2): add a way to exclude components from server prerendering within JSX~~ feat(v2): add a way to exclude components from build-time prerendering within JSX Feb 26, 2020

yangshun changed the title ~~feat(v2): add a way to exclude components from build-time prerendering within JSX~~ feat(v2): add a way to exclude components from build-time prerendering Feb 26, 2020

Improve naming

109d4aa

yangshun suggested changes Feb 29, 2020

View reviewed changes

lex111 added 4 commits March 26, 2020 11:23

Simplify realization

bdccc70

Merge github.com:facebook/docusaurus into feat-exclude-ssr

717077f

Merge branch 'feat-exclude-ssr' of github.com:facebook/docusaurus int…

12f3047

…o feat-exclude-ssr

Fix code style

a774ca1

lex111 added this to the v2.0.0-alpha.49 milestone Mar 28, 2020

yangshun suggested changes Mar 29, 2020

View reviewed changes

lex111 and others added 2 commits March 29, 2020 12:07

Fix docs

5138078

Update docusaurus-core.md

63ee326

yangshun merged commit 8143af6 into master Mar 29, 2020

yangshun deleted the feat-exclude-ssr branch March 29, 2020 15:44

Tacktician mentioned this pull request Nov 19, 2020

Updated code reference tags in testrunner config doc saucelabs/sauce-docs#40

Merged

9 tasks


		import React, {useEffect, useState} from 'react';

		const BrowserOnly = ({children}) => {

Uh oh!

Conversation

lex111 commented Feb 26, 2020

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

Related PRs

Uh oh!

docusaurus-bot commented Feb 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yangshun Feb 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yangshun left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yangshun Feb 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yangshun commented Mar 25, 2020

Uh oh!

lex111 commented Mar 28, 2020

Uh oh!

yangshun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

docusaurus-bot commented Feb 26, 2020 •

edited

Loading

yangshun Feb 26, 2020 •

edited

Loading

yangshun left a comment •

edited

Loading

yangshun Feb 29, 2020 •

edited

Loading