From 68b9f086c1d262be895c2750cb3a25879157733a Mon Sep 17 00:00:00 2001 From: Gaurav Tiwari Date: Sat, 12 Aug 2017 20:42:29 +0100 Subject: [PATCH] Add dev server config class and setup proxy (#637) * Add rack proxy Add options for ssl and set publicPath to only pack output path * Setup dev_server class Add dev_server config * Add and mount the dev_server proxy Move dev-server to defaults * Add tests Move to private Fix paths Update path names * Update changelog * Update changelog Fix typos Delete if path exists * Fix test env and update tests * Only add proxy in development Remove extra if * Nix extra CR * Move skipping compile on manifest lookups when dev server is running to manifest * Give proxy middleware its own initializer * Rely on @javan's fix instead of changing all these * Remove needless local variable * Fix compile predicate check * Nix dealing with HTTPS in development * Only proxy to dev server if its running * Better grouping * Nix need for config to return anything but full paths * Supply host_with_port * Only needed to delete this for HTTPS * Rare occurrence to run dev server in production Don't need to advertise that so loudly. * Not needed * Not needed * Not needed * Update documentation to our new proxy approach * Fix rubocop offenses --- CHANGELOG.md | 25 +++++++++++++++ Gemfile | 1 + Gemfile.lock | 4 +++ README.md | 35 +++------------------ lib/install/bin/webpack-dev-server.tt | 3 +- lib/install/config/loaders/core/assets.js | 3 +- lib/install/config/webpack/configuration.js | 3 +- lib/install/config/webpack/development.js | 1 - lib/install/config/webpacker.yml | 9 +++--- lib/webpacker.rb | 3 +- lib/webpacker/configuration.rb | 12 ++++--- lib/webpacker/dev_server.rb | 35 +++++++++++++++++++++ lib/webpacker/dev_server_proxy.rb | 23 ++++++++++++++ lib/webpacker/instance.rb | 4 +++ lib/webpacker/manifest.rb | 10 ++++-- lib/webpacker/railtie.rb | 9 ++++++ test/dev_server_test.rb | 11 +++++++ webpacker.gemspec | 1 + 18 files changed, 144 insertions(+), 48 deletions(-) create mode 100644 lib/webpacker/dev_server.rb create mode 100644 lib/webpacker/dev_server_proxy.rb create mode 100644 test/dev_server_test.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index c4f8b6088..2c72f7c5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,31 @@ - `Webpacker::Compiler.fresh?` and `Webpacker::Compiler.stale?` answer the question of whether compilation is needed. The old `Webpacker::Compiler.compile?` predicate is deprecated. +- Dev server config class that exposes config options through singleton. + + ```rb + Webpacker.dev_server.running? + ``` + +- Rack middleware proxies webpacker requests to dev server so we can always serve from same-origin and the lookup works out of the box - no more paths prefixing + +### Breaking changes + +**Note:** requires running `bundle exec rails webpacker:install` and update webpacker.yml + +- Move dev-server config options under defaults so it's transparently available in all environments + + +### Removed + +- Host info from manifest.json, now looks like this: + + ```json + { + "hello_react.js": "/packs/hello_react.js" + } + ``` + ### Fixed - Update `webpack-dev-server.tt` to respect RAILS_ENV and NODE_ENV values [#502](https://github.com/rails/webpacker/issues/502) diff --git a/Gemfile b/Gemfile index 32d7ff145..ca5add6b9 100644 --- a/Gemfile +++ b/Gemfile @@ -5,6 +5,7 @@ gemspec gem "rails" gem "rake", ">= 11.1" gem "rubocop", ">= 0.49", require: false +gem "rack-proxy", require: false group :test do gem "minitest", "~> 5.0" diff --git a/Gemfile.lock b/Gemfile.lock index 32e6f8e5f..fd2ecc4bf 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -3,6 +3,7 @@ PATH specs: webpacker (2.0) activesupport (>= 4.2) + rack-proxy (>= 0.6.1) railties (>= 4.2) GEM @@ -72,6 +73,8 @@ GEM ast (~> 2.2) powerpack (0.1.1) rack (2.0.1) + rack-proxy (0.6.1) + rack rack-test (0.6.3) rack (>= 1.0) rails (5.0.1) @@ -131,6 +134,7 @@ DEPENDENCIES bundler (~> 1.12) byebug minitest (~> 5.0) + rack-proxy rails rake (>= 11.1) rubocop (>= 0.49) diff --git a/README.md b/README.md index 2ba8ed34f..d9e23e69b 100644 --- a/README.md +++ b/README.md @@ -246,40 +246,13 @@ happens when you refer to any of the pack assets using the Webpacker helper meth That means you don't have to run any separate process. Compilation errors are logged to the standard Rails log. -If you want to use live code reloading, you'll need to run `./bin/webpack-dev-server` +If you want to use live code reloading, or you have enough JavaScript that on-demand compilation is too slow, you'll need to run `./bin/webpack-dev-server` in a separate terminal from `./bin/rails server`. This process will watch for changes in the `app/javascript/packs/*.js` files and automatically reload the browser to match. -Note: The dev server serves pack files from `http://localhost:8080/` by default, which -is a different origin than your application server. Therefore it's not compatible with -things like Service Workers, that requires you to serve from the same origin. - -If you'd rather not have to run the two processes separately by hand, you can use [Foreman](https://ddollar.github.io/foreman): - -```bash -gem install foreman -``` - -```yml -# Procfile -web: bundle exec rails s -webpacker: ./bin/webpack-dev-server -``` - -```bash -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. -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`. - -```bash -dev_server: - host: example.com -``` +Once you start this development server, Webpacker will automatically start proxying all +webpack asset requests to this server. When you stop the server, it'll revert to +on-demand compilation again. You can also pass CLI options supported by [webpack-dev-server](https://webpack.js.org/configuration/dev-server/). Please note that inline options will always take precedence over the ones already set in the configuration file. diff --git a/lib/install/bin/webpack-dev-server.tt b/lib/install/bin/webpack-dev-server.tt index 00bbd4bde..ac15988f2 100644 --- a/lib/install/bin/webpack-dev-server.tt +++ b/lib/install/bin/webpack-dev-server.tt @@ -28,8 +28,7 @@ begin HOSTNAME = args('--host') || dev_server["host"] PORT = args('--port') || dev_server["port"] - HTTPS = ARGV.include?('--https') || dev_server["https"] - DEV_SERVER_ADDR = "http#{"s" if HTTPS}://#{HOSTNAME}:#{PORT}" + DEV_SERVER_ADDR = "http://#{HOSTNAME}:#{PORT}" rescue Errno::ENOENT, NoMethodError $stdout.puts "Webpack dev_server configuration not found in #{CONFIG_FILE}." diff --git a/lib/install/config/loaders/core/assets.js b/lib/install/config/loaders/core/assets.js index 621cec109..0b3525e65 100644 --- a/lib/install/config/loaders/core/assets.js +++ b/lib/install/config/loaders/core/assets.js @@ -5,7 +5,8 @@ module.exports = { use: [{ loader: 'file-loader', options: { - publicPath: output.publicPath, + // Set publicPath with ASSET_HOST env if available so internal assets can use CDN + publicPath: output.publicPathWithHost, name: env.NODE_ENV === 'production' ? '[name]-[hash].[ext]' : '[name].[ext]' } }] diff --git a/lib/install/config/webpack/configuration.js b/lib/install/config/webpack/configuration.js index 2a37e63d2..d48074342 100644 --- a/lib/install/config/webpack/configuration.js +++ b/lib/install/config/webpack/configuration.js @@ -24,7 +24,8 @@ function formatPublicPath(host = '', path = '') { const output = { path: resolve('public', settings.public_output_path), - publicPath: formatPublicPath(env.ASSET_HOST, settings.public_output_path) + publicPath: `/${settings.public_output_path}/`.replace(/([^:]\/)\/+/g, '$1'), + publicPathWithHost: formatPublicPath(env.ASSET_HOST, settings.public_output_path) } let resolvedModules = [ diff --git a/lib/install/config/webpack/development.js b/lib/install/config/webpack/development.js index 7247e31aa..6dfb717e8 100644 --- a/lib/install/config/webpack/development.js +++ b/lib/install/config/webpack/development.js @@ -13,7 +13,6 @@ module.exports = merge(sharedConfig, { devServer: { clientLogLevel: 'none', - https: settings.dev_server.https, host: settings.dev_server.host, port: settings.dev_server.port, contentBase: output.path, diff --git a/lib/install/config/webpacker.yml b/lib/install/config/webpacker.yml index 8fcb01506..521ef0bd4 100644 --- a/lib/install/config/webpacker.yml +++ b/lib/install/config/webpacker.yml @@ -10,6 +10,11 @@ default: &default # ['app/assets', 'engine/foo/app/assets'] resolved_paths: [] + # webpack-dev-server configuration + dev_server: &dev_server + host: localhost + port: 3035 + extensions: - .coffee - .erb @@ -29,10 +34,6 @@ default: &default development: <<: *default compile: true - dev_server: - host: localhost - port: 8080 - https: false test: <<: *default diff --git a/lib/webpacker.rb b/lib/webpacker.rb index db2430047..0b6544ebc 100644 --- a/lib/webpacker.rb +++ b/lib/webpacker.rb @@ -14,7 +14,7 @@ def instance end delegate :logger, :logger=, :env, to: :instance - delegate :config, :compiler, :manifest, :commands, to: :instance + delegate :config, :compiler, :manifest, :commands, :dev_server, to: :instance delegate :bootstrap, :clobber, :compile, to: :commands end @@ -23,5 +23,6 @@ def instance require "webpacker/manifest" require "webpacker/compiler" require "webpacker/commands" +require "webpacker/dev_server" require "webpacker/railtie" if defined?(Rails) diff --git a/lib/webpacker/configuration.rb b/lib/webpacker/configuration.rb index f78504db8..37815ba6a 100644 --- a/lib/webpacker/configuration.rb +++ b/lib/webpacker/configuration.rb @@ -9,6 +9,14 @@ def refresh @data = load end + def dev_server + fetch(:dev_server) + end + + def compile? + fetch(:compile) + end + def source_path root_path.join(fetch(:source_path)) end @@ -33,10 +41,6 @@ def cache_path root_path.join(fetch(:cache_path)) end - def compile? - fetch(:compile) - end - private def fetch(key) data.fetch(key, defaults[key]) diff --git a/lib/webpacker/dev_server.rb b/lib/webpacker/dev_server.rb new file mode 100644 index 000000000..bac89071f --- /dev/null +++ b/lib/webpacker/dev_server.rb @@ -0,0 +1,35 @@ +class Webpacker::DevServer + delegate :config, to: :@webpacker + + def initialize(webpacker) + @webpacker = webpacker + end + + def running? + Socket.tcp(host, port, connect_timeout: 1).close + true + rescue Errno::ECONNREFUSED, NoMethodError + false + end + + def host + fetch(:host) + end + + def port + fetch(:port) + end + + def host_with_port + "#{host}:#{port}" + end + + private + def fetch(key) + config.dev_server.fetch(key, defaults[key]) + end + + def defaults + config.send(:defaults)[:dev_server] + end +end diff --git a/lib/webpacker/dev_server_proxy.rb b/lib/webpacker/dev_server_proxy.rb new file mode 100644 index 000000000..ccbe33b55 --- /dev/null +++ b/lib/webpacker/dev_server_proxy.rb @@ -0,0 +1,23 @@ +require "rack/proxy" + +class Webpacker::DevServerProxy < Rack::Proxy + def rewrite_response(response) + status, headers, body = response + headers.delete "transfer-encoding" + response + end + + def perform_request(env) + if env["PATH_INFO"] =~ /#{public_output_uri_path}/ && Webpacker.dev_server.running? + env["HTTP_HOST"] = Webpacker.dev_server.host_with_port + super(env) + else + @app.call(env) + end + end + + private + def public_output_uri_path + Webpacker.config.public_output_path.relative_path_from(Webpacker.config.public_path) + end +end diff --git a/lib/webpacker/instance.rb b/lib/webpacker/instance.rb index 28648e825..0775f3b58 100644 --- a/lib/webpacker/instance.rb +++ b/lib/webpacker/instance.rb @@ -21,6 +21,10 @@ def compiler @compiler ||= Webpacker::Compiler.new self end + def dev_server + @dev_server ||= Webpacker::DevServer.new self + end + def manifest @manifest ||= Webpacker::Manifest.new self end diff --git a/lib/webpacker/manifest.rb b/lib/webpacker/manifest.rb index 6628325c6..11ef101d7 100644 --- a/lib/webpacker/manifest.rb +++ b/lib/webpacker/manifest.rb @@ -12,7 +12,7 @@ class Webpacker::Manifest class MissingEntryError < StandardError; end - delegate :config, :compiler, :env, to: :@webpacker + delegate :config, :compiler, :env, :dev_server, to: :@webpacker def initialize(webpacker) @webpacker = webpacker @@ -23,13 +23,17 @@ def refresh end def lookup(name) - compile + compile if compiling? find name end private + def compiling? + config.compile? && !dev_server.running? + end + def compile - Webpacker.logger.tagged("Webpacker") { compiler.compile } if config.compile? + Webpacker.logger.tagged("Webpacker") { compiler.compile } end def find(name) diff --git a/lib/webpacker/railtie.rb b/lib/webpacker/railtie.rb index 11c54773b..c88967778 100644 --- a/lib/webpacker/railtie.rb +++ b/lib/webpacker/railtie.rb @@ -1,8 +1,17 @@ require "rails/railtie" require "webpacker/helper" +require "webpacker/dev_server_proxy" class Webpacker::Engine < ::Rails::Engine + initializer "webpacker.proxy" do |app| + if Rails.env.development? + app.middleware.insert_before 0, + Rails::VERSION::MAJOR >= 5 ? + Webpacker::DevServerProxy : "Webpacker::DevServerProxy" + end + end + initializer "webpacker.helper" do |app| ActiveSupport.on_load :action_controller do ActionController::Base.helper Webpacker::Helper diff --git a/test/dev_server_test.rb b/test/dev_server_test.rb new file mode 100644 index 000000000..f0b5bce36 --- /dev/null +++ b/test/dev_server_test.rb @@ -0,0 +1,11 @@ +require "webpacker_test_helper" + +class DevServerTest < Minitest::Test + def test_host + assert_equal "localhost", Webpacker.dev_server.host + end + + def test_port + assert_equal Webpacker.dev_server.port, 3035 + end +end diff --git a/webpacker.gemspec b/webpacker.gemspec index b4c97b96a..05c9d1543 100644 --- a/webpacker.gemspec +++ b/webpacker.gemspec @@ -14,6 +14,7 @@ Gem::Specification.new do |s| s.add_dependency "activesupport", ">= 4.2" s.add_dependency "railties", ">= 4.2" + s.add_dependency "rack-proxy", ">= 0.6.1" s.add_development_dependency "bundler", "~> 1.12"