Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moving header to React with shared store #277

Merged
merged 1 commit into from
Jul 20, 2016
Merged

Conversation

justin808
Copy link
Member

@justin808 justin808 commented Apr 14, 2016

Bugs and TODOs:

  1. Turbolinks is preventing the proper header link changes. If you open
    links in different tabs, you get mostly correct behavior.
  2. When you open the /simple in a different tab, the header is not
    highlighted.

NOTE: Not doing this PR in Reviewable, as I want the comments at the top level in the PR.


This change is Reviewable

@justin808
Copy link
Member Author

justin808 commented Jul 12, 2016

Build is failing!

I have no idea why this branch is breaking all tests.

Detected are the following stale generated files: app/assets/webpack/app-bundle.js app/assets/webpack/vendor-bundle.js app/assets/webpack/app-bundle.css app/assets/webpack/vendor-bundle.css app/assets/webpack/server-bundle.js React on Rails will ensure your JavaScript generated files are up to date, using your /client level package.json `build:test` command. Building Webpack assets... Completed building Webpack assets. ERROR when compiling base_js_code! See file tmp/base_js_code.js to correlate line numbers of error. Error is Error: Cannot find module "../components/NavigationBar/NavigationBar" webpackMissingModule ((execjs):59853:87) Object. ((execjs):59853:205) **webpack_require** ((execjs):55:30) Object. ((execjs):8232:32) **webpack_require** ((execjs):55:30) Object. ((execjs):8169:19) **webpack_require** ((execjs):55:30) Object. ((execjs):83:19) **webpack_require** ((execjs):55:30) (execjs):75:18 /home/travis/.rvm/gems/ruby-2.3.1/gems/execjs-2.7.0/lib/execjs/ruby_racer_runtime.rb:15:in `rescue in block in initialize' /home/travis/.rvm/gems/ruby-2.3.1/gems/execjs-2.7.0/lib/execjs/ruby_racer_runtime.rb:12:in`block in initialize' /home/travis/.rvm/gems/ruby-2.3.1/gems/execjs-2.7.0/lib/execjs/ruby_racer_runtime.rb:75:in `block in lock' /home/travis/.rvm/gems/ruby-2.3.1/gems/execjs-2.7.0/lib/execjs/ruby_racer_runtime.rb:73:in`Locker' /home/travis/.rvm/gems/ruby-2.3.1/gems/execjs-2.7.0/lib/execjs/ruby_racer_runtime.rb:73:in `lock' /home/travis/.rvm/gems/ruby-2.3.1/gems/execjs-2.7.0/lib/execjs/ruby_racer_runtime.rb:9:in`initialize' /home/travis/.rvm/gems/ruby-2.3.1/gems/execjs-2.7.0/lib/execjs/runtime.rb:57:in `new' /home/travis/.rvm/gems/ruby-2.3.1/gems/execjs-2.7.0/lib/execjs/runtime.rb:57:in`compile' /home/travis/.rvm/gems/ruby-2.3.1/gems/execjs-2.7.0/lib/execjs/module.rb:27:in `compile' /home/travis/.rvm/gems/ruby-2.3.1/gems/react_on_rails-6.0.4/lib/react_on_rails/server_rendering_pool/exec.rb:91:in`create_js_context' /home/travis/.rvm/gems/ruby-2.3.1/gems/react_on_rails-6.0.4/lib/react_on_rails/server_rendering_pool/exec.rb:10:in `block in reset_pool' /home/travis/.rvm/gems/ruby-2.3.1/gems/connection_pool-2.2.0/lib/connection_pool/timed_stack.rb:169:in`try_create' /home/travis/.rvm/gems/ruby-2.3.1/gems/connection_pool-2.2.0/lib/connection_pool/timed_stack.rb:81:in `block (2 levels) in pop' /home/travis/.rvm/gems/ruby-2.3.1/gems/connection_pool-2.2.0/lib/connection_pool/timed_stack.rb:77:in`loop' /home/travis/.rvm/gems/ruby-2.3.1/gems/connection_pool-2.2.0/lib/connection_pool/timed_stack.rb:77:in `block in pop' /home/travis/.rvm/gems/ruby-2.3.1/gems/connection_pool-2.2.0/lib/connection_pool/timed_stack.rb:76:in`synchronize' /home/travis/.rvm/gems/ruby-2.3.1/gems/connection_pool-2.2.0/lib/connection_pool/timed_stack.rb:76:in `pop' /home/travis/.rvm/gems/ruby-2.3.1/gems/connection_pool-2.2.0/lib/connection_pool.rb:89:in`checkout' /home/travis/.rvm/gems/ruby-2.3.1/gems/connection_pool-2.2.0/lib/connection_pool.rb:61:in `block in with' /home/travis/.rvm/gems/ruby-2.3.1/gems/connection_pool-2.2.0/lib/connection_pool.rb:60:in`handle_interrupt' /home/travis/.rvm/gems/ruby-2.3.1/gems/connection_pool-2.2.0/lib/connection_pool.rb:60:in `with' /home/travis/.rvm/gems/ruby-2.3.1/gems/react_on_rails-6.0.4/lib/react_on_rails/server_rendering_pool/exec.rb:72:in`eval_js' /home/travis/.rvm/gems/ruby-2.3.1/gems/react_on_rails-6.0.4/lib/react_on_rails/server_rendering_pool/exec.rb:36:in `server_render_js_with_console_logging' /home/travis/.rvm/gems/ruby-2.3.1/gems/react_on_rails-6.0.4/lib/react_on_rails/server_rendering_pool.rb:20:in`method_missing' /home/travis/.rvm/gems/ruby-2.3.1/gems/react_on_rails-6.0.4/app/helpers/react_on_rails_helper.rb:298:in `server_rendered_react_component_html' /home/travis/.rvm/gems/ruby-2.3.1/gems/react_on_rails-6.0.4/app/helpers/react_on_rails_helper.rb:112:in`react_component' /home/travis/build/shakacode/react-webpack-rails-tutorial/app/views/pages/index.html.erb:12:in `_app_views_pages_index_html_erb___3967042030590247683_59505580' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionview-5.0.0/lib/action_view/template.rb:158:in`block in render' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/notifications.rb:166:in `instrument' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionview-5.0.0/lib/action_view/template.rb:348:in`instrument' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionview-5.0.0/lib/action_view/template.rb:156:in `render' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionview-5.0.0/lib/action_view/renderer/template_renderer.rb:54:in`block (2 levels) in render_template' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionview-5.0.0/lib/action_view/renderer/abstract_renderer.rb:42:in `block in instrument' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/notifications.rb:164:in`block in instrument' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/notifications/instrumenter.rb:21:in `instrument' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/notifications.rb:164:in`instrument' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionview-5.0.0/lib/action_view/renderer/abstract_renderer.rb:41:in `instrument' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionview-5.0.0/lib/action_view/renderer/template_renderer.rb:53:in`block in render_template' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionview-5.0.0/lib/action_view/renderer/template_renderer.rb:61:in `render_with_layout' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionview-5.0.0/lib/action_view/renderer/template_renderer.rb:52:in`render_template' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionview-5.0.0/lib/action_view/renderer/template_renderer.rb:14:in `render' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionview-5.0.0/lib/action_view/renderer/renderer.rb:42:in`render_template' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionview-5.0.0/lib/action_view/renderer/renderer.rb:23:in `render' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionview-5.0.0/lib/action_view/rendering.rb:103:in`_render_template' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal/streaming.rb:217:in `_render_template' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionview-5.0.0/lib/action_view/rendering.rb:83:in`render_to_body' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal/rendering.rb:52:in `render_to_body' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal/renderers.rb:144:in`render_to_body' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/abstract_controller/rendering.rb:26:in `render' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal/rendering.rb:36:in`render' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal/instrumentation.rb:44:in `block (2 levels) in render' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/core_ext/benchmark.rb:12:in`block in ms' /home/travis/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/benchmark.rb:308:in `realtime' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/core_ext/benchmark.rb:12:in`ms' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal/instrumentation.rb:44:in `block in render' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal/instrumentation.rb:87:in`cleanup_view_runtime' /home/travis/.rvm/gems/ruby-2.3.1/gems/activerecord-5.0.0/lib/active_record/railties/controller_runtime.rb:25:in `cleanup_view_runtime' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal/instrumentation.rb:43:in`render' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal/implicit_render.rb:36:in `default_render' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal/basic_implicit_render.rb:4:in`block in send_action' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal/basic_implicit_render.rb:4:in `tap' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal/basic_implicit_render.rb:4:in`send_action' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/abstract_controller/base.rb:188:in `process_action' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal/rendering.rb:30:in`process_action' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/abstract_controller/callbacks.rb:20:in `block in process_action' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/callbacks.rb:126:in`call' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/callbacks.rb:506:in `block (2 levels) in compile' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/callbacks.rb:455:in`call' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/callbacks.rb:101:in `__run_callbacks__' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/callbacks.rb:750:in `_run_process_action_callbacks' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/callbacks.rb:90:in`run_callbacks' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/abstract_controller/callbacks.rb:19:in `process_action' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal/rescue.rb:20:in`process_action' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal/instrumentation.rb:32:in `block in process_action' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/notifications.rb:164:in`block in instrument' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/notifications/instrumenter.rb:21:in `instrument' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/notifications.rb:164:in`instrument' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal/instrumentation.rb:30:in `process_action' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal/params_wrapper.rb:248:in`process_action' /home/travis/.rvm/gems/ruby-2.3.1/gems/activerecord-5.0.0/lib/active_record/railties/controller_runtime.rb:18:in `process_action' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/abstract_controller/base.rb:126:in`process' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionview-5.0.0/lib/action_view/rendering.rb:30:in `process' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal.rb:190:in`dispatch' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_controller/metal.rb:262:in `dispatch' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_dispatch/routing/route_set.rb:50:in`dispatch' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_dispatch/routing/route_set.rb:32:in `serve' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_dispatch/journey/router.rb:39:in`block in serve' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_dispatch/journey/router.rb:26:in `each' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_dispatch/journey/router.rb:26:in`serve' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_dispatch/routing/route_set.rb:725:in `call' /home/travis/.rvm/gems/ruby-2.3.1/gems/rack-2.0.1/lib/rack/etag.rb:25:in`call' /home/travis/.rvm/gems/ruby-2.3.1/gems/rack-2.0.1/lib/rack/conditional_get.rb:25:in `call' /home/travis/.rvm/gems/ruby-2.3.1/gems/rack-2.0.1/lib/rack/head.rb:12:in`call' /home/travis/.rvm/gems/ruby-2.3.1/gems/rack-2.0.1/lib/rack/session/abstract/id.rb:222:in `context' /home/travis/.rvm/gems/ruby-2.3.1/gems/rack-2.0.1/lib/rack/session/abstract/id.rb:216:in`call' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_dispatch/middleware/cookies.rb:613:in `call' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_dispatch/middleware/callbacks.rb:38:in`block in call' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/callbacks.rb:97:in `__run_callbacks__' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/callbacks.rb:750:in`_run_call_callbacks' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/callbacks.rb:90:in `run_callbacks' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_dispatch/middleware/callbacks.rb:36:in`call' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_dispatch/middleware/remote_ip.rb:79:in `call' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_dispatch/middleware/debug_exceptions.rb:49:in`call' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_dispatch/middleware/show_exceptions.rb:31:in `call' /home/travis/.rvm/gems/ruby-2.3.1/gems/railties-5.0.0/lib/rails/rack/logger.rb:36:in`call_app' /home/travis/.rvm/gems/ruby-2.3.1/gems/railties-5.0.0/lib/rails/rack/logger.rb:24:in `block in call' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/tagged_logging.rb:70:in`block in tagged' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/tagged_logging.rb:26:in `tagged' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/tagged_logging.rb:70:in`tagged' /home/travis/.rvm/gems/ruby-2.3.1/gems/railties-5.0.0/lib/rails/rack/logger.rb:24:in `call' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_dispatch/middleware/request_id.rb:24:in`call' /home/travis/.rvm/gems/ruby-2.3.1/gems/rack-2.0.1/lib/rack/method_override.rb:22:in `call' /home/travis/.rvm/gems/ruby-2.3.1/gems/rack-2.0.1/lib/rack/runtime.rb:22:in`call' /home/travis/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.0/lib/active_support/cache/strategy/local_cache_middleware.rb:28:in `call' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_dispatch/middleware/executor.rb:12:in`call' /home/travis/.rvm/gems/ruby-2.3.1/gems/actionpack-5.0.0/lib/action_dispatch/middleware/static.rb:136:in `call' /home/travis/.rvm/gems/ruby-2.3.1/gems/rack-2.0.1/lib/rack/sendfile.rb:111:in`call' /home/travis/.rvm/gems/ruby-2.3.1/gems/railties-5.0.0/lib/rails/engine.rb:522:in `call' /home/travis/.rvm/gems/ruby-2.3.1/gems/rack-2.0.1/lib/rack/urlmap.rb:68:in`block in call' /home/travis/.rvm/gems/ruby-2.3.1/gems/rack-2.0.1/lib/rack/urlmap.rb:53:in `each' /home/travis/.rvm/gems/ruby-2.3.1/gems/rack-2.0.1/lib/rack/urlmap.rb:53:in`call' /home/travis/.rvm/gems/ruby-2.3.1/gems/capybara-2.7.1/lib/capybara/server.rb:43:in `call' /home/travis/.rvm/gems/ruby-2.3.1/gems/rack-2.0.1/lib/rack/handler/webrick.rb:86:in`service' /home/travis/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/webrick/httpserver.rb:140:in `service' /home/travis/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/webrick/httpserver.rb:96:in`run' /home/travis/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/webrick/server.rb:296:in `block in start_thread' ZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ react_renderer.rb: 92 wrote file tmp/base_js_code.js

@justin808 justin808 changed the title WIP for moving header to React with shared store Moving header to React with shared store Jul 12, 2016
@justin808
Copy link
Member Author

justin808 commented Jul 12, 2016

This is definitely an issue in with react-bootstrap right now, and this gives us lots of warnings.

react-bootstrap/react-bootstrap#1994

^(?!.*?Unknown prop) can be used filter the extra warnings in Chrome

@@ -1,5 +1,3 @@
// All webpack assets in development will be loaded via webpack dev server

// turbolinks comes from npm and is listed in webpack.client.base.config.js

//= require rails_startup
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is no longer needed, as we do the styling in react, rather than jQuery. Old file is immediately below.

@mapreal19
Copy link
Member

Reviewed 7 of 25 files at r1.
Review status: 7 of 25 files reviewed at latest revision, 8 unresolved discussions, some commit checks failed.


client/app/bundles/comments/containers/NavigationBarContainer.jsx, line 5 [r1] (raw file):

import { bindActionCreators } from 'redux';

import NavigationBar from '../components/NavigationBar/NavigationBar';

Does naming of components makes a difference? We have our component as Navigationbar.jsx. Shouldn't need to be NavigationBar.jsx


spec/features/shared/examples.rb, line 13 [r1] (raw file):

      expect(page).to have_css(".js-comment-author", text: name)
      expect(page).to have_css(".js-comment-text", text: text)
      expect(page).to have_css(".js-comment-count",

this should be #js-comment-count


spec/features/shared/examples.rb, line 23 [r1] (raw file):

    scenario "comment is not added" do
      expect(page).to have_selector(".comment", count: comments_count)
      expect(page).to have_css(".js-comment-count",

Same.


Comments from Reviewable

@mapreal19
Copy link
Member

Reviewed 1 of 25 files at r1.
Review status: 8 of 25 files reviewed at latest revision, 8 unresolved discussions, some commit checks failed.


Comments from Reviewable

@justin808 justin808 force-pushed the shared-redux-store branch 2 times, most recently from 380ac79 to e017384 Compare July 13, 2016 08:18
@justin808
Copy link
Member Author

Any update @mapreal19?

@mapreal19
Copy link
Member

Reviewed 1 of 27 files at r1.
Review status: 7 of 26 files reviewed at latest revision, 10 unresolved discussions, some commit checks failed.


client/app/bundles/comments/startup/NavigationBarApp.jsx, line 18 [r3] (raw file):

export default (_props, railsContext) => {
  // This is where we get the existing store.
  const stores = ReactOnRails.stores();

stores unused?


client/app/bundles/comments/startup/NavigationBarApp.jsx, line 27 [r3] (raw file):

  } else {
    return (
      <NavigationBar {...{ pathname }} />

On the simple React case here commentsCount would be undefined. Thus we don't show the counter and we get those failures on the specs


Comments from Reviewable

@mapreal19
Copy link
Member

Review status: 7 of 27 files reviewed at latest revision, 11 unresolved discussions.


config/initializers/react_on_rails.rb, line 34 [r4] (raw file):

  # Default is false. Can be overriden at the component level.
  # Set to false for debugging issues before turning on to true.
  config.prerender = true

one spec was failing because of this


Comments from Reviewable

@mapreal19
Copy link
Member

Review status: 7 of 27 files reviewed at latest revision, 12 unresolved discussions.


app/views/layouts/application.html.erb, line 24 [r4] (raw file):

<body>

<%= react_component "NavigationBarApp" %>

for the Simple React case should we pass the comments count as a prop?


Comments from Reviewable

return (
<Provider store={store}>
<NavigationBarContainer />

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justin808 Here's your problem - there are two components inside of the provider block.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I bet @radixhound is right!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying this out. Wonder why this worked in dev, but not testing!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rob and I spent a couple hours on this and have no idea why this works in manual testing but not in tests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out that it was Provider! @radixhound and @robwise.

Provider.js has this React call:

React.Children.only
object React.Children.only(object children)
Return the only child in children. Throws otherwise. 

I had a bug where there server app (but NOT the client app) had an extra component as pointed out above, and if the the server is not throwing errors, then the error does not show in the tests!

@robwise @mapreal19 @radixhound @jbhatab Please provide final review!

@justin808 justin808 force-pushed the shared-redux-store branch from 828be42 to 5140d74 Compare July 19, 2016 09:10
Using React & shared redux store for Nav Bar

* Header displays the count of comments for the two example tabs that
  use Redux.
* Example shows React on Rails shared redux store which allows 2 React
  on Rails components to both communicate with the same shared Redux
  store.
@justin808 justin808 force-pushed the shared-redux-store branch from 2632db9 to 0ecedf2 Compare July 20, 2016 01:04
@justin808 justin808 merged commit 47e4852 into master Jul 20, 2016
@justin808 justin808 deleted the shared-redux-store branch July 20, 2016 01:05
@mapreal19
Copy link
Member

Review status: 7 of 28 files reviewed at latest revision, 12 unresolved discussions.


spec/features/shared/examples.rb, line 4 [r7] (raw file):

require "features/shared/contexts"

shared_examples "New Comment Submission" do |expect_comment_count|

@justin808 we should try to avoid flag arguments.

You don't really know what is going when you see the caller

include_examples "New Comment Submission", false

http://martinfowler.com/bliki/FlagArgument.html


Comments from Reviewable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants