search

zekchan / react-ssr-error-boundary

34 stars 14 forks source link

Support new Context API #2

Open overlookmotel opened 5 years ago

overlookmotel commented 5 years ago

Hi.

Would it be difficult to support the newer Context API released in React 16.3?

As I'm sure you're aware, this module solves one of the big problems standing in the way of implementing a "userland" version of Suspense which works on the server side. But, as I see it, the complications around Context are a bit of deal-breaker at present. The new Context API is used by React Router, for example.

I guess the ideal would be to automatically propagate context from all providers higher up the tree into the sub-tree passed to renderToStaticMarkup(). Ideally the component implementing an error boundary shouldn't need to be aware what providers are in use higher up the tree.

I have no idea whether React exposes current contexts in some private property which can be hooked into.

I'd be very happy to help with this, but sounds like you understand the internals of React much better than me, so any pointers to where to start would be much appreciated.

zekchan commented 5 years ago

Hello! Sorry for long wait, I had some troubles with github notifications 😅. Thank you for your PR! Monkey-patching react for SSR seems like a good idea.I think, it would be great if we could monkey-patch react a couple more times for remove shim function.

mnebuerquo commented 5 years ago

This is a clever solution, but it doesn't solve the problem reliably.

Shimming the React.createContext function gives us a way to enumerate all active contexts, which then allows us to consume and re-provide the values of all the contexts, which allows the context to exist inside the inner renderToStaticMarkup.

The new problem is that we can't count on the order of execution of the code from separate modules. The shim code must execute before anyone calls createContext. If you use react-redux, that module calls createContext when it's module loads. If you load both modules, you have to ensure that your shim executes before react-redux is loaded. This is a race condition at best, or some hacky compile order requirements at worst. Either way, it's something that doesn't work without some non-standard hoop jumping. Many of the components which use contexts create the context globally at module load time, which is the worst possible case for this solution.

overlookmotel commented 5 years ago

@mnebuerquo I don't know if you're referring to using this for rendering Suspense on the server side or not. But if you are, I've published a separate module for this purpose:

https://www.npmjs.com/package/react-async-ssr

That supports both the old and new context APIs + also .useContext() hook.

mnebuerquo commented 5 years ago

Suspense is an interesting module. I'm doing something similar with SSR, error boundaries, and lazy loading, but in a very large project with a custom framework on top of React. I've had to solve the same problems as you did in this repo, and I've hit the same difficulty with contexts.

The Hard Question for me is this one: How do you ensure that the shim function has executed before react-redux calls React.createContext?

overlookmotel commented 5 years ago

I don't use Redux so can't comment on that specifically.

I use a different shim in react-async-ssr - shim is applied on consumer, not provider, so it'll likely work fine . Have a look at the source code if you're interested. All I can say is there are a lot of tests on react-async-ssr covering all the cases I could think of, so I'm pretty confident it works.

By the way, I'm not a maintainer on react-ssr-error-boundary - I just raised this issue, but then went on and wrote my own module instead.

mnebuerquo commented 5 years ago

I'm not using the shim in my solution now. I've just exposed an addContext function which you have to call to allow your context to pass through the wrapper. It's not an automatic solution, but it works. This is the only part of the SSR error boundary that doesn't make me happy.

overlookmotel commented 5 years ago

@mnebuerquo Interesting.

Please do have a look at react-async-ssr if you have a chance. It think it covers your use case in terms of SSR rendering Suspense, and it is fully "automatic".

I'd be very happy to receive a PR adding support for error boundaries too if you found it simple to add the code you've already developed for this.