Add <ClientSideOnly /> component

[DrewML]

No description provided.

[DrewML]

onServerRender is optional for the cases where you really just don't want anything rendered

[DrewML]

This is a static method, rather than just an inline check in render, so that a 3rd party SSR implementor could override this logic if they want, and have it automagically applied to every usage of ClientSideOnly in a theme.

Other alternatives include:

1. The export from this module is a factory fn that takes 1 argument (isSSR function), and returns a ClientSideOnly component

2. Make people pass isSSR as a prop every time

I chose the static method option because it only requires overriding in 1 place, and I can't think of a good argument for a theme or app having multiple different implementations of isSSR logic

[zetlen]

The static method works fine. If we need to change it at build time, it's a small change, and it allows for a lot of runtime flexibility.

Of course, prototypal inheritance allows us to make it an instance method and still affect all instances at once...

ClientSideOnly.prototype.isSSR = () => true;

But I think that confuses most people, so your way is better.

[DrewML]

Yeah, can be in the proto chain. I tend to only put things on the prototype that rely on instance-specific context, and other related things end up as static properties. Of course, that's purely a (personal) stylistic thing.

[zetlen]

I do the same thing. Leave it static!

[zetlen]

If you wanna change anything, do, but I approve

[zetlen]

The static method works fine. If we need to change it at build time, it's a small change, and it allows for a lot of runtime flexibility.

Of course, prototypal inheritance allows us to make it an instance method and still affect all instances at once...

ClientSideOnly.prototype.isSSR = () => true;

But I think that confuses most people, so your way is better.

[zetlen]

I'm not the most voracious consumer of React components, so this is a stylistic issue that most people might not have. Paging @jimbo for a second opinion. But I think it might be confusing to take a function-as-prop for server content, but regular old children for client content. If this was an intentional choice to encourage writing client-only content rather than server-only content, I dig it...but I'm still not sure how readable it is.

What if maybe:

import OnlyRender from '..';

// [...]

test('Returns value from invoking serverSide prop in SSR mode', () => {
    jest.spyOn(OnlyRender, 'isSSR').mockImplementation(() => true);
    const wrapper = shallow(
        <OnlyRender
          serverSide={() => <div>Server Content</div>}
          clientSide={() => <span>Client Content</span>}
        />
    );
    expect(wrapper.text()).toEqual('Server Content');
    OnlyRender.isSSR.mockReset();
});

test('Returns default value (null) when serverSide prop is not provided', () => {
    jest.spyOn(OnlyRender, 'isSSR').mockImplementation(() => true);
    const wrapper = shallow(<OnlyRender clientSide={() => <div>Client Content</div>} />);
    expect(wrapper.text()).toEqual('');
    OnlyRender.isSSR.mockReset();
});

test('Returns value from invoking clientSide prop in non-SSR mode', () => {
    jest.spyOn(OnlyRender, 'isSSR').mockImplementation(() => false);
    const wrapper = shallow(
        <OnlyRender
          serverSide={() => <div>Server Content</div>}
          clientSide={() => <span>Client Content</span>}
        />
    );
    expect(wrapper.text()).toEqual('Client Content');
    OnlyRender.isSSR.mockReset();
});

Or what about just letting people specify server-only and client-only content independently?

import OnlyRender from '..';

// [...]

test('Renders children of ServerSide component only in SSR mode', () => {
    jest.spyOn(OnlyRender, 'isSSR').mockImplementation(() => true);
    // OnlyRender itself is a pass-thru for a convenient top-level node
    const wrapper = shallow(
        <OnlyRender>
          <OnlyRender.ServerSide><div>Server Content</div></OnlyRender.ServerSide>
          <OnlyRender.ClientSide><div>Client Content</div></OnlyRender.ClientSide>
        </OnlyRender>
    );
    expect(wrapper.text()).toEqual('Server Content');
    OnlyRender.isSSR.mockReset();
});

test('Renders children of ClientSide component only in non-SSR mode', () => {
    jest.spyOn(OnlyRender, 'isSSR').mockImplementation(() => false);
    const wrapper = shallow(
        <OnlyRender>
          <OnlyRender.ServerSide><div>Server Content</div></OnlyRender.ServerSide>
          <OnlyRender.ClientSide><div>Client Content</div></OnlyRender.ClientSide>
        </OnlyRender>
    );
    expect(wrapper.text()).toEqual('Client Content');
    OnlyRender.isSSR.mockReset();
});

[zetlen]

(I don't feel especially strongly about this, so if neither of those suggestions are compelling, feel free to brush them off--your API is just fine, and I'm only trying to think about the newbie.)

[DrewML]

tbh, the current API is the way it is just because it was the first idea that came to mind.

I'm not a huge fan of the OnlyRender.ClientSide/OnlyRender.ServerSide API, but I have no objections if it's preferred that we use render props for both instead of render prop/children.

I'll go with whichever one of the ideas @jimbo ends up preferring

[jimbo]

Good discussion. I think @DrewML has the right idea with the original implementation here. This is a pass-through component (render signature of props => props.children) with a single exception (isSSR: true). Idiomatic and easy to follow.

[zetlen]

10-4, sounds like your current API will be great.

[jimbo]

Two questions:

1. This component only accepts one prop, a function named onServerRender that is essentially a render function. React Router v4's Route component offers three different props for this purpose—component, render, and children—each suited for a different use case. If our component doesn't offer all three, are we at least offering the best one?

2. If I recall correctly, when the tree rendered by the client doesn't exactly match the tree rendered by the server, React throws an error. Are we going to encounter that error when using this component?

[jimbo]

Good discussion. I think @DrewML has the right idea with the original implementation here. This is a pass-through component (render signature of props => props.children) with a single exception (isSSR: true). Idiomatic and easy to follow.

[zetlen]

@jimbo To your first question, I think Router has a different use case. We wouldn't want a component option, for instance, because in the typical use case, we wouldn't want the dev to feel like their "context" has changed. They should be able to pass props down, rather than pass a constructor.

To the second question--I don't know! Maybe React has to blow the whole UI away rather than build a reconciliation.

[DrewML]

If I recall correctly, when the tree rendered by the client doesn't exactly match the tree rendered by the server, React throws an error. Are we going to encounter that error when using this component?

That's....a very good point that I hadn't considered. Some relevant reading:

• Document that you can't rely on React 16 SSR patching up differences facebook/react#10591
• Need a way to prevent component render on client side initial load facebook/react#8017

Abramov made the following comment that is worth calling out:

Thoughts?

[zetlen]

Thanks, Dan, that's a good idea. Seems like this requires passing a function-as-prop (or function-as-child?) which receives hasMounted as an argument...

[jimbo]

Abramov's example is correct, but isn't the best solution. I don't think local component state is the right place to maintain the isSSR flag that a component checks. Here's how that would look:

1. user hard-navigates to /
2. server renders Home.js (isMounted: false)
3. client renders Home.js (isMounted: false)
4. client re-renders Home.js (isMounted: true)
5. user soft-navigates to /foo
6. client renders Foo.js
7. user soft-navigates back to /
8. client renders Home.js (isMounted: false)
9. client re-renders Home.js (isMounted: true)

Only the first render of a user's session is done on the server. Every subsequent render is done by the client—even visiting new routes—so we once we're client-side, we shouldn't use the double-render flow. If the isMounted: true state is local to a component, though, we lose that state when it unmounts.

Since the server's render is a one-time global event, we should probably store the flag in global state—in the Redux store—and change it from false to true in our ReactDOM.render() callback. Maybe this ClientSideOnly component can be connected to Redux and read that flag? Here's how that flow would look:

1. user hard-navigates to /
2. server renders Home.js (clientHasRendered: false)
3. client renders Home.js (clientHasRendered: false)
4. client re-renders Home.js (clientHasRendered: true)
5. user soft-navigates to /foo
6. client renders Foo.js
7. user soft-navigates back to /
8. client renders Home.js (clientHasRendered: true)

What do you guys think? @zetlen @DrewML

[zetlen]

@jimbo has a good point that local state seems wasteful for a one-time global event. I wonder, though, if this can be a compile-time optimization...

components/foo.js

/** global ClientOnly */
import React from 'react'
// note that ClientOnly is not imported; this way it's easier to detect at
// build time by string

export default class Foo extends React.Component {
    render() {
        return (<ClientOnly onServerRender={<div>Server content</div>}>
          <span>Client content</span>
        </ClientOnly>)
    }
}

in a webpack plugin

compiler.parser.plugin('<ClientOnly', expr => {
  /** here or at some other hook, detect its presence */
})

at server component build time

1. remove the node
2. inline the onServerRender prop body if it exists:

build/server/components/foo.js

/** global ClientOnly */
import React from 'react'

export default class Foo extends React.Component {
    render() {
        return <div>Server content</div>
    }
}

at client component build time

1. insert a noop definition ClientOnly = p => p.children
2. remove the onServerRender prop

in bundle:

import React from 'react'
const ClientOnly = p => p.children

export default class Foo extends React.Component {
    render() {
        return <ClientOnly><span>Client content</span></ClientOnly>
    }
}

Potentially you could even detect if ClientOnly has just one child node and remove it entirely, inlining its contents.

[zetlen]

@DrewML @jimbo bump

[DrewML]

I'd prefer @jimbo's approach. I'm not a big fan of moving things to build time optimizations unless it actually provides some sort of DX or performance improvements (doesn't seem like that will).

[jimbo]

@zetlen That would be viable, but I'm reluctant to add another magic global (similar to a JSX pragma). I would prefer to do something more straightforward.

[JulienPradet]

Hi!
I may have a few feedbacks around this.

First about the current API, I feel like onServerRender feels like an event when it's actually just a different children. I feel like it's clearer when you strip the on and just say serverRender. But that's actually just a feeling.

However, the more broader thing is that this component focus on stripping things out of the server. We have a few use cases in our stack that only changes props, rather than the full implementation.
For instance, we don't want to load the number of items in the cart on the Server side.

So we just strip the graphql component which is supposed to load {cart: {quantity: 5}} by a static prop {cart: {quantity: 0}}.

For this reason, I feel like a more broader component is more useful. Something along those lines :

<IsServerSide>
  {isServerSide => {
    const Fetcher = isServerSide ? GraphQlFetcher : StaticFetcher
    return <Fetcher>{data => <ViewComponent data={data} />}</Fetcher>
  }}
</IsServerSide>

Actually it feels a bit weirder with renderProps and it works better for us with HOCs since we use them a lot for the data fetching part.

branchServerClient(
  withProps(props => ({
    cart: {quantity: 0 }
  })),
  graphql(CartQtyQuery)
)(ViewComponent)

But hah! Just to say that it's a tiny bit restrictive to phrase it as a ClientSideOnly component.
And also, obviously our use case is still doable with your current implementation.

[zetlen]

Thanks, @JulienPradet ! The reason we have to use render props instead of direct children is that without a render lambda, React always renders elements bottom-to-top. Therefore, both server and client content would attempt to fully render as vdom before the <ClientOnly> logic ran. We don't want to evaluate the server-side branch of the logic on the client-side or vice versa, since it may depend on something absent in that environment. So we need to use a render function somewhere, whether as a prop or as a child.

[JulienPradet]

Thank you for you answer @zetlen :)

I did use renderProps in my example. Even though they are within the children prop, it's still a renderProp and React would evaluate only the return statement of this lambda. If I use what's in the tests, it would look like :

<IsServerSide>
    {isServerSide =>
        isServerSide ? <div>Server Content</div> : <span>Client Content</span>
    }
</IsServerSide>;

// or

<IsServerSide
    render={isServerSide =>
        isServerSide ? <div>Server Content</div> : <span>Client Content</span>
    }
/>;

In case you were talking about the HOC I've shown, indeed, it's slowing down the initialization of the app (not the subsequent renders).

About using the the server-side logic on the client side, as @jimbo stated previously, it's mandatory if you want to have a correct hydration phase on the client. In our experience, the only thing that's safe here is to use client side logic that's not available on the server (window, DOM, refs, etc.). You can't really use things specific to the server environment without breaking the hydration (except for caching strategies).

[DrewML]

Too many concurrent things going on with higher priority, so haven't been able to give this much more attn.

Will revisit with further discussions about community contribution opportunities for optional SSR later on.