Skip to content
This repository has been archived by the owner on Jan 22, 2020. It is now read-only.

Allow finer grain control of render properties #73

Closed
pklicnik opened this issue Aug 13, 2015 · 12 comments
Closed

Allow finer grain control of render properties #73

pklicnik opened this issue Aug 13, 2015 · 12 comments

Comments

@pklicnik
Copy link

When rendering on the server, react-engine constructs a object containing data for rendering based on "a mash of express render options, res.locals and meta info about react-engine" (quoted from the documentation in react-engine/lib/server.js). This is not ideal in all scenarios because this data is also sent to the client (via REACT_ENGINE variable) when there is no need.

We've specifically hit two scenarios:

  1. We are using third party middleware which attaches content to res.locals. That content is completely unnecessary for rendering. react-engine blindly copies this data and includes it for rendering on both server and client side. This is wasteful (increases the overall payload) and also leads to potential security issues.
  2. On the server, we fetch a HTML snippet and provide it as part of the render
var renderObj = {
    "top": encodeUriComponent("<div class='top'>My top section<div>")
};
res.render("template", renderObj);

Within our template, we then insert the raw HTML using the dangerouslySetInnerHTML API in React:

<div dangerouslySetInnerHTML={{__html: decodeURIComponent(this.props.top)}} />

This is a situation were we do want the content to be supplied during server side rendering, but it should not be sent as part of the payload to the client.

@dglozic
Copy link

dglozic commented Aug 13, 2015

Some of this has already been mentioned in the issue #50 (scenario 2, to be exact). We concluded that being able to configure what to omit would be a valuable addition.

@dglozic
Copy link

dglozic commented Aug 29, 2015

@samsel, are you hoping for a pull request for this or just deliberating :-)?

@samsel
Copy link
Contributor

samsel commented Sep 3, 2015

@pklicnik @dglozic

I have added renderOptionsKeysToFilter property to the server config spec.

renderOptionsKeysToFilter is an array of keys that need to be filtered out from the data object that gets fed into the react component for rendering. more info

Let me know if this will help.

#80

@dglozic
Copy link

dglozic commented Sep 3, 2015

Thanks, @samsel - we will try it out asap.

@samsel
Copy link
Contributor

samsel commented Sep 3, 2015

@dglozic changes are not merged/published yet. You can directly install from github to test the changes.

npm install git://github.com/samsel/react-engine.git#configurable_omit

@dglozic
Copy link

dglozic commented Sep 26, 2015

Hey, @samsel - I looked at 2.2.0 and unfortunately renderOptionsKeysToFilter is not doing what we needed. What it does is filtering data before passing to render, but that does not help. What I was looking for was a way to pass the data for rendering on the server, but not send it to the client.

Perhaps we need another array in options, something like 'clientKeysToFilter' whose role would be to omit keys from 'data' object before creating the script element on line 138 of server.js:

  // state (script) injection
  var script = format(TEMPLATE, Config.client.markupId, JSON.stringify(data));

@dglozic
Copy link

dglozic commented Oct 6, 2015

Bump.

@samsel
Copy link
Contributor

samsel commented Oct 6, 2015

@dglozic curious to know the use case behind having some data only for server rendering and not for client.

@dglozic
Copy link

dglozic commented Oct 6, 2015

We have a number of microservices that share some common areas (e.g. common header). In order not to make them stack-dependent, we just expose them as API that returns HTML so that any app can fetch the HTML snippet and inline it. In case of react, we just fetch the HTML in the controller, pass the snippet in as a string in the settings, the view renders it by by adding this:

and then the component adds the rest of the page. Since we will not need 'this.props.top' on the client, it is wasteful to send it, and this property can be long depending on the complexity of the snippet coming in. When you look at the source as rendered, a large chunk of it is encoded HTML snippet sitting inside the data sent by react-engine to the client. It bloats the HTML needlessly.

@samsel
Copy link
Contributor

samsel commented Oct 12, 2015

@dglozic can you please clarify what this.props.top is?

@dglozic
Copy link

dglozic commented Oct 17, 2015

@samsel the more I think about this, the more I realize that the real solution for our problem is not to selectively filter properties we are sending to the components on the server vs client. Doing so would cause checksum problems because React would complain that a server and client side rendering is not the same.

I think what we need to do is render the html/head/body elements on the server using another engine (say, dust.js), and then render the actualy body as a React component. On the server, we can render the react component in a string and just inline it in dust. On the client, we can mount the same react component on a DOM node other than 'document'. For this we need configurable mount node id in Client specs (and you already have an issue opened for this).

You can close this issue and we can track the rest in issue #68.

@samsel
Copy link
Contributor

samsel commented Oct 17, 2015

great thanks

@samsel samsel closed this as completed Oct 17, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants