Coverage remained the same at ?% when pulling bff7bfb on hchevalier:doc/xhr_hydration_examples into 73cf07e on shakacode:master.

Coverage remained the same at ?% when pulling f33fda0 on hchevalier:doc/xhr_hydration_examples into 73cf07e on shakacode:master.

Review status: 0 of 15 files reviewed at latest revision, 2 unresolved discussions.

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

  Note: client hydration will not trigger for components rendered through XHR. You will have to handle it with javascript.
  For an example, see [spec/dummy/app/views/pages/xhr_refresh.rb](https://github.com/shakacode/react_on_rails/tree/master/spec/dummy/app/views/pages/xhr_refresh.rb). 
  

red dots mean extra spaces

spec/dummy/app/assets/javascripts/jquery_rails_manifest.js, line 2 at r2 (raw file):

//= require jquery
//= require jquery_ujs

do we really need to use sprockets for jquery?

spec/dummy/app/views/pages/xhr_refresh.html.erb, line 2 at r2 (raw file):

<% provide :other_javascript_tags do %>
  <%= javascript_include_tag 'jquery_rails_manifest', defer: true %>

This should not be needed if we're using the rails/webpacker way.

The jquery bits should be put into the webpacker tag.

This is pre-webpacker: https://github.com/shakacode/react_on_rails/blob/master/spec/dummy_no_webpacker/app/views/layouts/application.html.erb#L19

This is post-webpacker:

https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/app/views/layouts/application.html.erb#L16

this is the post webpacker jquery: https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/client/package.json#L27,

https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/client/webpack.client.rails.build.config.js#L106,

https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/client/webpack.client.base.config.js#L65

spec/dummy/app/views/pages/xhr_refresh.html.erb, line 21 at r2 (raw file):

yes, please fix typo!

spec/dummy/app/views/pages/xhr_refresh.html.erb, line 25 at r2 (raw file):

yes, fix!

spec/dummy/client/app/components/HelloWorldRehydratable.js, line 48 at r2 (raw file):

        // See https://github.com/shakacode/react_on_rails/commit/912118445f55c6f59664bf2b9f9ed97779ee71c9
        // You may have to replace "nextElementSibling" by "previousElementSibling" if you use an older version
        domNode = domNode.nextElementSibling;

get the script tag from just querying for the script tag with the where the <script data-dom-id> matches the component's div id.

definitely make sure all this JS passes the linter.

Comments from Reviewable

Changes requested.

Review status: all files reviewed at latest revision, 6 unresolved discussions.

Comments from Reviewable

Review status: 9 of 14 files reviewed at latest revision, 4 unresolved discussions.

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

Done.

spec/dummy/app/views/pages/xhr_refresh.html.erb, line 2 at r2 (raw file):

Done.

spec/dummy/app/assets/javascripts/jquery_rails_manifest.js, line 2 at r2 (raw file):

Done.

spec/dummy/client/app/components/HelloWorldRehydratable.js, line 48 at r2 (raw file):

Done.

Comments from Reviewable

Some suggestions. Looking good.

Review status: all files reviewed at latest revision, all discussions resolved.

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

import 'es5-shim';
import 'jquery'
import {} from 'jquery-ujs'

Any explanation for the addition of these?

I think we should use something like this for all of these:

• https://github.com/shakacode/react-webpack-rails-tutorial/search?q=es5-shim&unscoped_q=es5-shim
• https://github.com/shakacode/react-webpack-rails-tutorial/search?q=jquery-ujs&unscoped_q=jquery-ujs

spec/dummy/client/app/components/HelloWorldRehydratable.jsx, line 47 at r3 (raw file):

      // Get component specification <script> tag
      const componentSpecificationTag = document.querySelector(`script[data-dom-id=${component.id}]`);
      if (componentSpecificationTag) {

How can this be falsey for a React on Rails component?

Comments from Reviewable

Review status: 10 of 15 files reviewed at latest revision, 2 unresolved discussions.

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

Done.

spec/dummy/client/app/components/HelloWorldRehydratable.jsx, line 47 at r3 (raw file):

Done.

Comments from Reviewable

Just one thing I see...

Review status: all files reviewed at latest revision, all discussions resolved.

spec/dummy/client/webpack.client.base.config.js, line 73 at r4 (raw file):

          {
            loader: 'expose-loader',
            options: '$'

Why do we need to use $ for jQuery in example? I don't like it, as it the $ is used for ES6 string interpolation.

If you just want to let others know about this option, you can add a comment to the line above for jQuery to create a similar entry to expose $ if your rails JS code needs $ exposed globally.

Comments from Reviewable

Review status: all files reviewed at latest revision, 1 unresolved discussion.

Comments from Reviewable

Review status: all files reviewed at latest revision, 1 unresolved discussion.

README.md, line 545 at r5 (raw file):

  Note: client hydration will not trigger for components rendered through XHR. You will have to handle it with javascript.
  For an example, see [spec/dummy/app/views/pages/xhr_refresh.rb](https://github.com/shakacode/react_on_rails/tree/master/spec/dummy/app/views/pages/xhr_refresh.rb). 

Are you sure this link works? This will be done from the README.md so it might not.

spec/dummy/client/app/components/HelloWorldRehydratable.jsx, line 43 at r5 (raw file):

    const match = document.querySelectorAll(`[id^=${registeredComponentName}-react-component-]`);
    // Not all browsers support forEach on NodeList so we go with a classic for-loop
    for (let i = 0; i < match.length; i += 1) {

The

Comments from Reviewable

Thanks @hchevalier!