ctrlplusb / react-sizeme

Make your React Components aware of their width and height!
MIT License
1.95k stars 94 forks source link

Figure out the SSR story #7

Closed lyuzashi closed 8 years ago

lyuzashi commented 8 years ago

I have been attempting to implement support for SSR. So far, the only semi-working solution I could come up with involves avoiding calling createResizeDetector if the window object is undefined.

My fork is here https://github.com/lyuzashi/react-sizeme/commit/1242009b424f9c0ab54aa0ecd5e815a20fbbdff8

Doing so will prevent any errors, however components that are wrapped by SizeMe are not included in the server-side rendered HTML.

This appears to be due in part to the RenderWrapper, which is helpfully commented to be treaded on lightly.

By setting the initial state of both width and height in SizeAwareComponent to NaN, the wrapper is short circuited and the component renders without error on both the server and client.

This approach, however, does not pass the included tests – specifically any time the placeholder is expected to render before a size change occurs.

Other attempts at cracking this puzzle have caused checksum errors.

I am not entirely sure what the placeholder does. I'm fairly new to React but I get the feeling it's pretty important not to short-circuit it.

Any help would be greatly appreciated. I'd love to get SSR working with this component – it looks very handy! Sorry this is so detailed.

As a note, I chose to try NaN as the size for SSR, since it theoretically is a dimensionless environment but existing math code wont error out or evaluate in unexpected ways (due to casting), as it might with null. Not sure if that's the best idea, but it's what I came up with as a suggestion.

ctrlplusb commented 8 years ago

Hi @lyuzashi

I'm happy for us to try and figure out the best story here for execution within an SSR environment.

Firstly, it will be good to know your intentions in using SSR. Is it for time-to-render performance or SEO concerns (or other)? Secondly, how are you using the dimension data that react-sizeme provides to your component, e.g are you intending to use the data to manifest your component in different forms?

My greatest concern is that you may negate any performance benefits that SSR is providing due to re-renders when your components do eventually get mounted onto the DOM.

Do you have any ideas on what sort of behaviour you would like to have from react-sizeme for SSR? What sort of trade offs are you happy to make?


In regards to the current behaviour of react-sizeme. It renders a placeholder component so that we can attempt to get the dimensions that will be available to your component without doing an unnecessary amount of rendering. It would be wasteful to render a full component tree only to have it re-render due to logic around the dimension data that they receive.

lyuzashi commented 8 years ago

Hey @ctrlplusb

Thank you for your very detailed and thoughtful response.

My initial intention was to use SizeMe in tandem with Component Queries to superficially adjust component styling, such as position and display properties. Not exactly a different form, but yes, the component would appear differently.

I completely understand the concern around negating SSR benefits, and know that it is not possible to determine a components size on the server.

It occurred to me that the fork I made of react-sizeme does in fact render the placeholder on the server, but since it contains none of the final component content, much of the page ends up missing.

My intent with SSR is to provide a useable page in clients without JavaScript, especially search engines and other crawlers (and I am aware Google runs JS to some extent).

Since a placeholder has the huge value of avoiding re-render, I propose an option be added to allow skipping the placeholder in cases where it would be useful – such as what I am trying to achieve.

I have updated my fork (https://github.com/lyuzashi/react-sizeme/commit/accaa314f6a02acc3424cd80d79df7c115317e85) and written test cases for a new option rerender which will set the width and height to NaN when truthy. This then skips the placeholder.

In my case, the page is initially loaded in a generic state, and then slightly restyled to match the component dimensions.

Let me know your thoughts. Cheers.

ctrlplusb commented 8 years ago

Thanks for getting back to me with with a detailed response, as well as for taking the time to spike out a patch. This is super helpful to me.

+1 to the idea of being able to skip the Placeholder render for an SSR context.

What do you think about the idea of us making an internal usePlaceholder boolean flag that checks for the existance of window (i.e. running in browser/server). If window doesn't exist then the usePlaceholder flag is set to false and the WrappedComponent will be rendered directly.

Personally, I would prefer that we just send null as the width / height values in the SSR case. I feel like NaN may confuse some into thinking that a mathematical error may have occurred. If we document the fact that in SSR renders your component will be have null values for the dimensions, then it is up to implementors to decide what logic they would like to have within their component for this case.

I feel like the above doesn't require the exposing of the "internals" of SizeMe whilst making it a seamless auto-behaviour for those running in SSR. Additionally, any further components that get loaded after an SSR return will still get the performance benefit of the Placeholder behaviour.

What do you think?

lyuzashi commented 8 years ago

You're most welcome and thank you for your continued support of sizeMe and its further development.

My initial thoughts were to do just that – automatically skip placeholder when window doesn't exist. This is, however, where I ran into problems with checksum.

I've updated my fork (https://github.com/lyuzashi/react-sizeme/commit/6bf17009c1b551621dc2dbd23bd9f8f162a6b769) to use an internal usePlaceholder flag, which automatically renders the original component on the server.

Unfortunately, since the component's default initial state on a renderer with window is to wrap in the placeholder, React cannot match the DOM – triggering a "checksum was invalid" warning.

I've been investigating how to pass a flag to RenderWrapper which would indicate the component is going to attach to existing markup and skip the placeholder again, however I have not been able to find a way to determine this case.

Perhaps you've got some ideas as to how to achieve this?

I really like "convention over configuration" approaches, such as the automatic behaviour that you've suggested.

I agree with your justification to use null. My initial hesitation was that it gets cast to zero in most math operations which might lead to incorrect calculations, but documenting it as you suggest will avoid such issues.

ctrlplusb commented 8 years ago

👍

Great, thanks for this insight. I'll make some time this w/end to have a look at your fork and see if I can figure out something in regards to the placeholder checksum issue.

ctrlplusb commented 8 years ago

Hi @lyuzashi

I've created a branch in which I have tried out what we discussed.

https://github.com/ctrlplusb/react-sizeme/tree/issue_7

I have it running in an SSR environment, but am getting the checksum issue. Trying to figure out a solution...

lyuzashi commented 8 years ago

Excellent @ctrlplusb!

Your code is much neater than my attempt.

The lazy loading of resizeDetector (thanks #6) also avoids checking for window there, which is great!

I might be wrong, but I don't think React runs componentDidMount or componentDidUpdate on the server, so there shouldn't be a need to wrap them in if statements.

Let me know how things go with the checksum. I put some thought and experiment into it yesterday and couldn't crack it. Might have another look next week if you're still going.

It looked like I'd have to dive into the internals of React and how it mounts components into existing markup, but it's a bit beyond my understanding of the framework so far.

Cheers!

ctrlplusb commented 8 years ago

😀👍

Very good point! I've set up my test incorrectly then. I'll need to modify it to better represent an SSR environment.

I think I may have cracked the case but I am out for the rest of the day. Will probably do another commit for your review tomorrow.

ctrlplusb commented 8 years ago

Morning @lyuzashi

I have pushed another change. :)

There is now a global flag attached to the SizeMe function which allows you to opt in to the SSR behaviour. In the initialisation phase of your application you need to set it like so:

import SizeMe from 'react-sizeme';

SizeMe.enableSSRBehaviour = true; // default is false

This will then allow us to duplicate the behaviour on client/server and avoid any checksum issues.

Would you mind trying it out for me and letting me know how it works for you. I'm also interested in whether your chrome react dev tools etc works okay.

Thanks!

lyuzashi commented 8 years ago

Hey @ctrlplusb

I've just had a chance to look at this. The flag works just as expected in my test set up. Nice work!

No problems in any of my tools either.

I'd be very happy to see this option to be merged into master. Would be great to revisit enabling this flag automatically too. I might have another look later down the track but in the mean time setting enableSSRBehavior works a treat.

Thanks again for being so responsive and open. sizeMe is a brilliant component!

ctrlplusb commented 8 years ago

Thanks for your help with this @lyuzashi!

ctrlplusb commented 8 years ago

In regards to the automatic setting of the variable... I did have a think about this but it is really hard to do. We can easily check for an SSR environment whilst on the server, but then we need to send back some sort of state to the client that indicates that we did in fact run in SSR. Tricky.

If the React libs somehow expose this fact maybe we could leverage that?

lyuzashi commented 8 years ago

Was my pleasure @ctrlplusb! And thanks for the shoutout and the detailed docs you wrote up for the feature.

From my small exploration, I too, could see that it would be really difficult to automatically detect when to enable this behaviour. I didn't find anything in the React libs, but I'd agree that is probably how the state should be transferred.

Thanks again!!