Fix Server Side rendering support

[marduke182]

This resolve issue #44.

I moved all the translateDOMPositionXY calls inside comoponentDidMount and componentDidUpdate.

The site dev server works and my server rendering don`t throw the warning.

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[pieterv]

Couple of thoughts on this approach:

• Mutating any object is generally bad but in this case we are mutating the DOM node after render which will result in us touching the DOM twice rather than once.
• The other thing is that it wont set any styles for the HTML that gets to the client so before the JS loads the rows will be out of place.

[pieterv]

Hey @marduke182, thanks for the PR. The way i was thinking about in the past to solve this was to have some state that tells us we are in the first pass and always render positions with left and top (since we know all browsers support this but not translate pos), then on any other render from there we can use the fancy translate3d stuff. See the inline message for why i think the current approach in this PR is bad. Do you think you would be able to update this to do that?

[marduke182]

Hi @pieterv, My first solution was use the state to store a flag variable, and in componentDidMount change the state to know that the component was mounted. But in the best practice says that never use setState inside componentDidMount. I update the pull request to show you my original solution.

Maybe the best way is use Hight Order Component to control the state of initialRender and pass by a property the argument.

Thanks for the feedback.

[pieterv]

Hmm i dont think this flag necessarily needs to live on state, since we don't actually want to force a render as a result of changing it, we just need a flag to signify that we are now out of the first render. The down side of this is it will make the render more non-pure, but it wont be to much worse than it already was (since it already relies on global state).

so:

componentDidMount() {
  this._initialRender = false;
}

Lets also create a wrapper around translateDOMPositionXY module that does this logic (translateDOMPositionXY is hard for us to change since its a core FB module), so if is this._initialRender then use left and top then after this we just delegate to translateDOMPositionXY.

Thanks for taking this on!

[marduke182]

I update the pull request with the final solution. All the modules affected have this change:

componentWillMount() {
  this._initialRender = true;
}

componentDidMount() {
  this._initialRender = false;
}

And wrap translateDOMPositionXY, like you said.

[tbrd]

I'm quite interested in this... any chance we can get it merged?

[djbobbydrake]

+1 - Second @tbrd - would love to get this merged

[AlesJiranek]

it would be great to have this pull request merged

[DenLilleMand]

👍

[HillLiu]

👍

[thomasdavis]

👍

[thomasdavis]

@marduke182 Thanks for the awesome pull requests, I've merged your fork into some of our code. Though it seems like there a whole bunch of other things that still don't work with SSR.

e.g. global cannot be found on the server -> https://github.com/facebook/fixed-data-table/blob/master/src/vendor_upstream/core/nativeRequestAnimationFrame.js

There a bunch of other small ones I've been band aiding. I wonder if it is time for a fork to really get SSR working?

[thomasdavis]

Also a good solution to making all rows initially appear for SEO purposes would be great.

[HillLiu]

@pieterv Do you have a plan merge this request?

[wcjordan]

@tbrd @HillLiu @DenLilleMand @djbobbydrake we've merged the PR for this into our fork at https://github.com/schrodinger/fixed-data-table-2 and should have the fix up on npm within a week.

@thomasdavis If you open an issue or PR with the SSR issues you're seeing, we'd appreciate your help getting it stable on our fork. Thanks!