cloverfield-tools / universal-react-boilerplate

A simple boilerplate Node app.
MIT License
904 stars 97 forks source link

Feature/react router with redux #59

Closed guidsen closed 8 years ago

guidsen commented 8 years ago

I've updated the PR for the Babel 6 upgrade.

Still one issue left: The onClick event in the App component doesn't fire. There might be something wrong with the React instance, but I couldn't fix the issue. Maybe somebody could test this and provide me a fix, cause I don't have really much time this week. Would be cool to have this in the repository.

What can we do about the CircleCI failing build? Seems a dep is causing this failing build.

ericelliott commented 8 years ago

Yes, I agree. Didn't notice the caret.

ericelliott commented 8 years ago

Also, I need to enable Greekeeper on this repo. =)

guidsen commented 8 years ago

Anyone has time to have a look at the issue?

ericelliott commented 8 years ago

I don't at the moment, sorry. );

edubsi commented 8 years ago

As far as I can tell the react app is not rerendered on the client. When I remove the routes the App is actually rerendered, when the state changes and the clicks are working. Sadly I am new to react and redux and can't figure out how to fix the issue. I am looking forward to see it glued together.

guidsen commented 8 years ago

Ha @eb88 thanks. You've pointed me in the right direction. I've fixed the issue. There was indeed something wrong with the client-side rendering. Reducers are being updated now. Only issue left is the fact that Webpack doesn't want to parse the app-home.js file in the root directory. Really strange, this worked before. Not a webpack expert.

It gives me the following error:

You may need an appropriate loader to handle this file type.
| export default {
|   APP_HOME: __dirname
| };
ericelliott commented 8 years ago

You'll also need to pull in master and fix merge conflicts. =)

guidsen commented 8 years ago

@ericelliott Done ;)!

ericelliott commented 8 years ago

Still getting the error you were complaining about?

ericelliott commented 8 years ago

Any luck with that Webpack error?

tkh44 commented 8 years ago

@ericelliott @guidsen The webpack error comes from here https://github.com/guidsen/universal-react-boilerplate/blob/feature/react-router-with-redux/webpack.config.dev.js#L28 and https://github.com/guidsen/universal-react-boilerplate/blob/feature/react-router-with-redux/webpack.config.prod.js#L36

That file is not included in babel-loader's scope. I would suggest putting the file in a special config folder or something along those lines with other files that need babel but aren't part of the main source.

tkh44 commented 8 years ago

Just tested it and works with

...
include: [
    path.join(__dirname, 'source'),
    path.join(__dirname, 'app-home.js')
]
...
ericelliott commented 8 years ago

Thanks for taking a look! =)

guidsen commented 8 years ago

@tkh44 Thanks for taking a look ;). Got it working now.

@ericelliott I've removed the unused files in routes/main cause they were just doing the same. App is working, I've added redux-logger as well to have some nice state output. Also changed the imports to absolutes :).

ericelliott commented 8 years ago

A few questions/comments:

  1. I see only one route - the default index route. Would you like to make another just to demonstrate the routing?
  2. Routes should really be in its own directory. Typically, there will be more than one per app, and we should demonstrate a recommended structure.
  3. I see some code that maybe could list some books, but I see no books to list. Should we prime the store with them and see some data being rendered in the components?
  4. For all filenames, please use all lowercase, and separate words with dashes. In other words, kebab-case filenames. Aids with cross-platform compatibility, because some platforms have trouble with mixed case. Specifically, some are case sensitive, and others are not.
guidsen commented 8 years ago

@ericelliott Read the answer of your comments below.

  1. So you mean adding a book detail route where we show just a book with a dummy image or so, to demonstrate how the user can add multiple routes? +1 for that one.
  2. What structure should we recommend and how should we bundle them? I suggest having a directory called routes inside the shared directory. So like this:
    • shared
      • routes
      • books
        • index.js
  3. I'm not sure what you mean here, could you explain more?
  4. I'll fix that!
ericelliott commented 8 years ago

I made a ton of changes to clarify some things. Sorry if I stepped on your toes. You can find them in the universal-fixes branch.

Please merge that into your PR. If you have better ideas, I'm happy to discuss those as well... note that I left the client rendering in a broken state (it was rendering blank in your branch... started to fix, then realized I might be taking to long...).

guidsen commented 8 years ago

I've merged your changes. I can't find why it's showing a blank page. Taking me to long.. Somebody else want to check this issue out? /cc @tkh44

ericelliott commented 8 years ago

Sorry I left the client in a sorry/unfinished state. When I pulled your branch, the client was rendering a blank page, so I figured I couldn't make it any worse.. ;)

tkh44 commented 8 years ago

@guidsen I can look at this tonight if it's still not working.

guidsen commented 8 years ago

@tkh44 Would love to. It has something to do with the props not being passed to the Title component. Had no time to debug further.

ericelliott commented 8 years ago

Any updates guys? I'm hoping to get this merged in the next couple days.

ericelliott commented 8 years ago

I've updated my universal-fixes branch to merge in changes from master.

ericelliott commented 8 years ago

Fixed and merged into master as v3.0.0. Still messy. I'll try to simplify the configuration by abstracting a lot of the moving parts with routing, redux, and universal render into its own module.

ericelliott commented 8 years ago

OK, I made an RDD to reduce all the boilerplate around universal routing and rendering with React & Redux to almost nothing.

I would love your feedback on the proposed API: https://github.com/ericelliott/react-easy-universal

LorbusChris commented 8 years ago

:+1: Awesome work done here, kudos! I really like the sane approach and clear structure of this repo. I'll be sure to check out react-easy-universal as well!