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 bin/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/runner"
Webpacker::Runner.run(ARGV)
8 changes: 8 additions & 0 deletions bin/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
85 changes: 85 additions & 0 deletions lib/webpacker/dev_server_runner.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
require "shellwords"
require "yaml"
require "socket"

module Webpacker
class DevServerRunner
def self.run(argv)
$stdout.sync = true

new(argv).run
end

def initialize(argv)
@argv = argv

@app_path = File.expand_path("../", __dir__)
@config_file = File.join(@app_path, "config/webpacker.yml")
@node_modules_path = File.join(@app_path, "node_modules")
@webpack_config = File.join(@app_path, "config/webpack/#{ENV["NODE_ENV"]}.js")
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 we can load options from Webpacker.config or dev_server itself instead of loading the yml again?

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.

It would be great to avoid that duplication. Currently, Webpacker.dev_server inherits form Webpacker::Instance which depends on Rails (for Rails.root here) and I think it would be preferable to avoid depending on Rails to execute these scripts. Perhaps a future improvement?

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense 👍

@default_listen_host_addr = ENV["NODE_ENV"] == 'development' ? 'localhost' : '0.0.0.0'

begin
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
end

def run
check_server!
update_argv
execute_cmd
end

private
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 update_argv
Copy link
Member

Choose a reason for hiding this comment

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

We need this empty method here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, removing!

end

def execute_cmd
argv = @argv

# 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
34 changes: 34 additions & 0 deletions lib/webpacker/runner.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
require "shellwords"

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__)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does __dir__ work correctly here (in a file in the gem)?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, good catch—error in local testing. I may need to pass the app_path into the runner which may mean keeping the install template with less code instead of using a bundler binstub.

@node_modules_path = File.join(@app_path, "node_modules")
@webpack_config = File.join(@app_path, "config/webpack/#{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

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 @@ -10,6 +10,8 @@ Gem::Specification.new do |s|
s.homepage = "https://github.com/rails/webpacker"
s.license = "MIT"

s.executables = `git ls-files -- bin/*`.split("\n").map{ |f| File.basename(f) }

s.required_ruby_version = ">= 1.9.3"

s.add_dependency "activesupport", ">= 4.2"
Expand Down