Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Binstubs #833

Merged
merged 12 commits into from
Sep 19, 2017
Merged
8 changes: 8 additions & 0 deletions exe/webpack
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#!/usr/bin/env ruby

ENV["RAILS_ENV"] ||= ENV["RACK_ENV"] || "development"
ENV["NODE_ENV"] ||= ENV["RAILS_ENV"]

require "webpacker"
require "webpacker/webpack_runner"
Webpacker::WebpackRunner.run(ARGV)
8 changes: 8 additions & 0 deletions exe/webpack-dev-server
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#!/usr/bin/env ruby

ENV["RAILS_ENV"] ||= ENV["RACK_ENV"] || "development"
ENV["NODE_ENV"] ||= ENV["RAILS_ENV"]

require "webpacker"
require "webpacker/dev_server_runner"
Webpacker::DevServerRunner.run(ARGV)
68 changes: 0 additions & 68 deletions lib/install/bin/webpack-dev-server.tt

This file was deleted.

26 changes: 0 additions & 26 deletions lib/install/bin/webpack.tt

This file was deleted.

6 changes: 2 additions & 4 deletions lib/install/template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@
puts "Creating javascript app source directory"
directory "#{__dir__}/javascript", Webpacker.config.source_path

puts "Copying binstubs"
directory "#{__dir__}/bin", "bin"

chmod "bin", 0755 & ~File.umask, verbose: false
puts "Installing binstubs"
run "bundle binstubs webpacker"

if File.exists?(".gitignore")
append_to_file ".gitignore", <<-EOS
Expand Down
72 changes: 72 additions & 0 deletions lib/webpacker/dev_server_runner.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
require "shellwords"
require "yaml"
require "socket"
require "webpacker/runner"

module Webpacker
class DevServerRunner < Webpacker::Runner
def run
load_config
check_server!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may be call this detect_port!?

execute_cmd
end

private

def load_config
@config_file = File.join(@app_path, "config/webpacker.yml")
@default_listen_host_addr = ENV["NODE_ENV"] == "development" ? "localhost" : "0.0.0.0"

dev_server = YAML.load_file(@config_file)[ENV["RAILS_ENV"]]["dev_server"]

@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}"
@listen_host_addr = args("--listen-host") || @default_listen_host_addr

rescue Errno::ENOENT, NoMethodError
$stdout.puts "Webpack dev_server configuration not found in #{@config_file}."
$stdout.puts "Please run bundle exec rails webpacker:install to install webpacker"
exit!
end

def check_server!
server = TCPServer.new(@listen_host_addr, @port)
server.close

rescue Errno::EADDRINUSE
$stdout.puts "Another program is running on port #{@port}. Set a new port in #{@config_file} for dev_server"
exit!
end

def execute_cmd
argv = @argv.dup

# Delete supplied host, port and listen-host CLI arguments
["--host", "--port", "--listen-host"].each do |arg|
argv.delete(args(arg))
argv.delete(arg)
end

env = { "NODE_PATH" => @node_modules_path.shellescape }

cmd = [
"#{@node_modules_path}/.bin/webpack-dev-server", "--progress", "--color",
"--config", @webpack_config,
"--host", @listen_host_addr,
"--public", "#{@hostname}:#{@port}",
"--port", @port.to_s
] + argv

Dir.chdir(@app_path) do
exec env, *cmd
end
end

def args(key)
index = @argv.index(key)
index ? @argv[index + 1] : nil
end
end
end
22 changes: 22 additions & 0 deletions lib/webpacker/runner.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
module Webpacker
class Runner
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps Runner should be the base class that all runner classes inherit from.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like:

# lib/webpacker/runners/webpack.rb
class Webpacker::WebpackRunner < Webpacker::Runner

# lib/webpacker/runners/dev_server.rb
class Webpacker::DevServerRunner < Webpacker::Runner

def self.run(argv)
$stdout.sync = true

new(argv).run
end

def initialize(argv)
@argv = argv

@app_path = File.expand_path(".", Dir.pwd)
Copy link
Member Author

@rossta rossta Sep 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Dir.pwd has pros and cons: we can install the executables as binstubs with bundler (which won't supply the application dir context) but we wouldn't be able to execute the binstubs from anywhere but the project root. I'm unsure of how this might affect various deployment scenarios.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah... maybe you can leverage the root_path accessor set via webpacker itself which defaults to Rails.root? Not sure what's best in this context?

Copy link
Member Author

@rossta rossta Sep 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rails.root would make it easy but I was hoping to avoid relying on loading rails since these are just thin wrappers around node executables.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some alternatives:

  • Re-instate the bin templates (backing out of the the bundler binstub change) so we can pass in the app path to the runner
  • Support an ENV var like APP_PATH to override the default

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, lets tackle that in another PR

@node_modules_path = File.join(@app_path, "node_modules")
@webpack_config = File.join(@app_path, "config/webpack/#{ENV["NODE_ENV"]}.js")

unless File.exist?(@webpack_config)
puts "Webpack config #{@webpack_config} not found, please run 'bundle exec rails webpacker:install' to install webpacker with default configs or add the missing config file for your custom environment."
exit!
end
end
end
end
15 changes: 15 additions & 0 deletions lib/webpacker/webpack_runner.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
require "shellwords"
require "webpacker/runner"

module Webpacker
class WebpackRunner < Webpacker::Runner
def run
env = { "NODE_PATH" => @node_modules_path.shellescape }
cmd = [ "#{@node_modules_path}/.bin/webpack", "--config", @webpack_config ] + @argv

Dir.chdir(@app_path) do
exec env, *cmd
end
end
end
end
2 changes: 2 additions & 0 deletions webpacker.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ Gem::Specification.new do |s|
s.summary = "Use Webpack to manage app-like JavaScript modules in Rails"
s.homepage = "https://github.com/rails/webpacker"
s.license = "MIT"
s.bindir = "exe"
s.executables = `git ls-files -- exe/*`.split("\n").map { |f| File.basename(f) }

s.required_ruby_version = ">= 1.9.3"

Expand Down