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

Author: gauravtiwari

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)

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"

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)

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.

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}."

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]'
     }
   }]

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 = [

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,

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

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)

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])

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

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

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

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)

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

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

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"