-
Notifications
You must be signed in to change notification settings - Fork 383
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
Babel 6 / CSS Modules / Rails hot reloading #175
Conversation
5e46c27
to
c088fc4
Compare
@alexfedoseev I'll start reviewing. |
|
||
// Next two depend on jQuery | ||
// This one depend on jQuery |
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.
depends
@alexfedoseev 👏 👏 👏 FABULOUS!!! As soon as CI tests pass, let's merge. @robwise there's a couple testing to-do's in the PR. |
@@ -1,2 +1,11 @@ | |||
module ApplicationHelper | |||
def javascript_include_env_tag(asset, dev_asset, params = {}) |
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.
This name is too close to the default rails one javascript_include_tag
and thus easy to miss.
I'd suggest env_specific_javascript_include_tag
and env_specific_stylesheet_link_tag
.
@import 'app-bundle'; | ||
|
||
// Non-webpack assets | ||
@import 'application_dev'; |
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.
Where is this partial located?
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.
Right next to current file.
4c9ff07
to
410bac6
Compare
"babel-preset-stage-0": "^6.3.13", | ||
"bootstrap-loader": "^1.0.0-rc", | ||
"bootstrap-sass": "^3.3.5", | ||
"css-loader": "^0.23.0", | ||
"es5-shim": "^4.3.1", | ||
"es6-promise": "^3.0.2", |
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.
Isn't this redundant given babel-polyfill
?
Configured testing so that: * If we're not running a webpack process to watch the client JS files, then we build the client or server files. * Test will let you know if it's skipping building. Expression used to see if a watch process is running: `pgrep -fl '\\-w \\-\\-config webpack\\.#{type}\\.rails\\.build\\.config\\.js'` * Renamed asset helpers so usage is like this: <%= env_stylesheet_link_tag 'application_prod', 'application_dev', media: 'all', 'data-turbolinks-track' => true %> <%= env_javascript_include_tag nil, 'http://localhost:3500/vendor-bundle.js' %> <%= env_javascript_include_tag nil, 'http://localhost:3500/app-bundle.js' %> <%= env_javascript_include_tag 'application_prod', 'application_dev', 'data-turbolinks-track' => true %> TODO: Should we consider having tests use the Rails hot reload server? Remove building bundles from install script Since the new spec configuration does it. Fix scss linter errors Had to turn off SelectorFormat since css modules require JS style names. Fix typo
583ba35
to
16e770c
Compare
…s-rails-hot Babel 6 / CSS Modules / Rails hot reloading
Merged. |
👍 🎆 🎉 |
bootstrap-loader
&sass-resources-loader
/cc: @justin808 @robwise @yorzi