Return markup hash form react component helper #800

udovenko · 2017-04-10T05:05:17Z

Implement handling of Hash result of server_rendered_react_component_html in react_on_rails helper
Implement additional example page for server rendered HTML that shows usage of htmlResult object
Add conditional title to dummy app layout
Implement rendering title string in generator function on server
Add test for conditional title to integration_spec

This change is

justin808 · 2017-04-10T09:10:26Z

@udovenko AWESOME start.

Besides the comments below, we need:

CHANGELOG.md entry
/docs/additional-reading/react-helmut.md
Update to the README

Reviewed 14 of 14 files at r1.
Review status: all files reviewed at latest revision, 11 unresolved discussions, some commit checks failed.

app/helpers/react_on_rails_helper.rb, line 121 at r1 (raw file):

    if server_rendered_html.is_a?(String)
      build_react_component_result_for_server_rendered_string(

Let's create a Hash of params and pass this to both methods. For more than 2 params, I prefer named params, or in our case, a hash of params.

app/helpers/react_on_rails_helper.rb, line 230 at r1 (raw file):

    result = compose_react_component_html_with_spec_and_console(
      component_specification_tag, rendered_output, options.replay_console ? console_script : "")

named params
Don't use a ternary as a param

app/helpers/react_on_rails_helper.rb, line 243 at r1 (raw file):

    # We expect uncapitalized component_name to be a key for rendered component HTML string:
    uncapitalized_component_name = component_name.clone
    uncapitalized_component_name[0] = uncapitalized_component_name[0].downcase

Why only the first character as downcased?

Note, downcase copies: https://ruby-doc.org/core-2.1.0/String.html#method-i-downcase while downcase! mutates.

I'd suggest we consider using the same key in the hash to mean the component, rather than the accessory information.

app/helpers/react_on_rails_helper.rb, line 245 at r1 (raw file):

    uncapitalized_component_name[0] = uncapitalized_component_name[0].downcase
    unless server_rendered_html[uncapitalized_component_name]
      fail "server_rendered_html expected to contain uncapitalized component_name."

display the expected component name in the error message

app/helpers/react_on_rails_helper.rb, line 261 at r1 (raw file):

prepend_render_rails_context(result)
create a local variable for this

docs/api/javascript-api.md, line 11 at r1 (raw file):

or you may use a

"generator function" to return a React component or an object with the following shape:

{ renderedHtml, redirectLocation, error }.

or, in the case of server rendering, you may use a "generator function" to return a React component or an object with the following shape:
{ renderedHtml, redirectLocation, error }.

See the example in /docs/additional-reading/react-helmut.md

docs/api/javascript-api.md, line 12 at r1 (raw file):

   * to a Ruby Hash. This is useful if you want to use something like React Helmet to return
   * additional values when rendering. In this case rendered component HTML **must** be returned
   * under key matching uncapitalized component name.

switch to componentHtml

In this case rendered component HTML must be returned
under the key componentHtml

spec/dummy/app/views/pages/rendered_htmls.erb, line 9 at r1 (raw file):

<% end %>

<%= render_hash["renderedHtmls"] %>

I really don't like having the component name be RenderedHtmls and the hash key being renderedHtmls.

<%= render_hash["componentHtml"] %>

CC: @alexfedoseev @robwise

spec/dummy/client/app/startup/clientRegistration.jsx, line 49 at r1 (raw file):

  CacheDisabled,
  RenderedHtml,
  RenderedHtmls,

I think we need to change the name from RenderedHtmls to ReactHelmutApp so it's clear that it's showing React Helmut.

spec/dummy/client/app/startup/ClientRenderedHtmls.jsx, line 9 at r1 (raw file):

 *  This is used for the client rendering hook after the page html is rendered.
 *  React will see that the state is the same and not do anything.
 *  Note, this is imported as "HelloWorldApp" by "clientRegistration.jsx"

please fix up, trim up comments to reflect this example

spec/dummy/client/app/startup/ServerRenderedHtmls.jsx, line 24 at r1 (raw file):

const renderedHtml = {
renderedHtmls: componentHtml,
title: helmet.title.toString(),
};

  const renderedHtml = {
    componentHtml,
    title: helmet.title.toString(),
  };

Comments from Reviewable

udovenko · 2017-04-10T23:50:54Z

Review status: all files reviewed at latest revision, 11 unresolved discussions, some commit checks failed.

app/helpers/react_on_rails_helper.rb, line 121 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

Let's create a Hash of params and pass this to both methods. For more than 2 params, I prefer named params, or in our case, a hash of params.

Yes, that's reasonable.

app/helpers/react_on_rails_helper.rb, line 230 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

named params

Don't use a ternary as a param

Will fix...

app/helpers/react_on_rails_helper.rb, line 243 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

Why only the first character as downcased?

Note, downcase copies: https://ruby-doc.org/core-2.1.0/String.html#method-i-downcase while downcase! mutates.

I'd suggest we consider using the same key in the hash to mean the component, rather than the accessory information.

So we will access the component by the same key as used for its registration?

app/helpers/react_on_rails_helper.rb, line 245 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

display the expected component name in the error message

Will do...

app/helpers/react_on_rails_helper.rb, line 261 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

prepend_render_rails_context(result)
create a local variable for this

ok

docs/api/javascript-api.md, line 11 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

or you may use a

"generator function" to return a React component or an object with the following shape:

{ renderedHtml, redirectLocation, error }.

or, in the case of server rendering, you may use a "generator function" to return a React component or an object with the following shape:
{ renderedHtml, redirectLocation, error }.

See the example in /docs/additional-reading/react-helmut.md

See comment above...

docs/api/javascript-api.md, line 12 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

switch to componentHtml

In this case rendered component HTML must be returned
under the key componentHtml

In this case rendered component must be under the key that matches its registration name (that is passed as component_name to react_component helper) inside componentHtml object. Suppose we registered component as App:

return {
   componentHtml: {
       App: reactDomServer.renderToString(...),
       additionalMarkup: blabla,
   }
}

spec/dummy/app/views/pages/rendered_htmls.erb, line 9 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

I really don't like having the component name be RenderedHtmls and the hash key being renderedHtmls.
<%= render_hash["componentHtml"] %>
CC: @alexfedoseev @robwise

The name of the hash key here must be the same as the key for component registration. I can register it as "ReactHelmetApp" then...

spec/dummy/client/app/startup/clientRegistration.jsx, line 49 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

I think we need to change the name from RenderedHtmls to ReactHelmutApp so it's clear that it's showing React Helmut.

see above...

Comments from Reviewable

udovenko · 2017-04-10T23:51:34Z

react-hElmet...

Review status: all files reviewed at latest revision, 11 unresolved discussions, some commit checks failed.

Comments from Reviewable

udovenko · 2017-04-11T04:25:27Z

Review status: 5 of 14 files reviewed at latest revision, 11 unresolved discussions, some commit checks failed.

app/helpers/react_on_rails_helper.rb, line 121 at r1 (raw file):

Previously, udovenko (Denis Udovenko) wrote…

Yes, that's reasonable.

Done.

app/helpers/react_on_rails_helper.rb, line 230 at r1 (raw file):

Previously, udovenko (Denis Udovenko) wrote…

Will fix...

Done.

app/helpers/react_on_rails_helper.rb, line 243 at r1 (raw file):

Previously, udovenko (Denis Udovenko) wrote…

So we will access the component by the same key as used for its registration?

Done.

app/helpers/react_on_rails_helper.rb, line 245 at r1 (raw file):

Previously, udovenko (Denis Udovenko) wrote…

Will do...

Done.

spec/dummy/client/app/startup/ReactHelmetClientApp.jsx, line 9 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

please fix up, trim up comments to reflect this example

Done.

spec/dummy/client/app/startup/ReactHelmetServerApp.jsx, line 24 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

const renderedHtml = {
renderedHtmls: componentHtml,
title: helmet.title.toString(),
};
  const renderedHtml = {
    componentHtml,
    title: helmet.title.toString(),
  };

Done.

Comments from Reviewable

udovenko · 2017-04-11T05:45:44Z

Review status: 4 of 16 files reviewed at latest revision, 11 unresolved discussions, some commit checks failed.

app/helpers/react_on_rails_helper.rb, line 261 at r1 (raw file):

Previously, udovenko (Denis Udovenko) wrote…

ok

Done.

docs/api/javascript-api.md, line 11 at r1 (raw file):

Previously, udovenko (Denis Udovenko) wrote…

See comment above...

Done.

docs/api/javascript-api.md, line 12 at r1 (raw file):

Previously, udovenko (Denis Udovenko) wrote…

In this case rendered component must be under the key that matches its registration name (that is passed as component_name to react_component helper) inside componentHtml object. Suppose we registered component as App:
return {
   componentHtml: {
       App: reactDomServer.renderToString(...),
       additionalMarkup: blabla,
   }
}

Done.

spec/dummy/app/views/pages/react_helmet.erb, line 9 at r1 (raw file):

Previously, udovenko (Denis Udovenko) wrote…

The name of the hash key here must be the same as the key for component registration. I can register it as "ReactHelmetApp" then...

Done.

spec/dummy/client/app/startup/clientRegistration.jsx, line 49 at r1 (raw file):

Previously, udovenko (Denis Udovenko) wrote…

see above...

Done.

spec/dummy/client/app/startup/ReactHelmetClientApp.jsx, line 9 at r1 (raw file):

Previously, udovenko (Denis Udovenko) wrote…

Done.

Done.

Comments from Reviewable

udovenko · 2017-04-11T06:24:16Z

@justin808 I think I've fixed all your review notices. Hope I didn't miss anything...

justin808 · 2017-04-11T08:28:41Z

@udovenko The main change is to fix the return keys. See below.

I'll try to get feedback from @robwise and @alexfedoseev on the API.

Reviewed 18 of 18 files at r2.
Review status: all files reviewed at latest revision, 22 unresolved discussions, some commit checks failed.

CHANGELOG.md, line 11 at r2 (raw file):

### Added
- Add possibility to return multiple HTML strings in a ```Hash``` as a result of ```react_component``` method call. Allows to build ```<head>``` contents with [React Helmet](https://github.com/nfl/react-helmet).
[#800](https://github.com/shakacode/react_on_rails/pull/800) by [udovenko](https://github.com/udovenko).

I think you wanted to use a single back tick, rather than three.

README.md, line 342 at r2 (raw file):

If you need to return multiple HTML strings from generator function and receive a Hash as a result of react_component helper call, you can put an object under renderedHtml key. This is useful when you using side effects libraries like react helmet.

For server rendering, if you wish to return multiple HTML strings from a generator function, such when using nfl/react-helmut, you may return an Object from your generator function with a single top level property of renderedHtml. Inside this Object, place a key called componentHtml, along with any other needed keys . This is useful when you using side effects libraries like react helmet. Your Ruby code with get this Object as a Hash containing keys componentHtml and any other custom keys that you added.

{ renderedHtml: { componentHtml, customKey1, customKey2} }

app/helpers/react_on_rails_helper.rb, line 227 at r2 (raw file):

  def build_react_component_result_for_server_rendered_string(params)
    unless params[:server_rendered_html] && params[:component_specification_tag] &&

Please assign the params to local variables. It's a bummer that we can't use named parameters due to a desire to maintain Ruby 2.0 compat.

app/helpers/react_on_rails_helper.rb, line 258 at r2 (raw file):

    # We expect component_name to be a key for rendered component HTML string:
    unless params[:server_rendered_html][params[:component_name]]

I thought we're switching to a constant of {renderedHtml: {componetHtml, other1, other2}}

app/helpers/react_on_rails_helper.rb, line 259 at r2 (raw file):

    # We expect component_name to be a key for rendered component HTML string:
    unless params[:server_rendered_html][params[:component_name]]
      fail "server_rendered_html expected to contain component_name #{params[:component_name]}."

I think it's awkward that the code inside of the the generator function needs to know it's name to work. If the name changes, this will be fragile.

I'd rather see that we fix the shape of the return Object: {renderedHtml: {componetHtml, other1, other2}}

@alexfedoseev @robwise, any opinions on this?

docs/additional-reading/react-helmet.md, line 1 at r2 (raw file):

# Using React Helmet to build ```<head>``` content

single backtick vs triple?

docs/additional-reading/react-helmet.md, line 13 at r2 (raw file):

```javascript
export default (props, _railsContext) => {
  const YourAppRegistrationKey = renderToString(<App {...props} />);

YourAppRegistrationKey does not sound right as the var name.

please use componentHtml

 const componentHtml = renderToString(<App {...props} />);
 const helmet = Helmet.renderStatic();
 const renderedHtml = {
    componentHtml,
    title: helmet.title.toString(),
  };

docs/additional-reading/react-helmet.md, line 74 at r2 (raw file):

Quoted 7 lines of code…

> ```ruby > <% render_hash = react_component("ReactHelmetApp", prerender: true, props: { hello: "world" }, trace: true) %> > > <% content_for :title do %> > <%= render_hash['title'] %> > <% end %> > > <%= render_hash["ReactHelmetApp"] %> ```

@alexfedoseev @robwise How about this:

<% react_helmut_app = react_component("ReactHelmetApp", prerender: true, props: { hello: "world" }, trace: true) %>
 
<% content_for :title do %>
  <%= react_helmut_app['title'] %>
<% end %>
 
<%= react_helmut_app["componentHtml"] %>

docs/api/javascript-api.md, line 12 at r2 (raw file):

   * On the server renderedHtml may be an Object, which converts to a Ruby Hash. This is useful
   * if you want to use something like ReactHelmet to return additional values when rendering.
   * In this case rendered component HTML must be returned under the same key as used for

Let's modify the doc to reflect this (no markdown, naturally).

For server rendering, if you wish to return multiple HTML strings from a generator function, such when using nfl/react-helmut, you may return an Object from your generator function with a single top level property of renderedHtml. Inside this Object, place a key called componentHtml, along with any other needed keys . This is useful when you using side effects libraries like react helmet. Your Ruby code with get this Object as a Hash containing keys componentHtml and any other custom keys that you added.

{ renderedHtml: { componentHtml, customKey1, customKey2} }

node_package/tests/ReactOnRails.test.js, line 138 at r2 (raw file):

  }

  ReactOnRails.setStore('storeGenerator', storeGenerator({}));

Why did this change? Was this an old bug?

spec/dummy/app/views/pages/react_helmet.erb, line 9 at r2 (raw file):

<% end %>

<%= render_hash["ReactHelmetApp"] %>

see example from docs above.

Comments from Reviewable

udovenko · 2017-04-12T01:35:20Z

Review status: 13 of 20 files reviewed at latest revision, 22 unresolved discussions, some commit checks failed.

CHANGELOG.md, line 11 at r2 (raw file):

Previously, justin808 (Justin Gordon) wrote…

I think you wanted to use a single back tick, rather than three.

Done.

README.md, line 342 at r2 (raw file):

Previously, justin808 (Justin Gordon) wrote…

If you need to return multiple HTML strings from generator function and receive a Hash as a result of react_component helper call, you can put an object under renderedHtml key. This is useful when you using side effects libraries like react helmet.

For server rendering, if you wish to return multiple HTML strings from a generator function, such when using nfl/react-helmut, you may return an Object from your generator function with a single top level property of renderedHtml. Inside this Object, place a key called componentHtml, along with any other needed keys . This is useful when you using side effects libraries like react helmet. Your Ruby code with get this Object as a Hash containing keys componentHtml and any other custom keys that you added.

{ renderedHtml: { componentHtml, customKey1, customKey2} }

Done.

app/helpers/react_on_rails_helper.rb, line 227 at r2 (raw file):

Previously, justin808 (Justin Gordon) wrote…

Please assign the params to local variables. It's a bummer that we can't use named parameters due to a desire to maintain Ruby 2.0 compat.

Done.

app/helpers/react_on_rails_helper.rb, line 258 at r2 (raw file):

Previously, justin808 (Justin Gordon) wrote…

I thought we're switching to a constant of {renderedHtml: {componetHtml, other1, other2}}

Done.

app/helpers/react_on_rails_helper.rb, line 259 at r2 (raw file):

Previously, justin808 (Justin Gordon) wrote…

I think it's awkward that the code inside of the the generator function needs to know it's name to work. If the name changes, this will be fragile.

I'd rather see that we fix the shape of the return Object: {renderedHtml: {componetHtml, other1, other2}}

@alexfedoseev @robwise, any opinions on this?

Done.

docs/additional-reading/react-helmet.md, line 1 at r2 (raw file):

Previously, justin808 (Justin Gordon) wrote…

single backtick vs triple?

Done.

docs/additional-reading/react-helmet.md, line 13 at r2 (raw file):

Previously, justin808 (Justin Gordon) wrote…

YourAppRegistrationKey does not sound right as the var name.

please use componentHtml

 const componentHtml = renderToString(<App {...props} />);
 const helmet = Helmet.renderStatic();
 const renderedHtml = {
    componentHtml,
    title: helmet.title.toString(),
  };

Done.

docs/additional-reading/react-helmet.md, line 74 at r2 (raw file):

Previously, justin808 (Justin Gordon) wrote…

<% render_hash = react_component("ReactHelmetApp", prerender: true, props: { hello: "world" }, trace: true) %>
 
<% content_for :title do %>
  <%= render_hash['title'] %>
<% end %>
 
<%= render_hash["ReactHelmetApp"] %>

@alexfedoseev @robwise How about this:

<% react_helmut_app = react_component("ReactHelmetApp", prerender: true, props: { hello: "world" }, trace: true) %>
 
<% content_for :title do %>
  <%= react_helmut_app['title'] %>
<% end %>
 
<%= react_helmut_app["componentHtml"] %>

Done.

docs/api/javascript-api.md, line 12 at r2 (raw file):

Previously, justin808 (Justin Gordon) wrote…

Let's modify the doc to reflect this (no markdown, naturally).

For server rendering, if you wish to return multiple HTML strings from a generator function, such when using nfl/react-helmut, you may return an Object from your generator function with a single top level property of renderedHtml. Inside this Object, place a key called componentHtml, along with any other needed keys . This is useful when you using side effects libraries like react helmet. Your Ruby code with get this Object as a Hash containing keys componentHtml and any other custom keys that you added.

{ renderedHtml: { componentHtml, customKey1, customKey2} }

Done.

node_package/tests/ReactOnRails.test.js, line 138 at r2 (raw file):

Previously, justin808 (Justin Gordon) wrote…

Why did this change? Was this an old bug?

This bug came from my previous PR. setStore method expects store instance, not store generator function. This change does not affect test results but I think this should be fixed to avoid misleading for other contributors.

spec/dummy/app/views/pages/react_helmet.erb, line 9 at r2 (raw file):

Previously, justin808 (Justin Gordon) wrote…

see example from docs above.

Done.

Comments from Reviewable

udovenko · 2017-04-12T01:40:09Z

@justin808 It's done I think.

justin808 · 2017-04-12T06:31:38Z

see comments below

Reviewed 2 of 18 files at r2, 7 of 8 files at r3.
Review status: all files reviewed at latest revision, 14 unresolved discussions, some commit checks failed.

app/helpers/react_on_rails_helper.rb, line 228 at r4 (raw file):

    # Destructure params to local variables:
    server_rendered_html, component_specification_tag, console_script, options =
      params.values_at(:server_rendered_html, :component_specification_tag, :console_script, :options)

nifty! @robwise not as good as ES6 destructuring!

HOWEVER:

https://robots.thoughtbot.com/ruby-2-keyword-arguments

Ruby 2.0 introduced keyword arguments!

Take a look at this:
https://github.com/shakacode/react_on_rails/pull/651/files

Only very old Ruby < 2 required hash for params! I'm glad I caught that.

app/helpers/react_on_rails_helper.rb, line 232 at r4 (raw file):

    unless server_rendered_html && component_specification_tag && console_script && options
      fail "At least one of params :server_rendered_html, :component_specification_tag, :console_script or :options " \
           "is missing."

see that PR 651 and see how required params are done

app/helpers/react_on_rails_helper.rb, line 261 at r4 (raw file):

    content_tag_options[:id] = options.dom_id

    component_html_key = "componentHtml"

This should be declared as a Ruby const at the top of this file, all caps

app/helpers/react_on_rails_helper.rb, line 263 at r4 (raw file):

unless params[:server_rendered_html][component_html_key]

unless params[:server_rendered_html][COMPONENT_HTML_KEY]

Comments from Reviewable

udovenko · 2017-04-12T07:18:10Z

Review status: 18 of 19 files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

app/helpers/react_on_rails_helper.rb, line 228 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

nifty! @robwise not as good as ES6 destructuring!

HOWEVER:

https://robots.thoughtbot.com/ruby-2-keyword-arguments

Ruby 2.0 introduced keyword arguments!

Take a look at this:
https://github.com/shakacode/react_on_rails/pull/651/files

Only very old Ruby < 2 required hash for params! I'm glad I caught that.

Done.

app/helpers/react_on_rails_helper.rb, line 232 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

see that PR 651 and see how required params are done

Done.

app/helpers/react_on_rails_helper.rb, line 261 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

This should be declared as a Ruby const at the top of this file, all caps

Done.

app/helpers/react_on_rails_helper.rb, line 263 at r4 (raw file):

Previously, justin808 (Justin Gordon) wrote…

unless params[:server_rendered_html][component_html_key]
unless params[:server_rendered_html][COMPONENT_HTML_KEY]

Done.

Comments from Reviewable

udovenko · 2017-04-12T07:19:08Z

@justin808 Done...

justin808 · 2017-04-12T08:59:02Z

Great job @udovenko!

Reviewed 1 of 1 files at r6.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

Comments from Reviewable

justin808 · 2017-04-13T08:38:47Z

Reviewed 4 of 4 files at r7.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

Comments from Reviewable

…on on the server * Implement handling of Hash result of server_rendered_react_component_html in react_on_rails helper * Implement additional example page for server rendered HTML that shows usage of htmlResult object

* Add conditional title to dummy app layout * Implement rendering title string in generator function on server * Add test for conditional title to integration_spec * Refactor code in react_on_rails helper

* Now test page yields title rendered by ReactHelmet * Add tests with disabled JS to integration spec to ensure correct title rendering on server * Mark HTML strings other than component HTML (composed with props and console script) as html_safe

…registration key

* Renamed example page to react_helmet * Changed route to example page * Renamed component registration to ReactHelmetApp * Additional refactoring

* Create additional-reading/react-helmet.mb * Fix comments for generator function * Update javascript-api.md

* Update README.md * Update CHANGELOG.md

…_... methods

…ponentHtml * Implement constant key usage in #build_react_component_result_for_server_rendered_hash method of react_on_rails helper * Updated generator function for server rendering * Additional refactoring

* Fixed additional-reading/react-helmet.md * Updated react_helmet.erb template accordingly

* Implement required params for #build_react_component_result_for... methods * Moved component_html_key to the constant

* Update react-helmet to v5.0.3 to resolve PropTypes warnings * Fixed rubocop warnings

coveralls · 2017-04-13T09:35:41Z

Coverage increased (+0.06%) to 97.977% when pulling 301adb2 on udovenko:return_markup_hash_form_react_component_helper into 2bef1d5 on shakacode:master.

justin808 · 2017-04-13T09:41:12Z

Reviewed 4 of 4 files at r8.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

spec/dummy/client/package.json, line 36 at r8 (raw file):

    "react": "^15.5.4",
    "react-dom": "^15.5.4",
    "react-helmet": "5.0.3",

no ^?

why?

@udovenko

Comments from Reviewable

justin808 · 2017-04-13T09:41:22Z

Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

Comments from Reviewable

udovenko mentioned this pull request Apr 10, 2017

Allow to return multiple markup strings from react_component helper #793

Closed

udovenko force-pushed the return_markup_hash_form_react_component_helper branch from 863fd08 to d612f46 Compare April 13, 2017 08:31

udovenko added 14 commits April 13, 2017 19:53

Implement test for page title rendered in generator function on server

c410106

* Add conditional title to dummy app layout * Implement rendering title string in generator function on server * Add test for conditional title to integration_spec * Refactor code in react_on_rails helper

Add some notes to API doc

6d49519

Implement example with ReactHelmet

2079156

* Now test page yields title rendered by ReactHelmet * Add tests with disabled JS to integration spec to ensure correct title rendering on server * Mark HTML strings other than component HTML (composed with props and console script) as html_safe

Fix for dummy application title

ba0b870

Pass params hash to #build_react_component_result_for_... methods

e6f2d34

Now component key in renderedHtml object expected to match component …

f768529

…registration key

Renamed example page and component registration

4c9bc3e

* Renamed example page to react_helmet * Changed route to example page * Renamed component registration to ReactHelmetApp * Additional refactoring

Add docs and fix comments

a6d8274

* Create additional-reading/react-helmet.mb * Fix comments for generator function * Update javascript-api.md

Update docs

337b477

* Update README.md * Update CHANGELOG.md

Small fixes for JS tests from previous PR

f18cedb

Updated CHANGELOG.md and README.md

3c20d3a

Assign params to local variables in #build_react_component_result_for…

a81da5b

…_... methods

Now rendered HTML for React component goes under the constant key com…

ebfb6c7

…ponentHtml * Implement constant key usage in #build_react_component_result_for_server_rendered_hash method of react_on_rails helper * Updated generator function for server rendering * Additional refactoring

udovenko added 7 commits April 13, 2017 19:57

Fixed react-helmet.md

010a28e

* Fixed additional-reading/react-helmet.md * Updated react_helmet.erb template accordingly

Updated javascript-api.md

362d8b8

Returnred rubocop back to gemfile

9938bd5

Implement required params

f8a0069

* Implement required params for #build_react_component_result_for... methods * Moved component_html_key to the constant

Returned back checking for "componentHtml" key presence

2eeb3f3

Updated CHANGELOG.md

cdaccfb

Fixed CHANGELOG.md

dd1190d

udovenko force-pushed the return_markup_hash_form_react_component_helper branch from 1fab7e2 to dd1190d Compare April 13, 2017 08:59

Update react-helmet

301adb2

* Update react-helmet to v5.0.3 to resolve PropTypes warnings * Fixed rubocop warnings

justin808 merged commit d5d579e into shakacode:master Apr 13, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Return markup hash form react component helper #800

Return markup hash form react component helper #800

udovenko commented Apr 10, 2017 •

edited by justin808

Loading

justin808 commented Apr 10, 2017

udovenko commented Apr 10, 2017

udovenko commented Apr 10, 2017

udovenko commented Apr 11, 2017

udovenko commented Apr 11, 2017

udovenko commented Apr 11, 2017

justin808 commented Apr 11, 2017

udovenko commented Apr 12, 2017

udovenko commented Apr 12, 2017

justin808 commented Apr 12, 2017

udovenko commented Apr 12, 2017

udovenko commented Apr 12, 2017

justin808 commented Apr 12, 2017

justin808 commented Apr 13, 2017

coveralls commented Apr 13, 2017

justin808 commented Apr 13, 2017

justin808 commented Apr 13, 2017

Return markup hash form react component helper #800

Return markup hash form react component helper #800

Conversation

udovenko commented Apr 10, 2017 • edited by justin808 Loading

justin808 commented Apr 10, 2017

udovenko commented Apr 10, 2017

udovenko commented Apr 10, 2017

udovenko commented Apr 11, 2017

udovenko commented Apr 11, 2017

udovenko commented Apr 11, 2017

justin808 commented Apr 11, 2017

udovenko commented Apr 12, 2017

udovenko commented Apr 12, 2017

justin808 commented Apr 12, 2017

udovenko commented Apr 12, 2017

udovenko commented Apr 12, 2017

justin808 commented Apr 12, 2017

justin808 commented Apr 13, 2017

coveralls commented Apr 13, 2017

justin808 commented Apr 13, 2017

justin808 commented Apr 13, 2017

udovenko commented Apr 10, 2017 •

edited by justin808

Loading