From afe9c996b6884fd35d7fc1f3f83078b1df409ff8 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Thu, 27 Jul 2017 23:39:53 -1000 Subject: [PATCH 01/16] Support React on Rails by merging Webpacker Lite See https://github.com/rails/webpacker/issues/594 From a long discussion on #464, this issue will summarize. I'll soon be posting proposed changes to the README.md. Summary of changes * Move base url out from manifest.json to manifest.rb * Assign env variables to dev server settings so it can be overridden at runtime. * The keys for dev_server should use same format as of now as documented in Paths on the README.md. Note that * hot is a new setting to indicate that the dev_server is used with hot reloading, which means that CSS should be inlined to be hot loaded. The presence of dev_server means that the webpack-dev-server is used for the given env. development: // put the created files to the /public/webpack/development directory public_output_path: webpack/development //# if dev_server is not provided, then dev_server is not used dev_server: hot: true # This is a new setting static: false host: localhost https: false --- Gemfile | 10 +++ Gemfile.lock | 36 ++++++++++- README.md | 16 ++++- lib/install/config/webpack/shared.js | 4 +- lib/install/config/webpacker.yml | 1 + lib/webpacker.rb | 10 +-- lib/webpacker/configuration.rb | 30 +++++++-- lib/webpacker/dev_server.rb | 78 ++++++++++++++++++++++++ lib/webpacker/env.rb | 2 +- lib/webpacker/file_loader.rb | 31 ++++++++-- lib/webpacker/helper.rb | 30 +++++++-- lib/webpacker/manifest.rb | 75 ++++++++++++++++++++--- test/configuration_test.rb | 12 ++-- test/dev_server_test.rb | 56 +++++++++++++++++ test/helper_test.rb | 22 +++++-- test/manifest_test.rb | 59 ++++++++++++++++-- test/test_app/public/packs/manifest.json | 8 +-- test/webpacker_test.rb | 5 ++ 18 files changed, 431 insertions(+), 54 deletions(-) create mode 100644 lib/webpacker/dev_server.rb create mode 100644 test/dev_server_test.rb diff --git a/Gemfile b/Gemfile index 1f00b8752..532cca3f4 100644 --- a/Gemfile +++ b/Gemfile @@ -9,3 +9,13 @@ gem "rubocop", ">= 0.47", require: false group :test do gem "minitest", "~> 5.0" end + +if ENV["USE_PRY"] + gem "awesome_print" + gem "pry" + gem "pry-byebug" + gem "pry-doc" + gem "pry-rails" + gem "pry-rescue" + gem "pry-stack_explorer" +end diff --git a/Gemfile.lock b/Gemfile.lock index fb75154fa..f49deed5b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -48,12 +48,19 @@ GEM tzinfo (~> 1.1) arel (7.1.4) ast (2.3.0) + awesome_print (1.8.0) + binding_of_caller (0.7.2) + debug_inspector (>= 0.0.1) builder (3.2.3) + byebug (9.0.6) + coderay (1.1.1) concurrent-ruby (1.0.5) + debug_inspector (0.0.3) erubis (2.7.0) globalid (0.3.7) activesupport (>= 4.1.0) i18n (0.8.1) + interception (0.5) loofah (2.0.3) nokogiri (>= 1.5.9) mail (2.6.4) @@ -71,6 +78,24 @@ GEM parser (2.4.0.0) ast (~> 2.2) powerpack (0.1.1) + pry (0.10.4) + coderay (~> 1.1.0) + method_source (~> 0.8.1) + slop (~> 3.4) + pry-byebug (3.4.2) + byebug (~> 9.0) + pry (~> 0.10) + pry-doc (0.10.0) + pry (~> 0.9) + yard (~> 0.9) + pry-rails (0.3.6) + pry (>= 0.10.4) + pry-rescue (1.4.5) + interception (>= 0.5) + pry + pry-stack_explorer (0.4.9.2) + binding_of_caller (>= 0.7) + pry (>= 0.9.11) rack (2.0.1) rack-test (0.6.3) rack (>= 1.0) @@ -106,6 +131,7 @@ GEM ruby-progressbar (~> 1.7) unicode-display_width (~> 1.0, >= 1.0.1) ruby-progressbar (1.8.1) + slop (3.6.0) sprockets (3.7.1) concurrent-ruby (~> 1.0) rack (> 1, < 3) @@ -121,17 +147,25 @@ GEM websocket-driver (0.6.5) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.2) + yard (0.9.9) PLATFORMS ruby DEPENDENCIES + awesome_print bundler (~> 1.12) minitest (~> 5.0) + pry + pry-byebug + pry-doc + pry-rails + pry-rescue + pry-stack_explorer rails rake (>= 11.1) rubocop (>= 0.47) webpacker! BUNDLED WITH - 1.14.6 + 1.15.3 diff --git a/README.md b/README.md index 60c079ff6..2433bd2fd 100644 --- a/README.md +++ b/README.md @@ -367,8 +367,14 @@ development: host: 0.0.0.0 port: 8080 https: false + hot: false ``` +If you have hot 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 @@ -601,8 +607,12 @@ 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: +The most widely used React integration is [shakacode/react_on_rails](https://github.com/shakacode/react_on_rails) which includes support for server rendering, redux and react-router. + +Other alternatives include [react-rails](https://github.com/reactjs/react-rails) and +[webpacker-react](https://github.com/renchap/webpacker-react) for more advanced react integration. + +If you're not concerned with view helpers to pass props or server rendering, can do it yourself: ```erb <%# views/layouts/application.html.erb %> @@ -1103,6 +1113,8 @@ git push heroku master Webpacker lazily compiles assets in test env so you can write your tests without any extra setup and everything will just work out of the box. +Note, [React on Rails] users should set configuration value `compile` to false, as React on Rails +handle compilation for test and production environments. Here is a sample system test case with hello_react example component: diff --git a/lib/install/config/webpack/shared.js b/lib/install/config/webpack/shared.js index 45abc33b6..ca1e60c42 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: { @@ -39,7 +38,6 @@ module.exports = { new webpack.EnvironmentPlugin(JSON.parse(JSON.stringify(env))), new ExtractTextPlugin(env.NODE_ENV === 'production' ? '[name]-[hash].css' : '[name].css'), new ManifestPlugin({ - publicPath: output.publicPath, writeToFileEmit: true }) ], diff --git a/lib/install/config/webpacker.yml b/lib/install/config/webpacker.yml index b670ae4cd..15df260f7 100644 --- a/lib/install/config/webpacker.yml +++ b/lib/install/config/webpacker.yml @@ -34,6 +34,7 @@ development: host: localhost port: 8080 https: false + hot: false test: <<: *default diff --git a/lib/webpacker.rb b/lib/webpacker.rb index 2a2791634..db55fe0bf 100644 --- a/lib/webpacker.rb +++ b/lib/webpacker.rb @@ -2,14 +2,15 @@ module Webpacker extend self def bootstrap - Webpacker::Env.load - Webpacker::Configuration.load - Webpacker::Manifest.load + Webpacker::Env.load_instance + Webpacker::Configuration.load_instance + Webpacker::DevServer.load_instance + Webpacker::Manifest.load_instance end def compile Webpacker::Compiler.compile - Webpacker::Manifest.load + Webpacker::Manifest.load_instance end def env @@ -19,6 +20,7 @@ def env 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/configuration.rb b/lib/webpacker/configuration.rb index b20808d9e..edd5e102a 100644 --- a/lib/webpacker/configuration.rb +++ b/lib/webpacker/configuration.rb @@ -4,12 +4,21 @@ class Webpacker::Configuration < Webpacker::FileLoader class << self + def reset + @defaults = nil + super + end + 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 @@ -45,19 +54,30 @@ def fetch(key) end def data - load if Webpacker.env.development? - raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::Configuration.load must be called first") unless instance + load_instance if Webpacker.env.development? + raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::Configuration.load_data must be called first") unless instance instance.data end def defaults @defaults ||= HashWithIndifferentAccess.new(YAML.load(default_file_path.read)[Webpacker.env]) end + + # Uses the webpack dev server host if appropriate + def output_path_or_url + if Webpacker::DevServer.dev_server? + Webpacker::DevServer.base_url + else + # Ensure we start with a slash so that the asset helpers don't prepend the default asset + # pipeline locations. + public_output_path.starts_with?("/") ? public_output_path : "/#{public_output_path}" + end + end end private - def load - return super unless File.exist?(@path) + def load_data + 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..ceb931026 --- /dev/null +++ b/lib/webpacker/dev_server.rb @@ -0,0 +1,78 @@ +# Same convention as manifest/configuration.rb + +# Loads webpacker configuration from config/webpacker.yml + +require "webpacker/configuration" + +class Webpacker::DevServer < Webpacker::FileLoader + class << self + def dev_server? + !dev_server_values.nil? + end + + # read settings for dev_server + def hot? + return false unless dev_server? + if ENV["WEBPACKER_HMR"].present? + val = ENV["WEBPACKER_HMR"].downcase + return true if val == "true" + return false if val == "false" + raise new ArgumentError("WEBPACKER_HMR value is #{ENV['WEBPACKER_HMR']}. Set to TRUE|FALSE") + end + fetch(:hot) + end + + def host + fetch(:host) + end + + def port + fetch(:port) + end + + def https? + fetch(:https) + end + + def protocol + https? ? "https" : "http" + end + + def file_path + Webpacker::Configuration.file_path + end + + # Uses the hot_reloading_host if appropriate + def base_url + "#{protocol}://#{host}:#{port}" + end + + private + + def dev_server_values + data.fetch(:dev_server, nil) + end + + def fetch(key) + return nil unless dev_server? + dev_server_values.fetch(key, dev_server_defaults[key]) + end + + def data + load_instance if Webpacker.env.development? + unless instance + raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::DevServer.load_data must be called first") + end + instance.data + end + + def dev_server_defaults + @defaults ||= Webpacker::Configuration.defaults[:dev_server] + end + end + + private + def load_data + Webpacker::Configuration.instance.data + end +end diff --git a/lib/webpacker/env.rb b/lib/webpacker/env.rb index f7a385e37..1f276e3bb 100644 --- a/lib/webpacker/env.rb +++ b/lib/webpacker/env.rb @@ -14,7 +14,7 @@ def file_path end private - def load + def load_data environments = File.exist?(@path) ? YAML.load(File.read(@path)).keys : [].freeze return ENV["NODE_ENV"] if environments.include?(ENV["NODE_ENV"]) return Rails.env if environments.include?(Rails.env) diff --git a/lib/webpacker/file_loader.rb b/lib/webpacker/file_loader.rb index 8d478420b..ea01f7016 100644 --- a/lib/webpacker/file_loader.rb +++ b/lib/webpacker/file_loader.rb @@ -4,21 +4,44 @@ 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) + def load_instance(path = file_path) + # Assume production is 100% cached and don't reload if file's mtime not changed + cached = self.instance && # if we have a singleton + (env == "production" || # skip if production bc always cached + (File.exist?(path) && self.instance.mtime == File.mtime(path))) # skip if mtime not changed + + return if cached self.instance = new(path) end + + def file_path + raise FileLoaderError.new("Subclass of Webpacker::FileLoader should override this method") + end + + def reset + self.instance = nil + load_instance + end + + private + + # Prefer the NODE_ENV to the rails env. + def env + ENV["NODE_ENV"].presence || Rails.env + end end private def initialize(path) @path = path - @data = load + @mtime = File.exist?(path) ? File.mtime(path) : nil + @data = load_data end - def load + def load_data {}.freeze end end diff --git a/lib/webpacker/helper.rb b/lib/webpacker/helper.rb index e90997e41..f707e25f1 100644 --- a/lib/webpacker/helper.rb +++ b/lib/webpacker/helper.rb @@ -9,8 +9,12 @@ 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) + path = Webpacker::Configuration.public_output_path + file = Webpacker::Manifest.lookup(name) + full_path = "#{path}/#{file}" + asset_path(full_path, **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. @@ -25,8 +29,7 @@ def asset_pack_path(name, **options) # <%= javascript_pack_tag 'calendar', 'data-turbolinks-track': 'reload' %> # => # def javascript_pack_tag(*names, **options) - sources = names.map { |name| Webpacker::Manifest.lookup("#{name}#{compute_asset_extname(name, type: :javascript)}") } - javascript_include_tag(*sources, **options) + javascript_include_tag(*asset_source(names, :javascript), **options) end # Creates a link tag that references the named pack file, as compiled by Webpack per the entries list @@ -42,8 +45,25 @@ 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 + # + # # In development mode with hot-reloading and enabled_when_hot_loading + # # <%= stylesheet_pack_tag('main', enabled_when_hot_loading: true) %> + # + # def stylesheet_pack_tag(*names, **options) - sources = names.map { |name| Webpacker::Manifest.lookup("#{name}#{compute_asset_extname(name, type: :stylesheet)}") } - stylesheet_link_tag(*sources, **options) + return "" if Webpacker::DevServer.hot? + stylesheet_link_tag(*asset_source(names, :stylesheet), **options) end + + private + def asset_source(names, type) + output_path_or_url = Webpacker::Configuration.output_path_or_url + names.map do |name| + path = Webpacker::Manifest.lookup("#{name}#{compute_asset_extname(name, type: type)}") + "#{output_path_or_url}/#{path}" + end + end end diff --git a/lib/webpacker/manifest.rb b/lib/webpacker/manifest.rb index bcd1d9710..b990b3d76 100644 --- a/lib/webpacker/manifest.rb +++ b/lib/webpacker/manifest.rb @@ -13,6 +13,10 @@ def file_path Webpacker::Configuration.manifest_path end + # Throws an error if the file is not found. If Configuration.compile? then compilation is invoked + # the file is missing. + # React on Rails users will need to set Configuration.compile? to false as compilation is configured + # in the package.json for React on Rails. def lookup(name) if Webpacker::Configuration.compile? compile_and_find!(name) @@ -21,24 +25,77 @@ def lookup(name) 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) - raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::Manifest.load must be called first") unless instance - instance.data[name.to_s] || raise(Webpacker::FileLoader::NotFoundError.new("Can't find #{name} in #{file_path}. Is webpack still compiling?")) + # Helper method to determine if the manifest file exists. Maybe Webpack needs to run? + # **Used by React on Rails.** + def exist? + path_object = Webpacker::Configuration.manifest_path + path_object.exist? + end + + # Find the real file name from the manifest key. Don't throw an error if the file is simply + # missing from the manifest. Return nil in that case. + # If no manifest file exists, then throw an error. + # **Used by React on Rails.** + def lookup_path_no_throw(name) + instance.confirm_manifest_exists + load_instance + unless instance + raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::Manifest.load must be called first") end + hashed_name = instance.data[name.to_s] + return hashed_name if hashed_name.blank? + Rails.root.join(File.join(Webpacker::Configuration.output_path, hashed_name)) + end - def compile_and_find!(name) - Webpacker.compile - find!(name) + private + def find!(name) + unless instance + raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::Manifest.load must be called first") end + instance.data[name.to_s] || missing_file_from_manifest_error(name) + end + + 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. + 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 compile_and_find!(name) + Webpacker.compile + find!(name) + end + end + + def confirm_manifest_exists + raise missing_manifest_file_error(@path) unless File.exist?(@path) end private - def load + + def load_data return super unless File.exist?(@path) JSON.parse(File.read(@path)) end diff --git a/test/configuration_test.rb b/test/configuration_test.rb index c6d55da5a..f2b23cdee 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_compile? diff --git a/test/dev_server_test.rb b/test/dev_server_test.rb new file mode 100644 index 000000000..5c59dacbb --- /dev/null +++ b/test/dev_server_test.rb @@ -0,0 +1,56 @@ +require "webpacker_test" + +class DevServerTest < Minitest::Test + require "webpacker_test" + + def check_assertion + stub_value = ActiveSupport::StringInquirer.new("development") + Webpacker.stub(:env, stub_value) do + Webpacker::Configuration.reset + Webpacker::DevServer.reset + result = yield + assert_equal(result[0], result[1]) + end + Webpacker::Configuration.reset + Webpacker::DevServer.reset + end + + def test_dev_server? + check_assertion { [true, Webpacker::DevServer.dev_server?] } + 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_hot? + check_assertion { [false, Webpacker::DevServer.hot?] } + + ENV.stub(:[], "TRUE") do + check_assertion { [true, Webpacker::DevServer.hot?] } + end + + ENV.stub(:[], "FALSE") do + check_assertion { [false, Webpacker::DevServer.hot?] } + end + ENV.stub(:[], "true") do + check_assertion { [true, Webpacker::DevServer.hot?] } + 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..1215581e6 100644 --- a/test/helper_test.rb +++ b/test/helper_test.rb @@ -7,29 +7,39 @@ 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(:hot?, true) do + # Webpacker::Configuration.reset + # Webpacker::DevServer.reset + assert_equal "", @view.stylesheet_pack_tag("bootstrap.css") + end + # Webpacker::Configuration.reset + # Webpacker::DevServer.reset end end diff --git a/test/manifest_test.rb b/test/manifest_test.rb index 528064800..ff220b6a2 100644 --- a/test/manifest_test.rb +++ b/test/manifest_test.rb @@ -3,30 +3,81 @@ 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. + 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(error.message, msg) 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_instance + assert_equal({}, Webpacker::Manifest.instance.data) + ensure + FileUtils.mv(temp_path, file_path) + Webpacker::Manifest.instance = nil + Webpacker::Manifest.load_instance + 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_path_no_throw_missing_file + value = Webpacker::Manifest.lookup_path_no_throw("non-existent-bundle.js") + assert_nil(value) + end + + def test_lookup_path_no_throw_file_exists + file_path = File.join(File.dirname(__FILE__), "test_app/public/packs", "bootstrap-300631c4f0e0f9c865bc.js").to_s + asset_file = "bootstrap.js" + assert_equal(file_path, Webpacker::Manifest.lookup_path_no_throw(asset_file).to_s) 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" } diff --git a/test/webpacker_test.rb b/test/webpacker_test.rb index f05871147..02547fb18 100644 --- a/test/webpacker_test.rb +++ b/test/webpacker_test.rb @@ -5,6 +5,11 @@ require "rails/test_help" require "webpacker" +if ENV["USE_PRY"] + require "pry" + require "awesome_print" +end + module TestApp class Application < ::Rails::Application config.root = File.join(File.dirname(__FILE__), "test_app") From 41cda6a4bfe38af7e5a56fa00fe0ee4d466be9e0 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Tue, 1 Aug 2017 14:20:53 -1000 Subject: [PATCH 02/16] Need to called load_instance for manifest in find! Since load_instance is a no-op for production, this should be fine performance-wise. If we're using `webpack -w` to recompile when files changes, we need to call this before find! Otherwise, we don't get new files. So if we startup both a compile process and the server with a Procfile, then Webpacker will load with no values in the manifest, and it will not be read again. * In the case of using --- README.md | 4 ++++ lib/webpacker/manifest.rb | 30 ++++++++++++++++++------------ 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 2433bd2fd..a67ed8c8a 100644 --- a/README.md +++ b/README.md @@ -459,6 +459,10 @@ 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/hot` 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. + ## Linking Styles, Images and Fonts diff --git a/lib/webpacker/manifest.rb b/lib/webpacker/manifest.rb index b990b3d76..f1b9799a5 100644 --- a/lib/webpacker/manifest.rb +++ b/lib/webpacker/manifest.rb @@ -21,6 +21,10 @@ def lookup(name) if Webpacker::Configuration.compile? compile_and_find!(name) else + # Since load_instance is a no-op for production, this should be fine. + # If we're using `webpack -w` to recompile when files changes, we need + # to call this before find! + load_instance find!(name) end end @@ -62,24 +66,26 @@ 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. +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 +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 From b73fa8ed8cf7c7a656bea41de031972e27aa32a2 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Tue, 1 Aug 2017 15:11:50 -1000 Subject: [PATCH 03/16] Provide WEBPACKER_DEV_SERVER env override 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. --- README.md | 6 ++++++ lib/webpacker/dev_server.rb | 25 +++++++++++++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index a67ed8c8a..ccb68f735 100644 --- a/README.md +++ b/README.md @@ -463,6 +463,12 @@ If you are using hot reloading, you should either set the `dev_server/hot` key t 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 diff --git a/lib/webpacker/dev_server.rb b/lib/webpacker/dev_server.rb index ceb931026..c8128a7d3 100644 --- a/lib/webpacker/dev_server.rb +++ b/lib/webpacker/dev_server.rb @@ -7,18 +7,20 @@ class Webpacker::DevServer < Webpacker::FileLoader class << self def dev_server? + env_val = env_value("WEBPACKER_DEV_SERVER") + + # override dev_server setup WEBPACKER_DEV_SERVER=FALSE + return false if env_val == false + + # If not specified, then check if values in the config file for the dev_server key !dev_server_values.nil? end # read settings for dev_server def hot? return false unless dev_server? - if ENV["WEBPACKER_HMR"].present? - val = ENV["WEBPACKER_HMR"].downcase - return true if val == "true" - return false if val == "false" - raise new ArgumentError("WEBPACKER_HMR value is #{ENV['WEBPACKER_HMR']}. Set to TRUE|FALSE") - end + env_val = env_value("WEBPACKER_HMR") + return env_val unless env_val.nil? fetch(:hot) end @@ -49,6 +51,17 @@ def base_url private + def env_value(env_key) + if ENV[env_key].present? + val = ENV[env_key] + val_upcase = val.upcase + return true if val_upcase == "TRUE" + return false if val_upcase == "FALSE" + raise new ArgumentError("#{env_key} value is #{val}. Set to TRUE|FALSE") + end + # returns nil + end + def dev_server_values data.fetch(:dev_server, nil) end From c71bd6fc68f5798df5fa3aaf20edbb620a36c095 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sat, 5 Aug 2017 19:48:27 -1000 Subject: [PATCH 04/16] Remove PRY --- Gemfile | 10 ---------- test/webpacker_test.rb | 5 ----- 2 files changed, 15 deletions(-) diff --git a/Gemfile b/Gemfile index 532cca3f4..1f00b8752 100644 --- a/Gemfile +++ b/Gemfile @@ -9,13 +9,3 @@ gem "rubocop", ">= 0.47", require: false group :test do gem "minitest", "~> 5.0" end - -if ENV["USE_PRY"] - gem "awesome_print" - gem "pry" - gem "pry-byebug" - gem "pry-doc" - gem "pry-rails" - gem "pry-rescue" - gem "pry-stack_explorer" -end diff --git a/test/webpacker_test.rb b/test/webpacker_test.rb index 02547fb18..f05871147 100644 --- a/test/webpacker_test.rb +++ b/test/webpacker_test.rb @@ -5,11 +5,6 @@ require "rails/test_help" require "webpacker" -if ENV["USE_PRY"] - require "pry" - require "awesome_print" -end - module TestApp class Application < ::Rails::Application config.root = File.join(File.dirname(__FILE__), "test_app") From 4cf3fcab5c03203216be7ab0b7f161b591908174 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sat, 5 Aug 2017 21:50:56 -1000 Subject: [PATCH 05/16] Updates per review from dhh --- README.md | 13 ++--- lib/install/config/webpacker.yml | 2 +- lib/webpacker/configuration.rb | 7 +-- lib/webpacker/dev_server.rb | 66 +++++++++-------------- lib/webpacker/file_loader.rb | 28 ++++------ lib/webpacker/helper.rb | 12 +++-- lib/webpacker/manifest.rb | 91 ++++++++++++++++---------------- test/dev_server_test.rb | 30 +++++++---- test/helper_test.rb | 6 +-- test/manifest_test.rb | 27 +++++----- 10 files changed, 128 insertions(+), 154 deletions(-) diff --git a/README.md b/README.md index ccb68f735..3fac4b54c 100644 --- a/README.md +++ b/README.md @@ -367,10 +367,10 @@ development: host: 0.0.0.0 port: 8080 https: false - hot: false + hot_reloading: false ``` -If you have hot turned to `true`, then the `stylesheet_pack_tag` generates no output, as you will want +If you have hot_reloading 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. @@ -459,7 +459,7 @@ 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/hot` key to TRUE or set the +If you are using hot reloading, you should either set the `dev_server/hot_reloading` 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. @@ -617,10 +617,7 @@ and #### React -The most widely used React integration is [shakacode/react_on_rails](https://github.com/shakacode/react_on_rails) which includes support for server rendering, redux and react-router. - -Other alternatives include [react-rails](https://github.com/reactjs/react-rails) and -[webpacker-react](https://github.com/renchap/webpacker-react) for more advanced react integration. +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: @@ -1124,7 +1121,7 @@ Webpacker lazily compiles assets in test env so you can write your tests without setup and everything will just work out of the box. Note, [React on Rails] users should set configuration value `compile` to false, as React on Rails -handle compilation for test and production environments. +handles compilation for test and production environments. Here is a sample system test case with hello_react example component: diff --git a/lib/install/config/webpacker.yml b/lib/install/config/webpacker.yml index 15df260f7..f34c6b37f 100644 --- a/lib/install/config/webpacker.yml +++ b/lib/install/config/webpacker.yml @@ -34,7 +34,7 @@ development: host: localhost port: 8080 https: false - hot: false + hot_reloading: false test: <<: *default diff --git a/lib/webpacker/configuration.rb b/lib/webpacker/configuration.rb index edd5e102a..85daa9165 100644 --- a/lib/webpacker/configuration.rb +++ b/lib/webpacker/configuration.rb @@ -4,11 +4,6 @@ class Webpacker::Configuration < Webpacker::FileLoader class << self - def reset - @defaults = nil - super - end - def entry_path source_path.join(fetch(:source_entry_path)) end @@ -65,7 +60,7 @@ def defaults # Uses the webpack dev server host if appropriate def output_path_or_url - if Webpacker::DevServer.dev_server? + if Webpacker::DevServer.enabled? Webpacker::DevServer.base_url else # Ensure we start with a slash so that the asset helpers don't prepend the default asset diff --git a/lib/webpacker/dev_server.rb b/lib/webpacker/dev_server.rb index c8128a7d3..7d54cfa62 100644 --- a/lib/webpacker/dev_server.rb +++ b/lib/webpacker/dev_server.rb @@ -6,22 +6,25 @@ class Webpacker::DevServer < Webpacker::FileLoader class << self - def dev_server? - env_val = env_value("WEBPACKER_DEV_SERVER") - - # override dev_server setup WEBPACKER_DEV_SERVER=FALSE - return false if env_val == false - - # If not specified, then check if values in the config file for the dev_server key - !dev_server_values.nil? + def enabled? + case ENV["WEBPACKER_DEV_SERVER"] + when /true/i then true + when /false/i then false + else !data[:dev_server].nil? + end end # read settings for dev_server - def hot? - return false unless dev_server? - env_val = env_value("WEBPACKER_HMR") - return env_val unless env_val.nil? - fetch(:hot) + def hot_reloading? + if enabled? + case ENV["WEBPACKER_HMR"] + when /true/i then true + when /false/i then false + else fetch(:hot_reloading) + end + else + false + end end def host @@ -50,38 +53,17 @@ def base_url end private - - def env_value(env_key) - if ENV[env_key].present? - val = ENV[env_key] - val_upcase = val.upcase - return true if val_upcase == "TRUE" - return false if val_upcase == "FALSE" - raise new ArgumentError("#{env_key} value is #{val}. Set to TRUE|FALSE") + def fetch(key) + if enabled? + data[:dev_server][key] || Webpacker::Configuration.defaults[:dev_server][key] + end end - # returns nil - end - def dev_server_values - data.fetch(:dev_server, nil) - end - - def fetch(key) - return nil unless dev_server? - dev_server_values.fetch(key, dev_server_defaults[key]) - end - - def data - load_instance if Webpacker.env.development? - unless instance - raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::DevServer.load_data must be called first") + def data + load_instance if Webpacker.env.development? + raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::DevServer.load_data must be called first") unless instance + instance.data end - instance.data - end - - def dev_server_defaults - @defaults ||= Webpacker::Configuration.defaults[:dev_server] - end end private diff --git a/lib/webpacker/file_loader.rb b/lib/webpacker/file_loader.rb index ea01f7016..76fe0a03f 100644 --- a/lib/webpacker/file_loader.rb +++ b/lib/webpacker/file_loader.rb @@ -8,30 +8,24 @@ class FileLoaderError < StandardError; end class << self def load_instance(path = file_path) - # Assume production is 100% cached and don't reload if file's mtime not changed - cached = self.instance && # if we have a singleton - (env == "production" || # skip if production bc always cached - (File.exist?(path) && self.instance.mtime == File.mtime(path))) # skip if mtime not changed - - return if cached - 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 - def reset - self.instance = nil - load_instance - end - private - - # Prefer the NODE_ENV to the rails env. - def env - ENV["NODE_ENV"].presence || Rails.env - end + 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 end private diff --git a/lib/webpacker/helper.rb b/lib/webpacker/helper.rb index f707e25f1..ef8fa3d53 100644 --- a/lib/webpacker/helper.rb +++ b/lib/webpacker/helper.rb @@ -54,16 +54,18 @@ def javascript_pack_tag(*names, **options) # # def stylesheet_pack_tag(*names, **options) - return "" if Webpacker::DevServer.hot? - stylesheet_link_tag(*asset_source(names, :stylesheet), **options) + if Webpacker::DevServer.hot_reloading? + "" + else + stylesheet_link_tag(*asset_source(names, :stylesheet), **options) + end end private def asset_source(names, type) - output_path_or_url = Webpacker::Configuration.output_path_or_url names.map do |name| - path = Webpacker::Manifest.lookup("#{name}#{compute_asset_extname(name, type: type)}") - "#{output_path_or_url}/#{path}" + name_ensuring_ext = "#{name}#{compute_asset_extname(name, type: type)}" + Webpacker::Manifest.asset_source(name_ensuring_ext) end end end diff --git a/lib/webpacker/manifest.rb b/lib/webpacker/manifest.rb index f1b9799a5..3a6f7b1e7 100644 --- a/lib/webpacker/manifest.rb +++ b/lib/webpacker/manifest.rb @@ -13,19 +13,32 @@ def file_path Webpacker::Configuration.manifest_path end - # Throws an error if the file is not found. If Configuration.compile? then compilation is invoked - # the file is missing. - # React on Rails users will need to set Configuration.compile? to false as compilation is configured - # in the package.json for React on Rails. - def lookup(name) + # Converts the "name" (aka the pack or bundle name) to to the full path for use in a browser. + def asset_source(name) + output_path_or_url = Webpacker::Configuration.output_path_or_url + mapped_name = lookup(name) + "#{output_path_or_url}/#{mapped_name}" + 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 - # Since load_instance is a no-op for production, this should be fine. - # If we're using `webpack -w` to recompile when files changes, we need - # to call this before find! + # Since load_instance 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_instance - find!(name) + 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 @@ -41,31 +54,9 @@ def exist? path_object.exist? end - # Find the real file name from the manifest key. Don't throw an error if the file is simply - # missing from the manifest. Return nil in that case. - # If no manifest file exists, then throw an error. - # **Used by React on Rails.** - def lookup_path_no_throw(name) - instance.confirm_manifest_exists - load_instance - unless instance - raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::Manifest.load must be called first") - end - hashed_name = instance.data[name.to_s] - return hashed_name if hashed_name.blank? - Rails.root.join(File.join(Webpacker::Configuration.output_path, hashed_name)) - end - private - def find!(name) - unless instance - raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::Manifest.load must be called first") - end - instance.data[name.to_s] || missing_file_from_manifest_error(name) - end - - def missing_file_from_manifest_error(bundle_name) - msg = <<-MSG + 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. @@ -74,25 +65,34 @@ def missing_file_from_manifest_error(bundle_name) 5. Your Webpack configuration is not creating a manifest. Your manifest contains: #{instance.data.to_json} - MSG - raise(Webpacker::FileLoader::NotFoundError.new(msg)) - end + MSG + raise(Webpacker::FileLoader::NotFoundError.new(msg)) + end - def missing_manifest_file_error(path_object) - msg = <<-MSG + 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 + raise(Webpacker::FileLoader::NotFoundError.new(msg)) + end - def compile_and_find!(name) - Webpacker.compile - find!(name) - end + def find(name, throw_if_missing: true) + 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, throw_if_missing: true) + Webpacker.compile + find(name, throw_if_missing: throw_if_missing) + end end def confirm_manifest_exists @@ -100,7 +100,6 @@ def confirm_manifest_exists end private - def load_data return super unless File.exist?(@path) JSON.parse(File.read(@path)) diff --git a/test/dev_server_test.rb b/test/dev_server_test.rb index 5c59dacbb..8c4963080 100644 --- a/test/dev_server_test.rb +++ b/test/dev_server_test.rb @@ -3,20 +3,28 @@ class DevServerTest < Minitest::Test require "webpacker_test" + def reset + Webpacker::Configuration.instance_variable_set(:@defaults, nil) + Webpacker::Configuration.instance_variable_set(:@instance, nil) + # Webpacker::Configuration.load_instance + Webpacker::DevServer.instance_variable_set(:@instance, nil) + # Webpacker::DevServer.load_instance + end + def check_assertion + reset stub_value = ActiveSupport::StringInquirer.new("development") Webpacker.stub(:env, stub_value) do - Webpacker::Configuration.reset - Webpacker::DevServer.reset - result = yield - assert_equal(result[0], result[1]) + Webpacker::DevServer.stub(:data, dev_server: {}) do + result = yield + assert_equal(result[0], result[1]) + end end - Webpacker::Configuration.reset - Webpacker::DevServer.reset + reset end def test_dev_server? - check_assertion { [true, Webpacker::DevServer.dev_server?] } + check_assertion { [true, Webpacker::DevServer.enabled?] } end def test_dev_server_host @@ -28,17 +36,17 @@ def test_dev_server_port end def test_dev_server_hot? - check_assertion { [false, Webpacker::DevServer.hot?] } + check_assertion { [false, Webpacker::DevServer.hot_reloading?] } ENV.stub(:[], "TRUE") do - check_assertion { [true, Webpacker::DevServer.hot?] } + check_assertion { [true, Webpacker::DevServer.hot_reloading?] } end ENV.stub(:[], "FALSE") do - check_assertion { [false, Webpacker::DevServer.hot?] } + check_assertion { [false, Webpacker::DevServer.hot_reloading?] } end ENV.stub(:[], "true") do - check_assertion { [true, Webpacker::DevServer.hot?] } + check_assertion { [true, Webpacker::DevServer.hot_reloading?] } end end diff --git a/test/helper_test.rb b/test/helper_test.rb index 1215581e6..7e25b4bd4 100644 --- a/test/helper_test.rb +++ b/test/helper_test.rb @@ -34,12 +34,8 @@ def test_stylesheet_pack_tag_splat end def test_stylesheet_pack_tag_outputs_nothing_for_hot - Webpacker::DevServer.stub(:hot?, true) do - # Webpacker::Configuration.reset - # Webpacker::DevServer.reset + Webpacker::DevServer.stub(:hot_reloading?, true) do assert_equal "", @view.stylesheet_pack_tag("bootstrap.css") end - # Webpacker::Configuration.reset - # Webpacker::DevServer.reset end end diff --git a/test/manifest_test.rb b/test/manifest_test.rb index ff220b6a2..3a7878904 100644 --- a/test/manifest_test.rb +++ b/test/manifest_test.rb @@ -10,20 +10,21 @@ 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. +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, msg) + assert_equal(msg, error.message) end end @@ -70,14 +71,14 @@ def test_confirm_manifest_exists_for_missing end end - def test_lookup_path_no_throw_missing_file - value = Webpacker::Manifest.lookup_path_no_throw("non-existent-bundle.js") + 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_path_no_throw_file_exists - file_path = File.join(File.dirname(__FILE__), "test_app/public/packs", "bootstrap-300631c4f0e0f9c865bc.js").to_s + def test_lookup_no_throw_file_exists + file_path = "bootstrap-300631c4f0e0f9c865bc.js" asset_file = "bootstrap.js" - assert_equal(file_path, Webpacker::Manifest.lookup_path_no_throw(asset_file).to_s) + assert_equal(file_path, Webpacker::Manifest.lookup(asset_file, throw_if_missing: false)) end end From 23c1f23aad952207ee8d7f60cf14523962373cbb Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sun, 6 Aug 2017 08:43:17 -1000 Subject: [PATCH 06/16] Ran bundle to remove pry gems --- Gemfile.lock | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index f49deed5b..14704f0d4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -48,19 +48,12 @@ GEM tzinfo (~> 1.1) arel (7.1.4) ast (2.3.0) - awesome_print (1.8.0) - binding_of_caller (0.7.2) - debug_inspector (>= 0.0.1) builder (3.2.3) - byebug (9.0.6) - coderay (1.1.1) concurrent-ruby (1.0.5) - debug_inspector (0.0.3) erubis (2.7.0) globalid (0.3.7) activesupport (>= 4.1.0) i18n (0.8.1) - interception (0.5) loofah (2.0.3) nokogiri (>= 1.5.9) mail (2.6.4) @@ -78,24 +71,6 @@ GEM parser (2.4.0.0) ast (~> 2.2) powerpack (0.1.1) - pry (0.10.4) - coderay (~> 1.1.0) - method_source (~> 0.8.1) - slop (~> 3.4) - pry-byebug (3.4.2) - byebug (~> 9.0) - pry (~> 0.10) - pry-doc (0.10.0) - pry (~> 0.9) - yard (~> 0.9) - pry-rails (0.3.6) - pry (>= 0.10.4) - pry-rescue (1.4.5) - interception (>= 0.5) - pry - pry-stack_explorer (0.4.9.2) - binding_of_caller (>= 0.7) - pry (>= 0.9.11) rack (2.0.1) rack-test (0.6.3) rack (>= 1.0) @@ -131,7 +106,6 @@ GEM ruby-progressbar (~> 1.7) unicode-display_width (~> 1.0, >= 1.0.1) ruby-progressbar (1.8.1) - slop (3.6.0) sprockets (3.7.1) concurrent-ruby (~> 1.0) rack (> 1, < 3) @@ -147,21 +121,13 @@ GEM websocket-driver (0.6.5) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.2) - yard (0.9.9) PLATFORMS ruby DEPENDENCIES - awesome_print bundler (~> 1.12) minitest (~> 5.0) - pry - pry-byebug - pry-doc - pry-rails - pry-rescue - pry-stack_explorer rails rake (>= 11.1) rubocop (>= 0.47) From 4498a482a77100c726790b824a0cade30094d7d5 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sun, 6 Aug 2017 23:07:06 -1000 Subject: [PATCH 07/16] Remove comment --- lib/webpacker/helper.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/webpacker/helper.rb b/lib/webpacker/helper.rb index ef8fa3d53..847a81285 100644 --- a/lib/webpacker/helper.rb +++ b/lib/webpacker/helper.rb @@ -49,10 +49,6 @@ def javascript_pack_tag(*names, **options) # <%= stylesheet_pack_tag('main') %> <% # Default is false for enabled_when_hot_loading%> # # No output # - # # In development mode with hot-reloading and enabled_when_hot_loading - # # <%= stylesheet_pack_tag('main', enabled_when_hot_loading: true) %> - # - # def stylesheet_pack_tag(*names, **options) if Webpacker::DevServer.hot_reloading? "" From 5740921a660e89e1f0b9fb7151677f020eca2141 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sun, 6 Aug 2017 23:27:52 -1000 Subject: [PATCH 08/16] Remove Manifest.exist? --- lib/webpacker/manifest.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/lib/webpacker/manifest.rb b/lib/webpacker/manifest.rb index 3a6f7b1e7..c16771282 100644 --- a/lib/webpacker/manifest.rb +++ b/lib/webpacker/manifest.rb @@ -47,13 +47,6 @@ def lookup_path(name) Rails.root.join(File.join(Webpacker::Configuration.output_path, lookup(name))) end - # Helper method to determine if the manifest file exists. Maybe Webpack needs to run? - # **Used by React on Rails.** - def exist? - path_object = Webpacker::Configuration.manifest_path - path_object.exist? - end - private def missing_file_from_manifest_error(bundle_name) msg = <<-MSG From 32ff0d060101a0d0fa7f484b7d3436a00d3f50ad Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Tue, 8 Aug 2017 23:05:30 -1000 Subject: [PATCH 09/16] Renamed load_instance and load_data back to load --- lib/webpacker.rb | 10 +++++----- lib/webpacker/configuration.rb | 6 +++--- lib/webpacker/dev_server.rb | 6 +++--- lib/webpacker/env.rb | 2 +- lib/webpacker/file_loader.rb | 6 +++--- lib/webpacker/manifest.rb | 6 +++--- test/manifest_test.rb | 4 ++-- 7 files changed, 20 insertions(+), 20 deletions(-) diff --git a/lib/webpacker.rb b/lib/webpacker.rb index db55fe0bf..394cd3692 100644 --- a/lib/webpacker.rb +++ b/lib/webpacker.rb @@ -2,15 +2,15 @@ module Webpacker extend self def bootstrap - Webpacker::Env.load_instance - Webpacker::Configuration.load_instance - Webpacker::DevServer.load_instance - Webpacker::Manifest.load_instance + Webpacker::Env.load + Webpacker::Configuration.load + Webpacker::DevServer.load + Webpacker::Manifest.load end def compile Webpacker::Compiler.compile - Webpacker::Manifest.load_instance + Webpacker::Manifest.load end def env diff --git a/lib/webpacker/configuration.rb b/lib/webpacker/configuration.rb index 85daa9165..668574060 100644 --- a/lib/webpacker/configuration.rb +++ b/lib/webpacker/configuration.rb @@ -49,8 +49,8 @@ def fetch(key) end def data - load_instance if Webpacker.env.development? - raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::Configuration.load_data must be called first") unless instance + load if Webpacker.env.development? + raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::Configuration.load must be called first") unless instance instance.data end @@ -71,7 +71,7 @@ def output_path_or_url end private - def load_data + def load return Webpacker::Configuration.defaults unless File.exist?(@path) HashWithIndifferentAccess.new(YAML.load(File.read(@path))[Webpacker.env]) end diff --git a/lib/webpacker/dev_server.rb b/lib/webpacker/dev_server.rb index 7d54cfa62..310086afd 100644 --- a/lib/webpacker/dev_server.rb +++ b/lib/webpacker/dev_server.rb @@ -60,14 +60,14 @@ def fetch(key) end def data - load_instance if Webpacker.env.development? - raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::DevServer.load_data must be called first") unless instance + load if Webpacker.env.development? + raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::DevServer.load must be called first") unless instance instance.data end end private - def load_data + def load Webpacker::Configuration.instance.data end end diff --git a/lib/webpacker/env.rb b/lib/webpacker/env.rb index 1f276e3bb..f7a385e37 100644 --- a/lib/webpacker/env.rb +++ b/lib/webpacker/env.rb @@ -14,7 +14,7 @@ def file_path end private - def load_data + def load environments = File.exist?(@path) ? YAML.load(File.read(@path)).keys : [].freeze return ENV["NODE_ENV"] if environments.include?(ENV["NODE_ENV"]) return Rails.env if environments.include?(Rails.env) diff --git a/lib/webpacker/file_loader.rb b/lib/webpacker/file_loader.rb index 76fe0a03f..a458441a8 100644 --- a/lib/webpacker/file_loader.rb +++ b/lib/webpacker/file_loader.rb @@ -7,7 +7,7 @@ class FileLoaderError < StandardError; end attr_accessor :data, :mtime, :path class << self - def load_instance(path = file_path) + def load(path = file_path) if instance.nil? || !production_env? || !file_cached?(path) self.instance = new(path) end @@ -32,10 +32,10 @@ def production_env? def initialize(path) @path = path @mtime = File.exist?(path) ? File.mtime(path) : nil - @data = load_data + @data = load end - def load_data + def load {}.freeze end end diff --git a/lib/webpacker/manifest.rb b/lib/webpacker/manifest.rb index c16771282..96f3a9029 100644 --- a/lib/webpacker/manifest.rb +++ b/lib/webpacker/manifest.rb @@ -32,10 +32,10 @@ def lookup(name, throw_if_missing: true) if Webpacker::Configuration.compile? compile_and_find(name, throw_if_missing: throw_if_missing) else - # Since load_instance checks a `mtime` on the manifest for a non-production env before loading, + # 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_instance + load raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::Manifest.load must be called first") unless instance return find(name, throw_if_missing: throw_if_missing) @@ -93,7 +93,7 @@ def confirm_manifest_exists end private - def load_data + def load return super unless File.exist?(@path) JSON.parse(File.read(@path)) end diff --git a/test/manifest_test.rb b/test/manifest_test.rb index 3a7878904..eb0ce1cb3 100644 --- a/test/manifest_test.rb +++ b/test/manifest_test.rb @@ -45,12 +45,12 @@ def test_file_not_existing temp_path = "#{file_path}.backup" FileUtils.mv(file_path, temp_path) # Point of this test is to ensure no crash - Webpacker::Manifest.load_instance + Webpacker::Manifest.load assert_equal({}, Webpacker::Manifest.instance.data) ensure FileUtils.mv(temp_path, file_path) Webpacker::Manifest.instance = nil - Webpacker::Manifest.load_instance + Webpacker::Manifest.load end end From 5599fd6141d8b2c704c9538f95f157fced217ef9 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Tue, 8 Aug 2017 23:06:02 -1000 Subject: [PATCH 10/16] Remove comments and React on Rails specifics --- README.md | 3 --- test/dev_server_test.rb | 2 -- 2 files changed, 5 deletions(-) diff --git a/README.md b/README.md index 3fac4b54c..2580ef1a2 100644 --- a/README.md +++ b/README.md @@ -1120,9 +1120,6 @@ git push heroku master Webpacker lazily compiles assets in test env so you can write your tests without any extra setup and everything will just work out of the box. -Note, [React on Rails] users should set configuration value `compile` to false, as React on Rails -handles compilation for test and production environments. - Here is a sample system test case with hello_react example component: ```js diff --git a/test/dev_server_test.rb b/test/dev_server_test.rb index 8c4963080..5cbd38d6e 100644 --- a/test/dev_server_test.rb +++ b/test/dev_server_test.rb @@ -6,9 +6,7 @@ class DevServerTest < Minitest::Test def reset Webpacker::Configuration.instance_variable_set(:@defaults, nil) Webpacker::Configuration.instance_variable_set(:@instance, nil) - # Webpacker::Configuration.load_instance Webpacker::DevServer.instance_variable_set(:@instance, nil) - # Webpacker::DevServer.load_instance end def check_assertion From 96fa67ebbcc84eb5d914618125ef63470a801732 Mon Sep 17 00:00:00 2001 From: Gaurav Tiwari Date: Thu, 10 Aug 2017 11:38:58 +0100 Subject: [PATCH 11/16] Inherit from configuration and fix typos --- lib/webpacker/dev_server.rb | 33 +++++++-------------------------- lib/webpacker/file_loader.rb | 5 +++-- lib/webpacker/helper.rb | 2 +- lib/webpacker/manifest.rb | 4 ++-- 4 files changed, 13 insertions(+), 31 deletions(-) diff --git a/lib/webpacker/dev_server.rb b/lib/webpacker/dev_server.rb index 310086afd..9af109c32 100644 --- a/lib/webpacker/dev_server.rb +++ b/lib/webpacker/dev_server.rb @@ -1,10 +1,7 @@ # Same convention as manifest/configuration.rb - # Loads webpacker configuration from config/webpacker.yml -require "webpacker/configuration" - -class Webpacker::DevServer < Webpacker::FileLoader +class Webpacker::DevServer < Webpacker::Configuration class << self def enabled? case ENV["WEBPACKER_DEV_SERVER"] @@ -15,12 +12,12 @@ def enabled? end # read settings for dev_server - def hot_reloading? + def hmr? if enabled? case ENV["WEBPACKER_HMR"] when /true/i then true when /false/i then false - else fetch(:hot_reloading) + else fetch(:hmr) end else false @@ -43,31 +40,15 @@ def protocol https? ? "https" : "http" end - def file_path - Webpacker::Configuration.file_path - end - # Uses the hot_reloading_host if appropriate def base_url "#{protocol}://#{host}:#{port}" end - private - def fetch(key) - if enabled? - data[:dev_server][key] || Webpacker::Configuration.defaults[:dev_server][key] - end - end - - def data - load if Webpacker.env.development? - raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::DevServer.load must be called first") unless instance - instance.data + def fetch(key) + if enabled? + data[:dev_server][key] || defaults[:dev_server][key] end - end - - private - def load - Webpacker::Configuration.instance.data end + end end diff --git a/lib/webpacker/file_loader.rb b/lib/webpacker/file_loader.rb index 416801561..44c8915c0 100644 --- a/lib/webpacker/file_loader.rb +++ b/lib/webpacker/file_loader.rb @@ -12,7 +12,7 @@ def load(path = file_path) self.instance = new(path) end end - + def file_path raise FileLoaderError.new("Subclass of Webpacker::FileLoader should override this method") end @@ -25,7 +25,8 @@ def file_cached?(path) # 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 diff --git a/lib/webpacker/helper.rb b/lib/webpacker/helper.rb index 72a686f8b..dab52cbe0 100644 --- a/lib/webpacker/helper.rb +++ b/lib/webpacker/helper.rb @@ -50,7 +50,7 @@ def javascript_pack_tag(*names, **options) # # No output # def stylesheet_pack_tag(*names, **options) - if Webpacker::DevServer.hot_reloading? + if Webpacker::DevServer.hmr? "" else stylesheet_link_tag(*sources_from_pack_manifest(names, type: :stylesheet), **options) diff --git a/lib/webpacker/manifest.rb b/lib/webpacker/manifest.rb index f7929eb8b..fa7f9807d 100644 --- a/lib/webpacker/manifest.rb +++ b/lib/webpacker/manifest.rb @@ -34,7 +34,7 @@ def lookup(name, throw_if_missing: true) instance.confirm_manifest_exists if Webpacker::Configuration.compile? - compile_and_find(name, throw_if_missing: throw_if_missing) + compile_and_find!(name, throw_if_missing: throw_if_missing) else # 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 @@ -42,7 +42,7 @@ def lookup(name, throw_if_missing: true) load raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::Manifest.load must be called first") unless instance - return find(name, throw_if_missing: throw_if_missing) + return find!(name, throw_if_missing: throw_if_missing) end end From fe292e6f70fc63fc70945e705d3c72ceb3fb85a8 Mon Sep 17 00:00:00 2001 From: Gaurav Tiwari Date: Thu, 10 Aug 2017 11:39:20 +0100 Subject: [PATCH 12/16] Rename to hmr? --- test/dev_server_test.rb | 10 +++++----- test/helper_test.rb | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/dev_server_test.rb b/test/dev_server_test.rb index 5cbd38d6e..f844298ac 100644 --- a/test/dev_server_test.rb +++ b/test/dev_server_test.rb @@ -33,18 +33,18 @@ def test_dev_server_port check_assertion { [8080, Webpacker::DevServer.port] } end - def test_dev_server_hot? - check_assertion { [false, Webpacker::DevServer.hot_reloading?] } + def test_dev_server_hmr? + check_assertion { [false, Webpacker::DevServer.hmr?] } ENV.stub(:[], "TRUE") do - check_assertion { [true, Webpacker::DevServer.hot_reloading?] } + check_assertion { [true, Webpacker::DevServer.hmr?] } end ENV.stub(:[], "FALSE") do - check_assertion { [false, Webpacker::DevServer.hot_reloading?] } + check_assertion { [false, Webpacker::DevServer.hmr?] } end ENV.stub(:[], "true") do - check_assertion { [true, Webpacker::DevServer.hot_reloading?] } + check_assertion { [true, Webpacker::DevServer.hmr?] } end end diff --git a/test/helper_test.rb b/test/helper_test.rb index 7e25b4bd4..2fe0e3a0e 100644 --- a/test/helper_test.rb +++ b/test/helper_test.rb @@ -34,7 +34,7 @@ def test_stylesheet_pack_tag_splat end def test_stylesheet_pack_tag_outputs_nothing_for_hot - Webpacker::DevServer.stub(:hot_reloading?, true) do + Webpacker::DevServer.stub(:hmr?, true) do assert_equal "", @view.stylesheet_pack_tag("bootstrap.css") end end From e4337db28cdad9c2eb89d0bc799baae39496eb46 Mon Sep 17 00:00:00 2001 From: Gaurav Tiwari Date: Thu, 10 Aug 2017 11:40:50 +0100 Subject: [PATCH 13/16] Update js for HMR --- lib/install/config/webpack/development.js | 11 +++++++++-- lib/install/config/webpack/shared.js | 4 +--- lib/install/config/webpacker.yml | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/install/config/webpack/development.js b/lib/install/config/webpack/development.js index 7247e31aa..248499708 100644 --- a/lib/install/config/webpack/development.js +++ b/lib/install/config/webpack/development.js @@ -1,8 +1,9 @@ // 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') +const { settings, output, env } = require('./configuration.js') module.exports = merge(sharedConfig, { devtool: 'cheap-eval-source-map', @@ -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 a04f2fab3..244efd65e 100644 --- a/lib/install/config/webpack/shared.js +++ b/lib/install/config/webpack/shared.js @@ -37,9 +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({ - writeToFileEmit: true - }) + new ManifestPlugin({ writeToFileEmit: true }) ], resolve: { diff --git a/lib/install/config/webpacker.yml b/lib/install/config/webpacker.yml index 03a5f65c8..00c6e80ed 100644 --- a/lib/install/config/webpacker.yml +++ b/lib/install/config/webpacker.yml @@ -33,7 +33,7 @@ development: host: localhost port: 8080 https: false - hot_reloading: false + hmr: false test: <<: *default From d523fb9bf77edab02b5e59e69f37bd971b03010e Mon Sep 17 00:00:00 2001 From: Gaurav Tiwari Date: Thu, 10 Aug 2017 12:28:03 +0100 Subject: [PATCH 14/16] Update readme --- README.md | 10 +++++----- lib/install/config/webpack/development.js | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 1f530f0ac..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,10 +360,10 @@ development: host: 0.0.0.0 port: 8080 https: false - hot_reloading: false + hmr: false ``` -If you have hot_reloading turned to `true`, then the `stylesheet_pack_tag` generates no output, as you will want +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. @@ -451,12 +451,12 @@ 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/hot_reloading` key to TRUE or set the +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. +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`. diff --git a/lib/install/config/webpack/development.js b/lib/install/config/webpack/development.js index 248499708..88bebac03 100644 --- a/lib/install/config/webpack/development.js +++ b/lib/install/config/webpack/development.js @@ -3,7 +3,7 @@ const webpack = require('webpack') const merge = require('webpack-merge') const sharedConfig = require('./shared.js') -const { settings, output, env } = require('./configuration.js') +const { settings, output } = require('./configuration.js') module.exports = merge(sharedConfig, { devtool: 'cheap-eval-source-map', From 007e36e42938b3052968e31a53b99e506feca6db Mon Sep 17 00:00:00 2001 From: Gaurav Tiwari Date: Fri, 11 Aug 2017 18:28:16 +0100 Subject: [PATCH 15/16] Change dev-server to a module and fix tests --- .gitignore | 1 + lib/webpacker.rb | 1 - lib/webpacker/compiler.rb | 3 ++ lib/webpacker/configuration.rb | 11 ----- lib/webpacker/dev_server.rb | 81 ++++++++++++++++------------------ lib/webpacker/helper.rb | 7 +-- lib/webpacker/manifest.rb | 12 +++-- test/compiler_test.rb | 4 +- test/dev_server_test.rb | 5 +-- 9 files changed, 55 insertions(+), 70 deletions(-) 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/lib/webpacker.rb b/lib/webpacker.rb index 2d9170554..bdb4f134f 100644 --- a/lib/webpacker.rb +++ b/lib/webpacker.rb @@ -4,7 +4,6 @@ module Webpacker def bootstrap Webpacker::Env.load Webpacker::Configuration.load - Webpacker::DevServer.load Webpacker::Manifest.load end diff --git a/lib/webpacker/compiler.rb b/lib/webpacker/compiler.rb index d85775e61..112a212dd 100644 --- a/lib/webpacker/compiler.rb +++ b/lib/webpacker/compiler.rb @@ -1,6 +1,7 @@ require "open3" require "webpacker/env" require "webpacker/configuration" +require "webpacker/dev_server" module Webpacker::Compiler extend self @@ -13,6 +14,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 6ad98cb2a..5188966d3 100644 --- a/lib/webpacker/configuration.rb +++ b/lib/webpacker/configuration.rb @@ -61,17 +61,6 @@ def data def defaults @defaults ||= HashWithIndifferentAccess.new(YAML.load(default_file_path.read)[Webpacker.env]) end - - # Uses the webpack dev server host if appropriate - def output_path_or_url - if Webpacker::DevServer.enabled? - Webpacker::DevServer.base_url - else - # Ensure we start with a slash so that the asset helpers don't prepend the default asset - # pipeline locations. - public_output_path.starts_with?("/") ? public_output_path : "/#{public_output_path}" - end - end end private diff --git a/lib/webpacker/dev_server.rb b/lib/webpacker/dev_server.rb index 9af109c32..0e72395c2 100644 --- a/lib/webpacker/dev_server.rb +++ b/lib/webpacker/dev_server.rb @@ -1,54 +1,47 @@ -# Same convention as manifest/configuration.rb -# Loads webpacker configuration from config/webpacker.yml - -class Webpacker::DevServer < Webpacker::Configuration - class << self - def enabled? - case ENV["WEBPACKER_DEV_SERVER"] - when /true/i then true - when /false/i then false - else !data[:dev_server].nil? - end - end +module Webpacker::DevServer + extend self - # read settings for dev_server - def hmr? - if enabled? - case ENV["WEBPACKER_HMR"] - when /true/i then true - when /false/i then false - else fetch(:hmr) - end - else - false - end - end + def running? + socket = Socket.tcp(host, port, connect_timeout: 1) + socket.close + true - def host - fetch(:host) - end + rescue Errno::ECONNREFUSED, NoMethodError + false + end - def port - fetch(:port) + def hmr? + case ENV["WEBPACKER_HMR"] + when /true/i then true + when /false/i then false + else fetch(:hmr) end - def https? - fetch(:https) - end + rescue NoMethodError + false + end - def protocol - https? ? "https" : "http" - end + def host + fetch(:host) + end - # Uses the hot_reloading_host if appropriate - def base_url - "#{protocol}://#{host}:#{port}" - end + def port + fetch(:port) + end - def fetch(key) - if enabled? - data[:dev_server][key] || defaults[:dev_server][key] - end - 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/helper.rb b/lib/webpacker/helper.rb index dab52cbe0..ddb492ecc 100644 --- a/lib/webpacker/helper.rb +++ b/lib/webpacker/helper.rb @@ -9,10 +9,7 @@ module Webpacker::Helper # In production mode: # <%= asset_pack_path 'calendar.css' %> # => "/packs/calendar-1016838bab065ae1e122.css" def asset_pack_path(name, **options) - path = Webpacker::Configuration.public_output_path - file = Webpacker::Manifest.lookup(name) - full_path = "#{path}/#{file}" - asset_path(full_path, **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 @@ -59,7 +56,7 @@ def stylesheet_pack_tag(*names, **options) private def sources_from_pack_manifest(names, type:) - names.map { |name| Webpacker::Manifest.asset_source(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 fa7f9807d..fd51f362e 100644 --- a/lib/webpacker/manifest.rb +++ b/lib/webpacker/manifest.rb @@ -18,10 +18,14 @@ def file_path end # Converts the "name" (aka the pack or bundle name) to to the full path for use in a browser. - def asset_source(name) - output_path_or_url = Webpacker::Configuration.output_path_or_url - mapped_name = lookup(name) - "#{output_path_or_url}/#{mapped_name}" + 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. 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/dev_server_test.rb b/test/dev_server_test.rb index f844298ac..260a69241 100644 --- a/test/dev_server_test.rb +++ b/test/dev_server_test.rb @@ -6,14 +6,13 @@ class DevServerTest < Minitest::Test def reset Webpacker::Configuration.instance_variable_set(:@defaults, nil) Webpacker::Configuration.instance_variable_set(:@instance, nil) - Webpacker::DevServer.instance_variable_set(:@instance, nil) end def check_assertion reset stub_value = ActiveSupport::StringInquirer.new("development") Webpacker.stub(:env, stub_value) do - Webpacker::DevServer.stub(:data, dev_server: {}) do + Webpacker::Configuration.stub(:data, dev_server: {}) do result = yield assert_equal(result[0], result[1]) end @@ -22,7 +21,7 @@ def check_assertion end def test_dev_server? - check_assertion { [true, Webpacker::DevServer.enabled?] } + check_assertion { [false, Webpacker::DevServer.running?] } end def test_dev_server_host From 7f7fcace87aa8c8f4bd98cb1f1ec14f167799384 Mon Sep 17 00:00:00 2001 From: Gaurav Tiwari Date: Fri, 11 Aug 2017 18:32:09 +0100 Subject: [PATCH 16/16] Remove extra requires --- lib/webpacker/compiler.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/webpacker/compiler.rb b/lib/webpacker/compiler.rb index 112a212dd..bf8eb9dc3 100644 --- a/lib/webpacker/compiler.rb +++ b/lib/webpacker/compiler.rb @@ -1,7 +1,4 @@ require "open3" -require "webpacker/env" -require "webpacker/configuration" -require "webpacker/dev_server" module Webpacker::Compiler extend self