-
Notifications
You must be signed in to change notification settings - Fork 1.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
Add HMR to the dev server #641
Changes from all commits
f641cd0
97a8d00
a6bc2ce
f4654de
686e41b
7cbf899
7989628
c8260d6
b240023
604b30d
63cddc5
bdaa925
51b34ce
8c1dd96
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,31 @@ | ||
const ExtractTextPlugin = require('extract-text-webpack-plugin') | ||
const path = require('path') | ||
const { env } = require('../configuration.js') | ||
const { env, settings } = require('../configuration.js') | ||
|
||
const postcssConfigPath = path.resolve(process.cwd(), '.postcssrc.yml') | ||
const isProduction = env.NODE_ENV === 'production' | ||
const extractCSS = !settings.dev_server.hmr | ||
|
||
module.exports = { | ||
const extractOptions = { | ||
fallback: 'style-loader', | ||
use: [ | ||
{ loader: 'css-loader', options: { minimize: isProduction } }, | ||
{ loader: 'postcss-loader', options: { sourceMap: true, config: { path: postcssConfigPath } } }, | ||
'resolve-url-loader', | ||
{ loader: 'sass-loader', options: { sourceMap: true, indentedSyntax: true } } | ||
] | ||
} | ||
|
||
// For production extract styles to a separate bundle | ||
const extractCSSLoader = { | ||
test: /\.(scss|sass|css)$/i, | ||
use: ExtractTextPlugin.extract(extractOptions) | ||
} | ||
|
||
// For hot-reloading use regular loaders | ||
const inlineCSSLoader = { | ||
test: /\.(scss|sass|css)$/i, | ||
use: ExtractTextPlugin.extract({ | ||
fallback: 'style-loader', | ||
use: [ | ||
{ loader: 'css-loader', options: { minimize: env.NODE_ENV === 'production' } }, | ||
{ loader: 'postcss-loader', options: { sourceMap: true, config: { path: postcssConfigPath } } }, | ||
'resolve-url-loader', | ||
{ loader: 'sass-loader', options: { sourceMap: true } } | ||
] | ||
}) | ||
use: ['style-loader'].concat(extractOptions.use) | ||
} | ||
|
||
module.exports = isProduction || extractCSS ? extractCSSLoader : inlineCSSLoader |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,11 @@ | ||
const { join } = require('path') | ||
const { settings } = require('../configuration.js') | ||
|
||
module.exports = { | ||
test: /\.(js|jsx)?(\.erb)?$/, | ||
exclude: /node_modules/, | ||
loader: 'babel-loader' | ||
loader: 'babel-loader', | ||
options: { | ||
cacheDirectory: join(settings.cache_path, 'babel-loader') | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,41 +1,12 @@ | ||
const ExtractTextPlugin = require('extract-text-webpack-plugin') | ||
const { env } = require('../configuration.js') | ||
const { env, settings } = require('../configuration.js') | ||
|
||
// Change it to false if you prefer Vue styles to be inlined by javascript in runtime | ||
const extractStyles = false | ||
|
||
const cssLoader = [ | ||
{ loader: 'css-loader', options: { minimize: env.NODE_ENV === 'production' } }, | ||
{ loader: 'postcss-loader', options: { sourceMap: true } }, | ||
'resolve-url-loader' | ||
] | ||
const sassLoader = cssLoader.concat([ | ||
{ loader: 'sass-loader', options: { sourceMap: true, indentedSyntax: true } } | ||
]) | ||
const scssLoader = cssLoader.concat([ | ||
{ loader: 'sass-loader', options: { sourceMap: true } } | ||
]) | ||
|
||
function vueStyleLoader(loader) { | ||
if (extractStyles) { | ||
return ExtractTextPlugin.extract({ | ||
fallback: 'vue-style-loader', | ||
use: loader | ||
}) | ||
} | ||
return ['vue-style-loader'].concat(loader) | ||
} | ||
const isProduction = env.NODE_ENV === 'production' | ||
const extractCSS = !settings.dev_server.hmr | ||
|
||
module.exports = { | ||
test: /\.vue$/, | ||
loader: 'vue-loader', | ||
options: { | ||
loaders: { | ||
js: 'babel-loader', | ||
file: 'file-loader', | ||
css: vueStyleLoader(cssLoader), | ||
scss: vueStyleLoader(scssLoader), | ||
sass: vueStyleLoader(sassLoader) | ||
} | ||
extractCSS: isProduction || extractCSS | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,7 @@ development: | |
dev_server: | ||
host: localhost | ||
port: 3035 | ||
hmr: false | ||
|
||
test: | ||
<<: *default | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,10 @@ def running? | |
false | ||
end | ||
|
||
def hot_module_replacing? | ||
fetch(:hmr) | ||
end | ||
|
||
def host | ||
fetch(:host) | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,17 +32,26 @@ def javascript_pack_tag(*names, **options) | |
# in config/webpack/shared.js. By default, this list is auto-generated to match everything in | ||
# app/javascript/packs/*.js. In production mode, the digested reference is automatically looked up. | ||
# | ||
# Note: If the development server is running and hot module replacement is active, this will return nothing. | ||
# In that setup you need to configure your styles to be inlined in your JavaScript for hot reloading. | ||
# | ||
# Examples: | ||
# | ||
# # In development mode: | ||
# <%= stylesheet_pack_tag 'calendar', 'data-turbolinks-track': 'reload' %> # => | ||
# <link rel="stylesheet" media="screen" href="/packs/calendar.css" data-turbolinks-track="reload" /> | ||
# | ||
# # In development mode with hot module replacement: | ||
# <%= stylesheet_pack_tag 'calendar', 'data-turbolinks-track': 'reload' %> # => | ||
# nil | ||
# | ||
# # In production mode: | ||
# <%= stylesheet_pack_tag 'calendar', 'data-turbolinks-track': 'reload' %> # => | ||
# <link rel="stylesheet" media="screen" href="/packs/calendar-1016838bab065ae1e122.css" data-turbolinks-track="reload" /> | ||
def stylesheet_pack_tag(*names, **options) | ||
stylesheet_link_tag(*sources_from_pack_manifest(names, type: :stylesheet), **options) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to update the above docs to indicate what happens in HMR, as well as for the possibility that the webpack-dev-server is not running for development mode. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, added some notes in the README. Anything missing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The doc for the method should be updated (or removed? with a reference to the readme.) |
||
unless Webpacker.dev_server.running? && Webpacker.dev_server.hot_module_replacing? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not correct in that React on Rails users will build static assets for test runs while the dev server is possibly running for development. Thus, we'll skip this tag when we need it for tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @justin808 What are the implications? I guess dev server can be used too for serving assets in test environment and don't think that will affect any behaviour. Obviously, if someone wants to statically build and test they can turn off the dev-server and use watcher or on-demand compilation. With dev-server proxy usage is more transparent and consistent so the output will be same in views regardless of compiler used. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I fixed it such that Webpacker.dev_server.running? will return false in testing by moving the dev_server configuration to the development env only in webpacker.yml. So now the tag will appear in test mode. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @gauravtiwari wrote:
Since we have a different test setup in webpack for test compared to dev (we want more logging in dev), I'd feel uncomfortable that some of the setup is coming from a different webpack config. |
||
stylesheet_link_tag(*sources_from_pack_manifest(names, type: :stylesheet), **options) | ||
end | ||
end | ||
|
||
private | ||
|
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.
@justin808 You mean this doc?
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.
Maybe adjust this line to indicate more than "development mode":
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 👍