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

V2 -- Updated Dependencies, Webpack, Hot reloader #72

Merged
merged 5 commits into from
Nov 22, 2016
Merged

V2 -- Updated Dependencies, Webpack, Hot reloader #72

merged 5 commits into from
Nov 22, 2016

Conversation

stylesuxx
Copy link
Contributor

I have made the needed changes for the v4 gen to work with this PR too, so as soon as this gets pulled, i will open a PR in the other repo.

* Updated to the recent dependencies where possible
* Introduced react-hot-loader v3.0.0 beta
* Updated webpack config to new "style", there might still be some issues with the css related loaders, we will see in the generator.

* #60 I have not encountered this problem during testing
@stylesuxx stylesuxx changed the title Updated Dependencies, Webpack, Hot reloader V2 - Updated Dependencies, Webpack, Hot reloader Nov 20, 2016
@stylesuxx stylesuxx changed the title V2 - Updated Dependencies, Webpack, Hot reloader V2 -- Updated Dependencies, Webpack, Hot reloader Nov 20, 2016
@sthzg
Copy link
Member

sthzg commented Nov 21, 2016

Hi @weblogixx @stylesuxx,

could you coordinate these updates between you two? According to the last comment react-webpack-generators/generator-react-webpack#302 (comment) @weblogixx is planning to do the updates. But that might have overlapped with this PR (on a side-note: I need to check the Github email settings, as I don't get emails for new PRs).

@stylesuxx thanks for the PR. I haven't had time to check in detail, but I haven't seen a few of the integrations I had to do to make it work (changes to babelrc, wrapping Main in the <AppContainer /> component, manually triggering the module.hot.accept() API, adding react-hot-loader/patch as an entry point in Dev.js). The last time I've set it up was about three weeks ago, do you know if this is obsolete by now? (I still see it in their next-docs branch https://github.com/gaearon/react-hot-loader/tree/next-docs/docs, but code might progress quicker than docs 😄 ).

As said above, as @weblogixx mentioned to update the projects in the linked comment I think it would be the best if you two coordinated it between each other so that there is no double-work on that.

@stylesuxx
Copy link
Contributor Author

OK, I have not seen this comment yet. But it works for me - If @weblogixx has something tested, then I am happy.

Point is, my redux gen is using the V4 of the react gen which uses the latest V2.x template so problems down here are bubbling up on my side and I thought I'll just make the first step to get things rolling ;-)

@sthzg pretty sure I did test hot reloading, but I guess I should double check because I did not make this changes.

Let me know how I can help.

@weblogixx
Copy link
Member

Hi @stylesuxx,

unfortunately, I do not have the time for an emergency fix today :(. @sthzg: Could you please have a look at it? From the first (5 minute) view, it looks good and may get published.

Thanks and sorry :(

Copy link
Member

@sthzg sthzg left a comment

Choose a reason for hiding this comment

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

@stylesuxx 👍 thanks for the updates, the integration looks hot 😄.
there are just two small issues that break Travis (but only due to the airbnb rules, so they are a matter of a minute).


if (module.hot) {
module.hot.accept('./components/App', () => {
const NextApp = require('./components/App').default;
Copy link
Member

Choose a reason for hiding this comment

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

Looking at Travis, this little bugger needs the // eslint-diasable-line (to mute the global require() error)

const NextApp = require('./components/App').default;
ReactDOM.render(
<AppContainer>
<NextApp/>
Copy link
Member

Choose a reason for hiding this comment

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

And here the airbnb rules mandate the space before closing the tag (=> <NextApp />)

@sthzg sthzg merged commit ba0f20c into react-webpack-generators:2.x Nov 22, 2016
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.

3 participants