Synchronously enhance component when running on the server

[mpeyper]

Q	A
Fixed Issues?	Fixes #473
Documentation only PR	👍
Patch: Bug Fix?	👍
Minor: New Feature?
Major: Breaking Change?
Tests Added + Pass?	No + Yes

As per #473, the recent changes to prevent a react warning about cross component updates broke SSR rendering. This change attempts to identify when the component is rendering one the server (typeof window === 'undefined') and synchronously enhances the component, while still only enhancing as an effect when rendering on the client.

What I don't know (and don't have an SSR environment to test in) is if there is a single render of null after hydration on the client. I don't think it will, but I'm not sure.

I also haven't written a test for this scenario as I'm not sure how remove window from the test environments for a since case. Happy to write it if anyone has any bright ideas on this?

[mpeyper]

@jlowcs you might want to take a look and try the changes out. I can provide you the built output if you want.

[jlowcs]

Sadly, looking at the code from my phone, I'm pretty sure that this will not work during hydration (during which window is defined, as hydration happens in the browser) as the hydrated component will not match the server-rendered component.

This is my working code:

const createDynamic = (identifier, enhancers, options) => {
    const { context = ReactReduxContext } = options
    const dynamicEnhancer = createDynamicTarget(enhancers)(identifier)
    return Component => {
        const createEnhancedComponent = (store) => {
            const res = () => dynamicEnhancer(store)(Component);
            res.store = store;
            return res;
        }
        const Dynamic = React.forwardRef((props, ref) => {
            const { store } = useContext(context)
            const firstAppRender = useIsFirstAppRender();
            const [EnhancedComponent, setEnhancedComponent] = useState(!firstAppRender ? null : createEnhancedComponent(store));
            useEffect(() => {
                if (!EnhancedComponent || store !== EnhancedComponent.store) {
                    setEnhancedComponent(createEnhancedComponent(store));
                }
            }, [store])
            return EnhancedComponent && <EnhancedComponent identifier={identifier} {...props} ref={ref} />
        })

        hoistNonReactStatics(Dynamic, Component)
        Dynamic.displayName = wrapDisplayName(Component, 'Dynamic')

        Dynamic.createInstance = (instanceIdentifier, ...instanceEnhancers) =>
            createDynamic(instanceIdentifier, enhancers.concat(instanceEnhancers), options)(Component)

        return Dynamic
    }
}

That useIsFirstAppRender() uses a context that is updated by a useEffect to determine if it is the first render (both for SSR and hydration), but that hook is very specific to our app.

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

import { AppRenderContext } from './context';

// eslint-disable-next-line @typescript-eslint/ban-types
export function AppRenderInfoProvider({ children }: PropsWithChildren<{}>): JSX.Element | null {
    const [firstRender, setFirstRender] = useState(true);

    useEffect(() => {
        setFirstRender(false);
    }, []);

    return <AppRenderContext.Provider value={{ firstRender }}>{children}</AppRenderContext.Provider>;
}

import { useContext } from 'react';

import { AppRenderContext } from './context';

export function useIsFirstAppRender(): boolean {
    const context = useContext(AppRenderContext);

    if (!context) {
        return false;
    }

    return context.firstRender;
}

I'm thinking we could wrap the Store provider to add that logic. Other than that, I don't have any ideas.

[jlowcs]

As for testing, you should be able to switch to a node environment through a comment in your test file, but I haven't tried rendering react components in a node environment yet: https://jestjs.io/docs/en/configuration#testenvironment-string

Usually, the way I approach testing the absence of window is by importing a getWindow() function which I can then mock in my test.

[mpeyper]

How does this look?

const DynamicSSRContext = React.createContext(false)

// eslint-disable-next-line react/prop-types
export const DynamicSSR = ({ children }) => {
  const [firstRender, setFirstRender] = useState(true);

  useEffect(() => {
    setFirstRender(false);
  }, []);

  return <DynamicSSRContext.Provider value={firstRender}>{children}</DynamicSSRContext.Provider>
}

const createDynamic = (identifier, enhancers, options) => {
  const { context = ReactReduxContext } = options
  const dynamicEnhancer = createDynamicTarget(enhancers)(identifier)
  return Component => {
    const Dynamic = React.forwardRef((props, ref) => {
      const { store } = useContext(context)
      const [lastStore, setLastStore] = useState(store)
      const firstRender = useContext(DynamicSSRContext)

      const [EnhancedComponent, setEnhancedComponent] = useState(() => firstRender ? dynamicEnhancer(store)(Component) : null);
      useEffect(() => {
        if (!EnhancedComponent || store !== lastStore) {
          setEnhancedComponent(() => dynamicEnhancer(store)(Component));
          setLastStore(store)
        }
      }, [store])

      return EnhancedComponent && <EnhancedComponent identifier={identifier} {...props} ref={ref} />
    })

    hoistNonReactStatics(Dynamic, Component)
    Dynamic.displayName = wrapDisplayName(Component, 'Dynamic')

    Dynamic.createInstance = (instanceIdentifier, ...instanceEnhancers) =>
      createDynamic(instanceIdentifier, enhancers.concat(instanceEnhancers), options)(Component)

    return Dynamic
  }
}

You would add DynamicSSR higher in the tree (like right under your Provider) if you are doing SSR, otherwise users not doing SSR can just leave it out (it works with or without it for them). Technically having it is producing a slightly more efficient initial render if there are nested dynamic components that render in the first pass, so I'm wondering if we just call it DynamicProvider or something generic like that?

I'll be honest though, I'm not sure if this code is doing anything fundamentally wrong or breaking some rules when we got to a concurrent rendering world with react and I feel like I don't know enough to tell if this is a good idea or not.

Edit: changed the implementation a bit.

[jlowcs]

I'll be honest though, I'm not sure if this code is doing anything fundamentally wrong or breaking some rules when we got to a concurrent rendering world with react and I feel like I don't know enough to tell if this is a good idea or not.

Same here tbh. Maybe we could get some insight from someone else?

Otherwise, code looks like it would do the job. I'm happy to test it as a part as the lib if you provide a build :)

[jlowcs]

I'm wondering if we just call it DynamicProvider or something generic like that?

I think that may be better, as DynamicSSR might give the impression that it should only be used when rendering on the server, and not also on the client for hydration.

You could document it this way: "while not required for client-side rendering (CSR), <DynamicProvider> must be used as high as possible in the React tree to allow for server-side rendering (SSR). It will also slightly improve first-pass CSR rendering performance.`

[mpeyper]

@jlowcs I'm thinking we push forward with this solution. Naming is TBD, but I'm having an issue with testing it. I was hoping you might know more about SSR and could help me out...

test('ssr example', () => {
  const TestComponent = () => <p>Hello World</p>

  const output = ReactDomServer.renderToString(<TestComponent />)

  const container = document.createElement('div')
  container.innerHtml = output

  ReactDom.hydrate(<TestComponent />, container)

  expect(container.innerHTML).toBe('<p>Hello World</p>')
})

This test passes, however, it is also producing a warning in the console that makes me wonder if the way I'm either rendering or hydrating is actually correct:

    console.error
      Warning: Expected server HTML to contain a matching <p> in <div>.
          in p (created by TestComponent)
          in TestComponent

      at printWarning (../../node_modules/react-dom/cjs/react-dom.development.js:88:30)
      at error (../../node_modules/react-dom/cjs/react-dom.development.js:60:5)
      at warnForInsertedHydratedElement (../../node_modules/react-dom/cjs/react-dom.development.js:6603:5)
      at didNotFindHydratableContainerInstance (../../node_modules/react-dom/cjs/react-dom.development.js:7793:5)
      at insertNonHydratedInstance (../../node_modules/react-dom/cjs/react-dom.development.js:16482:15)
      at tryToClaimNextHydratableInstance (../../node_modules/react-dom/cjs/react-dom.development.js:16575:5)
      at updateHostComponent (../../node_modules/react-dom/cjs/react-dom.development.js:17269:5)
      at beginWork (../../node_modules/react-dom/cjs/react-dom.development.js:18627:14)

Note: this is not the test I'm writing for this, just me trying to remove variables to work out why the warning is occurring.

[jlowcs]

It's nighttime in my part of the world but I'll try and find some time to investigate it tomorrow.

The error seems to indicate that the hydration result does not match the server-rendered html.

That could maybe come from the fact that hydrate is asynchronous? It takes a callback as a 3rd parameter: https://reactjs.org/docs/react-dom.html#hydrate. If the hydration actually runs after the dom was cleaned, it might have trouble finding the <p> which could explain the error message.

[mpeyper]

DOH!

- container.innerHtml = output
+ container.innerHTML = output

Nevermind 😓

[jlowcs]

Haha, TypeScript would have helped with that!

[mpeyper]

@jlowcs I've published an alpha build for you to try. Please update all your @redux-dynostore dependencies to ^3.2.0-alpha.0 and let me know how it goes.

[jlowcs]

@mpeyper I can confirm that it's working fine with our app!

[mpeyper]

@jlowcs Would you mind giving another alpha build a quick test before I put this out for real? 3.2.0-alpha.1

[jlowcs]

@mpeyper still working fine on my end :)

[mpeyper]

alright, let's do it.

[mpeyper]

@jlowcs should be good to go in v3.2.0

[jlowcs]

Thanks for being so quick with this!