-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
separate into npm module and example project #626
Conversation
…eed them here now
Since there's not a lot of change other than deletions in this branch, it's probably easier to read through the diff on the |
very interesting. |
@quicksnap no problem, looking forward to your feedback. by the way, i think it was a mistake to bring the api logic into the npm module, since it should be focused just on universal rendering. i'm going to remove it from there and bring it back into the |
@quicksnap ok, api is now back in the |
The npm module is now |
I've opened another PR #685 that takes a similar approach to here, using If anyone would prefer to continue using redux-router, they can map their dependency at version |
Once again, I got caught up and didn't get a chance to look at this. I'm going to try to review in a few hours. Does anyone else have feedback so far? |
This seems very interesting. Haven't had a look at it yet but the idea behind this seems like the right way forward. Gonna have a look soon. |
Is As for the movement towards an npm package - that's a must. Where the demarkation line should be drawn is another question. |
There are bound to be different answers to the question of where the line should be drawn, based on the person answering it. It's hard to imagine a single optimal Webpack config that works for everybody but just how much someone would want to deal with it depends on their use case. Then again as I'm writing this I realize this is a pretty comprehensive discussion, not really something for this PR. Basically what would be a major version bump that totally changes the nature of the project :) If the intention is to make a sort of general purpose universal renderer for a specific React/Redux-based setup, it's also conceivable to rename it from "universal-hot-example". |
@msikma Yes, the setup has to be specific (or choosable from a finite set of predefined options). That's the only possible way to do it. |
I think in the end there could be a pretty decent amount of flexibility given that we'd generate the Webpack config and other things in JS. So it wouldn't be that bad. We could cover a range from simple websites to optimized larger applications if we let the user handle a few things by themselves such as routing. There's lots of different tradeoffs that could be made. |
I'm going to close this PR as it's superseded by #759 Please direct any further comments over there. |
This PR removes the base 'starter' parts of this example repo (i.e. webpack configuration, serverside rendering, routing) and leaves just the project code (i.e. example redux flow, api, css modules, theming, chat).
It's able to function without the rest of the code because I've created another branch,
npm-module
(https://github.com/bdefore/react-redux-universal-hot-example/tree/npm-module) which is publishable as an npm module that you can then declare as a dependency in the example project or projects from scratch. For the meantime, I've also put it up here with a README https://github.com/bdefore/redux-universal-renderer and published it on npm asredux-universal-renderer
Why make an npm module? Developers who base their projects on this repo have suffered from the following issues, that I think would be alleviated by this change:
Very curious to hear what you guys think!