reduxjs / redux

A JS library for predictable global state management
https://redux.js.org
MIT License
60.8k stars 15.27k forks source link

Tweak Universal example #643

Closed gaearon closed 9 years ago

gaearon commented 9 years ago

Here's a few issues I'd like to see addressed:

gaearon commented 9 years ago

What if npm start printed a message like Run "npm run dev" for development, and "npm run prod" for production? npm run dev would launch both server and WebpackDevServer, npm run prod would use compiled assets. Does this make any sense?

cc @erikras @davezuko who have experience with universal examples

timdorr commented 9 years ago

That could work. I'm tweaking some my stuff in #642 around the port numbers and what it prints to screen when booting up.

timdorr commented 9 years ago

As for the last point, it's implied that everything except client.js and server.js is universal stuff. But I'm ambivalent about it. I personally keep everything top-level (following @erikras's lead on that), but don't mind reorganizing if that's more standard.

gaearon commented 9 years ago

Let's reorganize.

gaearon commented 9 years ago

Also, a few things:

gaearon commented 9 years ago

api folder actually is a good example of something that would go in server/ folder because it's not used on client.

timdorr commented 9 years ago

Just addressed the first two bullet points in this issue in my other PR. I don't think we should worry about production usage, because there are too many assumptions about setup there. Running a universal app in production on Heroku and your own Ubuntu box is drastically different.

timdorr commented 9 years ago

api isn't universal in this example, but would be in other contexts. Take a blogging engine. That would have a way to fetch the posts on the page, which would apply to the initial server-side render and then as you browse around. I'd want to hint at that so people don't end up writing two versions of the same thing.

gaearon commented 9 years ago

@timdorr

api isn't universal in this example, but would be in other contexts.

In this case we need "increment" action to work through server, otherwise its role is confusing. Bonus point: optimistic update.

gaearon commented 9 years ago

I guess my point is: we have two places with setTimeout calls. actions and api confuse me because they look very similar and the relationship is obscured. I'd assume api is what runs on server (actual REST-like API), and actions is how I access it, both on client and server.

timdorr commented 9 years ago

Hmm, I didn't want to get into a real API example, since that starts touching on things way outside of the realm of redux (a HTTP client like superagent or whatwg-fetch and more express coding). Hence the fake async via a setTimeout. I figure those looking for a more sophisticated example could check out awesome-redux or react-redux-universal-hot-example.

But yeah, we can get rid of the async increment action. That was some copy pasta on my part.

timdorr commented 9 years ago

Oh! I could, rather than wrapping the initial render in a promise to prefetch data, instead just use redux-promise to issue an initial action and hook into the .then returned from dispatch to send the response once it resolves. That would make it so the entire prefetch is redux-based.

gaearon commented 9 years ago

Yes, or you could use redux-thunk which we use in Reddit example for the same purpose.

dvdzkwsk commented 9 years ago

A bit late to the party, but I really like that dev vs. prod approach @gaearon; it's something I've been toying with integrating into the react-redux-starter-kit, since right now the server essentially just acts like your prod example.

I originally thought just having a prod mode would suffice, and didn't really want to have to run babel/register against the server, but after looking at the universal Redux example it definitely seems more convenient to be able to differentiate between serving a live vs. compiled app.

dvdzkwsk commented 9 years ago

I do have a question about the existing universal example, though. Since only one webpack bundle gets compiled, how would non-js imports be handled? Currently the application is able to render on the server because it makes use of babel/register, but once you get into importing any of the things that make webpack awesome (images, styles, etc.), I'm not sure how well this method holds up.

I was temporarily avoiding this issue by never running a non-compiled application from the server, but that makes server development fairly tedious. I'm at a loss for how the current example would accommodate these features. Does anybody have experience with this?

timdorr commented 9 years ago

Again, I didn't want to make any decisions about how someone might use it in production. There a eleventy bajillion ways of doing it and many options to consider (Data uri images or static files? Hash cache busting? etc etc.). Those are an exercise in how you might set up webpack, which has nothing to do with redux and how you use it with server rendering.

gaearon commented 9 years ago

I was temporarily avoiding this issue by never running a non-compiled application from the server, but that makes server development fairly tedious. I'm at a loss for how the current example would accommodate these features. Does anybody have experience with this?

One way is to just use Webpack for Node server. Not everybody would be on board with this though.

gaearon commented 9 years ago

I agree with @timdorr regarding keeping the scope to minimum. That said,

should be covered because it's related to universal rendering.

timdorr commented 9 years ago

@hedgerh and I had talked about that in the PR on his repo. I think those sort of "next steps" items should be put in an advanced recipe. Possibly a sort of "Tying It All Together" recipe.

Do you have a sense of when react-router is going to land a 1.0 (or at least 1.0-rc)? There look to be substantial changes upcoming, so it might be a good idea to wait a bit to avoid having to re-write such an article later on.

gaearon commented 9 years ago

1.0 isn't likely to change much from betas. 2.0 will be interesting but it'll take more time.

gaearon commented 9 years ago

I'd keep it open because there's still api / actions weirdness. Let's close when we refactor it to use async actions all the way.

timdorr commented 9 years ago

OK, sounds good.

What do you think about refactoring around react-transform and webpack-hot-middleware (since we're already using Express)?

gaearon commented 9 years ago

What do you think about refactoring around react-transform and webpack-hot-middleware (since we're already using Express)?

Sounds good to me, but this would need to be done for all examples at once.

timdorr commented 9 years ago

Well, in light of #690, I can focus on just the hot-middleware conversion. Shouldn't be that much work.

timdorr commented 9 years ago

What about including the efforts of #721 in the universal example? Or is that too much stuff in one place? I worry it might be getting too specific and guiding new users towards having to set up all this stuff just to get started with redux (rather than starting minimally and tacking on features as needed)

gaearon commented 9 years ago

Wouldn't #721 actually simplify the universal example by doing everything via Express?

timdorr commented 9 years ago

I mean more in terms of adding devTools into the mix. It's becoming the kitchensink example instead of the universal example :smile:

gaearon commented 9 years ago

No, I don't want to add DevTools there.

timdorr commented 9 years ago

OK, I'll just do hot-middleware and call it a day.

mwildehahn commented 8 years ago

@timdorr it is understandably out of scope of the redux documentation to go into how to run a universal example in production, but it also is a little annoying coming across "in production you wouldn't do this and it's out of scope to talk about what you should do". I've been incredibly impressed by the redux documentation and think it would be awesome if there was an "advanced recipe" like you mentioned, or just resources where people new to universal apps can learn more about them.

timdorr commented 8 years ago

The only mention of scope comes with the handling of static files. That's entirely a configuration thing that you would handle with nginx or whatever frontend webserver you have. That's something left to your sysadmin (which may be yourself!) to determine how to handle.

Nonetheless, that section of the docs was removed in #766 and will be put into the next documentation rebuild.

Outside of that, I do all of the things in the universal example in my real world production apps. There might be a slight change here or there, but it's almost entirely the same thing. This is production-ready code.

mwildehahn commented 8 years ago

@timdorr good to know, thanks!