Doc/xhr hydration examples #1095

hchevalier · 2018-05-27T19:43:44Z

This change is

coveralls · 2018-05-27T19:57:58Z

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

coveralls · 2018-05-27T19:57:58Z

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

coveralls · 2018-05-27T19:57:58Z

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

hchevalier · 2018-05-27T20:23:35Z

spec/dummy/app/views/pages/xhr_refresh.html.erb

+<p>
+  This example demonstrates client side manual rehydration after a component replacement through XHR.<br/><br/>
+
+  The "Refresh" button on this page will trigger an asynchrounous refresh of component-container content.<br/>


Typo on "asynchronous"

hchevalier · 2018-05-27T20:23:55Z

spec/dummy/app/views/pages/xhr_refresh.html.erb

+  Components will be prerendered by the server and inserted in the DOM (spec/dummy/app/views/pages/xhr_refresh.js.erb)<br/>
+  No client rehydration will occur, preventing any event handler to be correctly attached<br/><br/>
+
+  Thus, the onChange handler of the HelloWorld component won't trigger whereas the one from HellowWorldRehydratable will, thanks to the "hydrate" javscript event dispacthed from xhr_refresh.js.erb<br />


Typo on "javascript"

justin808 · 2018-05-28T07:25:31Z

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):

Previously, hchevalier wrote…

Typo on "asynchronous"

yes, please fix typo!

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

Previously, hchevalier wrote…

Typo on "javascript"

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

justin808 · 2018-05-28T07:25:57Z

Changes requested.

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

Comments from Reviewable

hchevalier · 2018-05-28T15:59:23Z

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

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

Previously, justin808 (Justin Gordon) wrote…

red dots mean extra spaces

Done.

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

Previously, justin808 (Justin Gordon) wrote…

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

Done.

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

Previously, justin808 (Justin Gordon) wrote…

do we really need to use sprockets for jquery?

Done.

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

Previously, justin808 (Justin Gordon) wrote…

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.

Done.

Comments from Reviewable

justin808 · 2018-05-30T01:00:29Z

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:

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

hchevalier · 2018-05-30T20:54:28Z

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):

Previously, justin808 (Justin Gordon) wrote…

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

Done.

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

Previously, justin808 (Justin Gordon) wrote…

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

Done.

Comments from Reviewable

justin808 · 2018-05-30T23:56:50Z

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

justin808 · 2018-06-01T01:24:21Z

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

Comments from Reviewable

justin808 · 2018-06-01T01:30:24Z

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

justin808 · 2018-06-01T01:31:04Z

Thanks @hchevalier!

hchevalier added 5 commits May 27, 2018 21:07

Document hydration for XHR-substituted components

629435f

Update README.md

2cd62b8

Fix rubocop offenses

2a89b41

Update CHANGELOG.md

7f6d547

Add PR reference to CHANGELOG.md

bff7bfb

Add missing route to dummy_no_webpacker

788bc8a

hchevalier commented May 27, 2018

View reviewed changes

hchevalier added 4 commits May 28, 2018 12:55

Refactor code according to review feedbacks

f9cb0b4

Stop using sprocket jquery

3737f92

Remove unused piece of code

d0f029c

Fix eslint offenses on HelloWorldRehydratable

a23d314

Refactor code according to review feedbacks

ffdb42e

Remove unused aliased jquery exposition

f33fda0

justin808 merged commit 047f229 into shakacode:master Jun 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Doc/xhr hydration examples #1095

Doc/xhr hydration examples #1095

hchevalier commented May 27, 2018 •

edited by justin808

Loading

coveralls commented May 27, 2018

coveralls commented May 27, 2018

coveralls commented May 27, 2018 •

edited

Loading

hchevalier May 27, 2018

hchevalier May 27, 2018 •

edited

Loading

justin808 commented May 28, 2018

justin808 commented May 28, 2018

hchevalier commented May 28, 2018

justin808 commented May 30, 2018

hchevalier commented May 30, 2018

justin808 commented May 30, 2018

justin808 commented Jun 1, 2018

justin808 commented Jun 1, 2018

justin808 commented Jun 1, 2018

Doc/xhr hydration examples #1095

Doc/xhr hydration examples #1095

Conversation

hchevalier commented May 27, 2018 • edited by justin808 Loading

coveralls commented May 27, 2018

coveralls commented May 27, 2018

coveralls commented May 27, 2018 • edited Loading

hchevalier May 27, 2018

Choose a reason for hiding this comment

hchevalier May 27, 2018 • edited Loading

Choose a reason for hiding this comment

justin808 commented May 28, 2018

justin808 commented May 28, 2018

hchevalier commented May 28, 2018

justin808 commented May 30, 2018

hchevalier commented May 30, 2018

justin808 commented May 30, 2018

justin808 commented Jun 1, 2018

justin808 commented Jun 1, 2018

justin808 commented Jun 1, 2018

hchevalier commented May 27, 2018 •

edited by justin808

Loading

coveralls commented May 27, 2018 •

edited

Loading

hchevalier May 27, 2018 •

edited

Loading