SSR support

[jquense]

awesome! I'm really hoping we can reach a spot where we can hydrate strings from react-dom/server so we don't also need to write the streaming/toString code

[gaearon]

I'm really hoping we can reach a spot where we can hydrate strings from react-dom/server so we don't also need to write the streaming/toString code

That definitely sounds like how it should work. You shouldn’t need to reimplement the SSR.

[ManasJayanth]

Ready for review. I have a test repo here

[jquense]

gonna try and grab some time to take a look. Can you give a high-level overview of where this differs from the react-dom implementation make it a bit easier to focus on the right stuff?

[ManasJayanth]

I haven't studied react-dom rehydration entirely. For the most part it isn't very different when it comes to warning the user. diffHydrationProperties uses our Events.listenTo which I would say is the only major difference. I can take a look the react-dom once again tomorrow as its a little late here :)

[ManasJayanth]

I certainly understand that approach, however a downside is it can be harder to trim down to the essentials when you are starting with a lot of potential legacy and assumptions.

Hydration as I understood is

1. Validation of the client rendered markup wrt the server rendered one.
   2. Registering all event handlers

Are you saying that since [1] is a non essential, we keep it to the bare minimum? I thought ensuring the state of the client rendered tree is identical to the server meant running all validation checks that RD ran. Hence the all-or-nothing approach. Please correct me here if needed since I haven’t kept up with the React Core codebase in a while.

Also can you please point out exactly ‘legacy and assumptions’. I did not understand which parts you are referring to. I will be replying to the review comments individually, but incase there’s still something let’s discuss it over.

On a practical note, all the scattered DEV flags are confusing, it'd be great if we could split the code (as much as feasible) into the just the bits that are required to function and move everything else under one big flag, Like maybe have all the warnings in a separate file and then conditionally require it vs intermingling them. Sort of like how prop-types the package does it. As it is we'd still end up with a bunch of mostly empty function defs that could add some bytes we don't need

As I understand, All the DEV checks scattered all over I didn’t really understand your idea. Can you maybe help me out with an example? I really wish parts of hydration block in the reconciler host config itself were optional (again I wasn’t sure as I explained above) and if it were so then I can imagine implementing your idea if putting them behind one big flag. I’m obviously missing something here.

Definitions of the warning functions left over. I really wish a tool could help us out, unlike what react-do does. I wondering if this would work: place all the warning functions in a separate module, import statically, but call them only behind the DEV flag. Rollup will miss it, would GCC catch the unused require and DCE it?

[jquense]

I didn’t really understand your idea. Can you maybe help me out with an example?

Yeah sure! It's more of a thought but I was thinking, depending on how much overlap there is, but have a ProdSSR.js and DevSSR.js, that export the hydration reconciler and conditionally require them. The prod one would be essentially just the "attach event handlers" bit, and the other one would have all the warnings in it.

but call them only behind the DEV flag. Rollup will miss it, would GCC catch the unused require and DCE it?

I think this would work especially if you used require to import the whole bad of things vs import?

[ManasJayanth]

@jquense Updated the PR with changes we discussed. requireing the modules conditionally wasn't needed. Rollup DCE'd the unused module

[ManasJayanth]

@jquense Just a gentle reminder to review the PR :)