diff --git a/.gitignore b/.gitignore index 207100846..6b4101e28 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ /.bundle /pkg /test/test_app/log +/test/test_app/tmp node_modules diff --git a/README.md b/README.md index 2ba8ed34f..77b49e836 100644 --- a/README.md +++ b/README.md @@ -271,7 +271,7 @@ foreman start ``` By default, `webpack-dev-server` listens on `0.0.0.0` that means listening -on all IP addresses available on your system: LAN IP address, localhost, 127.0.0.1 etc. +on all IP addresses available on your system: LAN IP address, localhost, 127.0.0.1 etc. However, we use `localhost` as default hostname for serving assets in browser and if you want to change that, for example on cloud9 you can do so by changing host set in `config/webpacker.yml`. @@ -360,8 +360,14 @@ development: host: 0.0.0.0 port: 8080 https: false + hmr: false ``` +If you have hmr turned to `true`, then the `stylesheet_pack_tag` generates no output, as you will want +to configure your styles to be inlined in your JavaScript for hot reloading. During production and testing, the +`stylesheet_pack_tag` will create the appropriate HTML tags. + + #### Resolved Paths If you are adding webpacker to an existing app that has most of the assets inside @@ -445,6 +451,16 @@ You can checkout these links on this subject: - https://webpack.js.org/configuration/dev-server/#devserver-hot - https://webpack.js.org/guides/hmr-react/ +If you are using hot reloading, you should either set the `dev_server/hmr` key to TRUE or set the +ENV value `WEBPACKER_HMR=TRUE`. That way, your stylesheet pack tag will do **nothing** because you +need your styles inlined in your JavaScript for hot reloading to work properly. + +### Live Reloading or Static Reloading +Live Reloading is having your assets for development provided by the webpack-dev-server. + +You can override the the presence of the `dev_server` setup by setting ENV value: `WEBPACKER_DEV_SERVER=FALSE`. + +You might do this if you are switching back and forth between statically compiling files during development and trying to get hot reloading to work. ## Linking Styles, Images and Fonts @@ -593,9 +609,9 @@ and #### React -You may consider using [react-rails](https://github.com/reactjs/react-rails) or -[webpacker-react](https://github.com/renchap/webpacker-react) for more advanced react integration. -However here is how you can do it yourself: +If you need more advanced React-integration, like server rendering, redux, or react-router, see [shakacode/react_on_rails](https://github.com/shakacode/react_on_rails), [react-rails](https://github.com/reactjs/react-rails), and [webpacker-react](https://github.com/renchap/webpacker-react). + +If you're not concerned with view helpers to pass props or server rendering, can do it yourself: ```erb <%# views/layouts/application.html.erb %> diff --git a/lib/install/config/webpack/development.js b/lib/install/config/webpack/development.js index 7247e31aa..88bebac03 100644 --- a/lib/install/config/webpack/development.js +++ b/lib/install/config/webpack/development.js @@ -1,5 +1,6 @@ // Note: You must restart bin/webpack-dev-server for changes to take effect +const webpack = require('webpack') const merge = require('webpack-merge') const sharedConfig = require('./shared.js') const { settings, output } = require('./configuration.js') @@ -16,6 +17,7 @@ module.exports = merge(sharedConfig, { https: settings.dev_server.https, host: settings.dev_server.host, port: settings.dev_server.port, + hot: settings.dev_server.hmr, contentBase: output.path, publicPath: output.publicPath, compress: true, @@ -27,5 +29,10 @@ module.exports = merge(sharedConfig, { stats: { errorDetails: true } - } + }, + + plugins: settings.dev_server.hmr ? [ + new webpack.HotModuleReplacementPlugin(), + new webpack.NamedModulesPlugin() + ] : [] }) diff --git a/lib/install/config/webpack/shared.js b/lib/install/config/webpack/shared.js index 6feacb716..244efd65e 100644 --- a/lib/install/config/webpack/shared.js +++ b/lib/install/config/webpack/shared.js @@ -27,8 +27,7 @@ module.exports = { output: { filename: '[name].js', - path: output.path, - publicPath: output.publicPath + path: output.path }, module: { @@ -38,10 +37,7 @@ module.exports = { plugins: [ new webpack.EnvironmentPlugin(JSON.parse(JSON.stringify(env))), new ExtractTextPlugin(env.NODE_ENV === 'production' ? '[name]-[contenthash].css' : '[name].css'), - new ManifestPlugin({ - publicPath: output.publicPath, - writeToFileEmit: true - }) + new ManifestPlugin({ writeToFileEmit: true }) ], resolve: { diff --git a/lib/install/config/webpacker.yml b/lib/install/config/webpacker.yml index 8fcb01506..00c6e80ed 100644 --- a/lib/install/config/webpacker.yml +++ b/lib/install/config/webpacker.yml @@ -33,6 +33,7 @@ development: host: localhost port: 8080 https: false + hmr: false test: <<: *default diff --git a/lib/webpacker.rb b/lib/webpacker.rb index 958eff25d..bdb4f134f 100644 --- a/lib/webpacker.rb +++ b/lib/webpacker.rb @@ -20,6 +20,7 @@ def env require "webpacker/logger" require "webpacker/env" require "webpacker/configuration" +require "webpacker/dev_server" require "webpacker/manifest" require "webpacker/compiler" require "webpacker/railtie" if defined?(Rails) diff --git a/lib/webpacker/compiler.rb b/lib/webpacker/compiler.rb index d85775e61..bf8eb9dc3 100644 --- a/lib/webpacker/compiler.rb +++ b/lib/webpacker/compiler.rb @@ -1,6 +1,4 @@ require "open3" -require "webpacker/env" -require "webpacker/configuration" module Webpacker::Compiler extend self @@ -13,6 +11,8 @@ module Webpacker::Compiler mattr_accessor(:watched_paths) { [] } def compile + return if Webpacker::DevServer.running? + if stale? cache_source_timestamp run_webpack diff --git a/lib/webpacker/configuration.rb b/lib/webpacker/configuration.rb index c04ce8bdd..5188966d3 100644 --- a/lib/webpacker/configuration.rb +++ b/lib/webpacker/configuration.rb @@ -8,8 +8,12 @@ def entry_path source_path.join(fetch(:source_entry_path)) end + def public_output_path + fetch(:public_output_path) + end + def output_path - public_path.join(fetch(:public_output_path)) + public_path.join(public_output_path) end def manifest_path @@ -61,7 +65,7 @@ def defaults private def load - return super unless File.exist?(@path) + return Webpacker::Configuration.defaults unless File.exist?(@path) HashWithIndifferentAccess.new(YAML.load(File.read(@path))[Webpacker.env]) end end diff --git a/lib/webpacker/dev_server.rb b/lib/webpacker/dev_server.rb new file mode 100644 index 000000000..0e72395c2 --- /dev/null +++ b/lib/webpacker/dev_server.rb @@ -0,0 +1,47 @@ +module Webpacker::DevServer + extend self + + def running? + socket = Socket.tcp(host, port, connect_timeout: 1) + socket.close + true + + rescue Errno::ECONNREFUSED, NoMethodError + false + end + + def hmr? + case ENV["WEBPACKER_HMR"] + when /true/i then true + when /false/i then false + else fetch(:hmr) + end + + rescue NoMethodError + false + end + + def host + fetch(:host) + end + + def port + fetch(:port) + end + + def https? + fetch(:https) + end + + def protocol + https? ? "https" : "http" + end + + def base_url + "#{protocol}://#{host}:#{port}" + end + + def fetch(key) + Webpacker::Configuration.data[:dev_server][key] || Webpacker::Configuration.defaults[:dev_server][key] + end +end diff --git a/lib/webpacker/file_loader.rb b/lib/webpacker/file_loader.rb index 42eca50a0..44c8915c0 100644 --- a/lib/webpacker/file_loader.rb +++ b/lib/webpacker/file_loader.rb @@ -4,13 +4,29 @@ class NotFoundError < StandardError; end class FileLoaderError < StandardError; end class_attribute :instance - attr_accessor :data + attr_accessor :data, :mtime, :path class << self def load(path = file_path) - self.instance = new(path) + if instance.nil? || !production_env? || !file_cached?(path) + self.instance = new(path) + end + end + + def file_path + raise FileLoaderError.new("Subclass of Webpacker::FileLoader should override this method") end + private + def file_cached?(path) + File.exist?(path) && self.instance.mtime == File.mtime(path) + end + + # Prefer the NODE_ENV to the rails env. + def production_env? + (ENV["NODE_ENV"].presence || Rails.env) == "production" + end + protected def ensure_loaded_instance(klass) raise Webpacker::FileLoader::FileLoaderError.new("#{klass.name}#load must be called first") unless instance @@ -20,6 +36,7 @@ def ensure_loaded_instance(klass) private def initialize(path) @path = path + @mtime = File.exist?(path) ? File.mtime(path) : nil @data = load end diff --git a/lib/webpacker/helper.rb b/lib/webpacker/helper.rb index 0034ed099..ddb492ecc 100644 --- a/lib/webpacker/helper.rb +++ b/lib/webpacker/helper.rb @@ -9,8 +9,9 @@ module Webpacker::Helper # In production mode: # <%= asset_pack_path 'calendar.css' %> # => "/packs/calendar-1016838bab065ae1e122.css" def asset_pack_path(name, **options) - asset_path(Webpacker::Manifest.lookup(name), **options) + asset_path(Webpacker::Manifest.pack_path(name), **options) end + # Creates a script tag that references the named pack file, as compiled by Webpack per the entries list # 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. @@ -41,13 +42,21 @@ def javascript_pack_tag(*names, **options) # # In production mode: # <%= stylesheet_pack_tag 'calendar', 'data-turbolinks-track': 'reload' %> # => # + # # In development mode with hot-reloading + # <%= stylesheet_pack_tag('main') %> <% # Default is false for enabled_when_hot_loading%> + # # No output + # def stylesheet_pack_tag(*names, **options) - stylesheet_link_tag(*sources_from_pack_manifest(names, type: :stylesheet), **options) + if Webpacker::DevServer.hmr? + "" + else + stylesheet_link_tag(*sources_from_pack_manifest(names, type: :stylesheet), **options) + end end private def sources_from_pack_manifest(names, type:) - names.map { |name| Webpacker::Manifest.lookup(pack_name_with_extension(name, type: type)) } + names.map { |name| Webpacker::Manifest.pack_path(pack_name_with_extension(name, type: type)) } end def pack_name_with_extension(name, type:) diff --git a/lib/webpacker/manifest.rb b/lib/webpacker/manifest.rb index 015b65f7f..fd51f362e 100644 --- a/lib/webpacker/manifest.rb +++ b/lib/webpacker/manifest.rb @@ -17,31 +17,90 @@ def file_path Webpacker::Configuration.manifest_path end - def lookup(name) + # Converts the "name" (aka the pack or bundle name) to to the full path for use in a browser. + def pack_path(name) + relative_pack_path = "#{Webpacker::Configuration.public_output_path}/#{lookup(name)}" + + if Webpacker::DevServer.running? + "#{Webpacker::DevServer.base_url}/#{relative_pack_path}" + else + relative_pack_path.starts_with?("/") ? relative_pack_path : "/#{relative_pack_path}" + end + end + + # Converts the "name" (aka the pack or bundle name) to the possibly hashed name per a manifest. + # + # If Configuration.compile? then compilation is invoked the file is missing. + # + # Options + # throw_if_missing: default is true. If false, then nill is returned if the file is missing. + def lookup(name, throw_if_missing: true) + instance.confirm_manifest_exists + if Webpacker::Configuration.compile? - compile_and_find!(name) + compile_and_find!(name, throw_if_missing: throw_if_missing) else - find!(name) + # Since load checks a `mtime` on the manifest for a non-production env before loading, + # we should always call this before a call to `find!` since one may be using + # `webpack -w` and a file may have been added to the manifest since Rails first started. + load + raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::Manifest.load must be called first") unless instance + + return find!(name, throw_if_missing: throw_if_missing) end end + # Why does this method exist? Testing? It's not in the README def lookup_path(name) - Rails.root.join(File.join(Webpacker::Configuration.public_path, lookup(name))) + Rails.root.join(File.join(Webpacker::Configuration.output_path, lookup(name))) end private - def find!(name) + def missing_file_from_manifest_error(bundle_name) + msg = <<-MSG +Webpacker can't find #{bundle_name} in your manifest at #{file_path}. Possible causes: + 1. You are hot reloading. + 2. You want to set Configuration.compile to true for your environment. + 3. Webpack has not re-run to reflect updates. + 4. You have misconfigured Webpacker's config/webpacker.yml file. + 5. Your Webpack configuration is not creating a manifest. +Your manifest contains: +#{instance.data.to_json} + MSG + raise(Webpacker::FileLoader::NotFoundError.new(msg)) + end + + def missing_manifest_file_error(path_object) + msg = <<-MSG +Webpacker can't find the manifest file: #{path_object} +Possible causes: + 1. You have not invoked webpack. + 2. You have misconfigured Webpacker's config/webpacker_.yml file. + 3. Your Webpack configuration is not creating a manifest. +MSG + raise(Webpacker::FileLoader::NotFoundError.new(msg)) + end + + def find!(name, throw_if_missing: true) ensure_loaded_instance(self) - instance.data[name.to_s] || - raise(Webpacker::FileLoader::NotFoundError.new("Can't find #{name} in #{file_path}. Is webpack still compiling?")) + value = instance.data[name.to_s] + return value if value.present? + + if throw_if_missing + missing_file_from_manifest_error(name) + end end - def compile_and_find!(name) + def compile_and_find!(name, throw_if_missing: true) Webpacker.logger.tagged("Webpacker") { Webpacker.compile } - find!(name) + find!(name, throw_if_missing: throw_if_missing) end end + def confirm_manifest_exists + raise missing_manifest_file_error(@path) unless File.exist?(@path) + end + private def load return super unless File.exist?(@path) diff --git a/test/compiler_test.rb b/test/compiler_test.rb index 3059ae6ac..979c5fa67 100644 --- a/test/compiler_test.rb +++ b/test/compiler_test.rb @@ -16,7 +16,7 @@ def test_watched_paths end def test_freshness - assert Webpacker::Compiler.stale? - assert !Webpacker::Compiler.fresh? + refute Webpacker::Compiler.stale? + refute !Webpacker::Compiler.fresh? end end diff --git a/test/configuration_test.rb b/test/configuration_test.rb index dc1d96f6c..bb91a64c1 100644 --- a/test/configuration_test.rb +++ b/test/configuration_test.rb @@ -3,31 +3,31 @@ class ConfigurationTest < Minitest::Test def test_entry_path entry_path = File.join(File.dirname(__FILE__), "test_app/app/javascript", "packs").to_s - assert_equal Webpacker::Configuration.entry_path.to_s, entry_path + assert_equal entry_path, Webpacker::Configuration.entry_path.to_s end def test_file_path file_path = File.join(File.dirname(__FILE__), "test_app/config", "webpacker.yml").to_s - assert_equal Webpacker::Configuration.file_path.to_s, file_path + assert_equal file_path, Webpacker::Configuration.file_path.to_s end def test_manifest_path manifest_path = File.join(File.dirname(__FILE__), "test_app/public/packs", "manifest.json").to_s - assert_equal Webpacker::Configuration.manifest_path.to_s, manifest_path + assert_equal manifest_path, Webpacker::Configuration.manifest_path.to_s end def test_output_path output_path = File.join(File.dirname(__FILE__), "test_app/public/packs").to_s - assert_equal Webpacker::Configuration.output_path.to_s, output_path + assert_equal output_path, Webpacker::Configuration.output_path.to_s end def test_source - assert_equal Webpacker::Configuration.source.to_s, "app/javascript" + assert_equal "app/javascript", Webpacker::Configuration.source.to_s end def test_source_path source_path = File.join(File.dirname(__FILE__), "test_app/app/javascript").to_s - assert_equal Webpacker::Configuration.source_path.to_s, source_path + assert_equal source_path, Webpacker::Configuration.source_path.to_s end def test_cache_path diff --git a/test/dev_server_test.rb b/test/dev_server_test.rb new file mode 100644 index 000000000..260a69241 --- /dev/null +++ b/test/dev_server_test.rb @@ -0,0 +1,61 @@ +require "webpacker_test" + +class DevServerTest < Minitest::Test + require "webpacker_test" + + def reset + Webpacker::Configuration.instance_variable_set(:@defaults, nil) + Webpacker::Configuration.instance_variable_set(:@instance, nil) + end + + def check_assertion + reset + stub_value = ActiveSupport::StringInquirer.new("development") + Webpacker.stub(:env, stub_value) do + Webpacker::Configuration.stub(:data, dev_server: {}) do + result = yield + assert_equal(result[0], result[1]) + end + end + reset + end + + def test_dev_server? + check_assertion { [false, Webpacker::DevServer.running?] } + end + + def test_dev_server_host + check_assertion { ["localhost", Webpacker::DevServer.host] } + end + + def test_dev_server_port + check_assertion { [8080, Webpacker::DevServer.port] } + end + + def test_dev_server_hmr? + check_assertion { [false, Webpacker::DevServer.hmr?] } + + ENV.stub(:[], "TRUE") do + check_assertion { [true, Webpacker::DevServer.hmr?] } + end + + ENV.stub(:[], "FALSE") do + check_assertion { [false, Webpacker::DevServer.hmr?] } + end + ENV.stub(:[], "true") do + check_assertion { [true, Webpacker::DevServer.hmr?] } + end + end + + def test_dev_server_https? + check_assertion { [false, Webpacker::DevServer.https?] } + end + + def test_dev_server_protocol? + check_assertion { ["http", Webpacker::DevServer.protocol] } + end + + def test_base_url? + check_assertion { ["http://localhost:8080", Webpacker::DevServer.base_url] } + end +end diff --git a/test/helper_test.rb b/test/helper_test.rb index 527968b24..2fe0e3a0e 100644 --- a/test/helper_test.rb +++ b/test/helper_test.rb @@ -7,29 +7,35 @@ def setup end def test_asset_pack_path - assert_equal @view.asset_pack_path("bootstrap.js"), "/packs/bootstrap-300631c4f0e0f9c865bc.js" - assert_equal @view.asset_pack_path("bootstrap.css"), "/packs/bootstrap-c38deda30895059837cf.css" + assert_equal "/packs/bootstrap-300631c4f0e0f9c865bc.js", @view.asset_pack_path("bootstrap.js") + assert_equal "/packs/bootstrap-c38deda30895059837cf.css", @view.asset_pack_path("bootstrap.css") end def test_javascript_pack_tag script = %() - assert_equal @view.javascript_pack_tag("bootstrap.js"), script + assert_equal script, @view.javascript_pack_tag("bootstrap.js") end def test_javascript_pack_tag_splat script = %(\n) + %() - assert_equal @view.javascript_pack_tag("bootstrap.js", "application.js", defer: true), script + assert_equal script, @view.javascript_pack_tag("bootstrap.js", "application.js", defer: true) end def test_stylesheet_pack_tag style = %() - assert_equal @view.stylesheet_pack_tag("bootstrap.css"), style + assert_equal style, @view.stylesheet_pack_tag("bootstrap.css") end def test_stylesheet_pack_tag_splat style = %(\n) + %() - assert_equal @view.stylesheet_pack_tag("bootstrap.css", "application.css", media: "all"), style + assert_equal style, @view.stylesheet_pack_tag("bootstrap.css", "application.css", media: "all") + end + + def test_stylesheet_pack_tag_outputs_nothing_for_hot + Webpacker::DevServer.stub(:hmr?, true) do + assert_equal "", @view.stylesheet_pack_tag("bootstrap.css") + end end end diff --git a/test/manifest_test.rb b/test/manifest_test.rb index 528064800..eb0ce1cb3 100644 --- a/test/manifest_test.rb +++ b/test/manifest_test.rb @@ -3,30 +3,82 @@ class ManifestTest < Minitest::Test def test_file_path file_path = File.join(File.dirname(__FILE__), "test_app/public/packs", "manifest.json").to_s - assert_equal Webpacker::Manifest.file_path.to_s, file_path + assert_equal(Webpacker::Manifest.file_path.to_s, file_path) end def test_lookup_exception manifest_path = File.join(File.dirname(__FILE__), "test_app/public/packs", "manifest.json").to_s asset_file = "calendar.js" + msg = <<-MSG +Webpacker can't find #{asset_file} in your manifest at #{manifest_path}. Possible causes: + 1. You are hot reloading. + 2. You want to set Configuration.compile to true for your environment. + 3. Webpack has not re-run to reflect updates. + 4. You have misconfigured Webpacker's config/webpacker.yml file. + 5. Your Webpack configuration is not creating a manifest. +Your manifest contains: +#{Webpacker::Manifest.instance.data.to_json} + MSG Webpacker::Configuration.stub :compile?, false do error = assert_raises Webpacker::FileLoader::NotFoundError do Webpacker::Manifest.lookup(asset_file) end - - assert_equal error.message, "Can't find #{asset_file} in #{manifest_path}. Is webpack still compiling?" + assert_equal(msg, error.message) end end def test_lookup_success asset_file = "bootstrap.js" - assert_equal Webpacker::Manifest.lookup(asset_file), "/packs/bootstrap-300631c4f0e0f9c865bc.js" + assert_equal("bootstrap-300631c4f0e0f9c865bc.js", Webpacker::Manifest.lookup(asset_file)) end def test_lookup_path file_path = File.join(File.dirname(__FILE__), "test_app/public/packs", "bootstrap-300631c4f0e0f9c865bc.js").to_s asset_file = "bootstrap.js" - assert_equal Webpacker::Manifest.lookup_path(asset_file).to_s, file_path + assert_equal(file_path, Webpacker::Manifest.lookup_path(asset_file).to_s) + end + + def test_file_not_existing + begin + file_path = File.join(File.dirname(__FILE__), "test_app/public/packs", "manifest.json") + temp_path = "#{file_path}.backup" + FileUtils.mv(file_path, temp_path) + # Point of this test is to ensure no crash + Webpacker::Manifest.load + assert_equal({}, Webpacker::Manifest.instance.data) + ensure + FileUtils.mv(temp_path, file_path) + Webpacker::Manifest.instance = nil + Webpacker::Manifest.load + end + end + + def test_lookup_success + asset_file = "bootstrap.js" + assert_equal("bootstrap-300631c4f0e0f9c865bc.js", Webpacker::Manifest.lookup(asset_file)) + end + + def test_confirm_manifest_exists_for_existing + Webpacker::Manifest.instance.confirm_manifest_exists + end + + def test_confirm_manifest_exists_for_missing + assert_raises do + Webpacker::Manifest.stub :path, "non-existent-file" do + confirm_manifest_exists + end + end + end + + def test_lookup_no_throw_missing_file + value = Webpacker::Manifest.lookup("non-existent-bundle.js", throw_if_missing: false) + assert_nil(value) + end + + def test_lookup_no_throw_file_exists + file_path = "bootstrap-300631c4f0e0f9c865bc.js" + asset_file = "bootstrap.js" + assert_equal(file_path, Webpacker::Manifest.lookup(asset_file, throw_if_missing: false)) end end diff --git a/test/test_app/public/packs/manifest.json b/test/test_app/public/packs/manifest.json index f7b77dd63..a5b6cb8e5 100644 --- a/test/test_app/public/packs/manifest.json +++ b/test/test_app/public/packs/manifest.json @@ -1,6 +1,6 @@ { - "bootstrap.css": "/packs/bootstrap-c38deda30895059837cf.css", - "application.css": "/packs/application-dd6b1cd38bfa093df600.css", - "bootstrap.js": "/packs/bootstrap-300631c4f0e0f9c865bc.js", - "application.js": "/packs/application-k344a6d59eef8632c9d1.js" + "bootstrap.css": "bootstrap-c38deda30895059837cf.css", + "application.css": "application-dd6b1cd38bfa093df600.css", + "bootstrap.js": "bootstrap-300631c4f0e0f9c865bc.js", + "application.js": "application-k344a6d59eef8632c9d1.js" }