-
Notifications
You must be signed in to change notification settings - Fork 395
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
18 react hot loader implementation #69
18 react hot loader implementation #69
Conversation
"react-dom": "^15.2.1", | ||
"react-redux": "^4.4.5", | ||
"react-router": "^2.5.2", | ||
"redux": "^3.5.2", | ||
"redux-thunk": "^2.1.0", | ||
"toastr": "^2.1.2" | ||
"toastr": "^2.1.2", | ||
"webpack-dev-server": "^1.14.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move webpack-dev-server to devDependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, pending commit / push
|
||
module: { | ||
loaders: [ | ||
{ | ||
test: /\.(ts|tsx)$/, | ||
exclude: /node_modules/, | ||
loader: 'ts-loader' | ||
loaders: ['react-hot', 'babel','ts-loader'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I test to execute the app without babel-loader and it's working. So we only need:
loaders: ['react-hot', 'ts-loader']
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think webpack works if we configure this line like:
loaders: ['react-hot-loader', 'ts-loader']
or
loaders: ['react-hot', 'ts']
It's the same for it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good points!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -0,0 +1,120 @@ | |||
# React Hot Loader + Redux dev tool support |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We were writing this info in AboutPage, isn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the about page we can have a summary, but is a good idea to have detailed readme.md when a given user navigates to this sample in Github the readme.md will be automatically displayed
@@ -28,6 +29,7 @@ | |||
"css-loader": "^0.23.1", | |||
"deep-freeze": "0.0.1", | |||
"enzyme": "^2.4.1", | |||
"express": "^4.14.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove dependency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right, done
…re the dev server, removed devserver.js
Adding react hot loading plus dev tool support.
Added as well a readme.md (feedback welcome on this document, is just barebones, we could add a lot of useful info here: e.g. links to hot loading, redux, dev tool, snapshots...).
TODO: I will open a new issue to proper create a dev configuration including react hot loading, but that's out of the scope for this case (a new case will be created).