shakacode / old-react-on-rails-examples

Various example apps for React on Rails, outdated
1 stars 0 forks source link

Denis/Questions and thoughts #36

Open udovenko opened 7 years ago

udovenko commented 7 years ago

I'm still not completely familiar with all techs that use in this repo so it is very likely that I've missed or misunderstood sometging. But anyway...

General:

  1. Why projects are organized as subdirectories within a single repo? It seems easier to manage access for different repos. Moreover, as the number of examples will grow, the repo simply will be bloated with data wich can lead to a long cloning process, inconvenient search experience and so on...
  2. I think repos (or example apps subdirs) should be organized and named around different techs and approaches (like react-router-v4-flow-webpack-2 and etc.) rather than dummy apps purpouses. I expect that apps will allways about something like todos.
udovenko commented 7 years ago

Specifically for todo-app-production:

  1. It is beter to list the tech stack somewhere to show people what this example is about (see point 2 above).
  2. I see no server rendering setup and implementation. Was this example build to show only client-side functionality?
  3. NavLinks do not work. I see no routes defenition.

To be continued...

udovenko commented 7 years ago
  1. Just catched my eye: for Ruby code there are no early returns used. And Rubocop does not throw warnings for this:
    if @todo.save
      render json: @todo, status: :created, location: @todo
    else
      render json: @todo.errors, status: :unprocessable_entity
    end

    Seems unusual...

udovenko commented 7 years ago
  1. application.html.erb allways prints empty ENV['REACT_ON_RAILS_ENV']. Am I missing some configuration?
  2. The way that reducers work seems a bit clumsy and unclear. Maybe it's because I'm not closely familiar with redux-actions, but I think bigger projects require something like Immutable.Record instances with their own methods for store state elements.
udovenko commented 7 years ago
  1. I think Storebook should show real components from the app is being developed, not a dummy buttons. And again I'm not sure that this is a good idea to throw all possible techs in a single example project. Maybe it is better to build separate example for this.
  2. I see no webpack-devserver setup and all the components are implemented as functions. No HotReloading?
udovenko commented 7 years ago
  1. I really like an idea of modular Webpack config and writing tests for each webpack-helper module.
  2. But I don't like separate files for different environments like store.dev.js and store.prod.js. They have only two different lines, I think it is better to use if statements with comments inside single file.
udovenko commented 7 years ago
  1. Small notice - RSpec output is not colorized.
  2. Why :selenium_chrome webdriver for RSpec feature specs? It allways causes problems and requires developer to check ChromeDriver version against his Chrome version. Why not Phantomjs?