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

Refactor singleton usage #1808

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion lib/install/bin/webpack
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,7 @@ require "bundler/setup"

require "webpacker"
require "webpacker/webpack_runner"
Webpacker::WebpackRunner.run(ARGV)

Dir.chdir(File.expand_path("../../", __FILE__)) do
Webpacker::WebpackRunner.run(ARGV)
end
5 changes: 4 additions & 1 deletion lib/install/bin/webpack-dev-server
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,7 @@ require "bundler/setup"

require "webpacker"
require "webpacker/dev_server_runner"
Webpacker::DevServerRunner.run(ARGV)

Dir.chdir(File.expand_path("../../", __FILE__)) do
Webpacker::DevServerRunner.run(ARGV)
end
16 changes: 12 additions & 4 deletions lib/webpacker/compiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class Webpacker::Compiler
# Webpacker::Compiler.env['FRONTEND_API_KEY'] = 'your_secret_key'
cattr_accessor(:env) { {} }

delegate :config, :logger, to: :@webpacker
delegate :config, :logger, to: :webpacker

def initialize(webpacker)
@webpacker = webpacker
Expand Down Expand Up @@ -38,6 +38,8 @@ def stale?
end

private
attr_reader :webpacker

def last_compilation_digest
compilation_digest_path.read if compilation_digest_path.exist? && config.public_manifest_path.exist?
end
Expand All @@ -60,7 +62,11 @@ def remove_compilation_digest
def run_webpack
logger.info "Compiling…"

stdout, sterr , status = Open3.capture3(webpack_env, "#{RbConfig.ruby} ./bin/webpack")
stdout, sterr , status = Open3.capture3(
webpack_env,
"#{RbConfig.ruby} ./bin/webpack",
chdir: File.expand_path(config.root_path)
)

if status.success?
logger.info "Compiled all packs in #{config.public_output_path}"
Expand All @@ -75,17 +81,19 @@ def run_webpack
def default_watched_paths
[
*config.resolved_paths_globbed,
"#{config.source_path.relative_path_from(Rails.root)}/**/*",
"#{config.source_path.relative_path_from(config.root_path)}/**/*",
"yarn.lock", "package.json",
"config/webpack/**/*"
].freeze
end

def compilation_digest_path
config.cache_path.join(".last-compilation-digest-#{Webpacker.env}")
config.cache_path.join(".last-compilation-digest-#{webpacker.env}")
end

def webpack_env
return env unless defined?(ActionController::Base)

env.merge("WEBPACKER_ASSET_HOST" => ActionController::Base.helpers.compute_asset_host,
"WEBPACKER_RELATIVE_URL_ROOT" => ActionController::Base.relative_url_root)
end
Expand Down
8 changes: 7 additions & 1 deletion lib/webpacker/dev_server.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
class Webpacker::DevServer
DEFAULT_ENV_PREFIX = "WEBPACKER_DEV_SERVER".freeze

# Configure dev server connection timeout (in seconds), default: 0.01
# Webpacker.dev_server.connect_timeout = 1
cattr_accessor(:connect_timeout) { 0.01 }
Expand Down Expand Up @@ -58,9 +60,13 @@ def pretty?
fetch(:pretty)
end

def env_prefix
config.dev_server.fetch(:env_prefix, DEFAULT_ENV_PREFIX)
end

private
def fetch(key)
ENV["WEBPACKER_DEV_SERVER_#{key.upcase}"] || config.dev_server.fetch(key, defaults[key])
ENV["#{env_prefix}_#{key.upcase}"] || config.dev_server.fetch(key, defaults[key])
end

def defaults
Expand Down
19 changes: 13 additions & 6 deletions lib/webpacker/dev_server_proxy.rb
Original file line number Diff line number Diff line change
@@ -1,18 +1,25 @@
require "rack/proxy"

class Webpacker::DevServerProxy < Rack::Proxy
delegate :config, :dev_server, to: :@webpacker

def initialize(app = nil, opts = {})
@webpacker = opts.delete(:webpacker) || Webpacker.instance
super
end

def rewrite_response(response)
_status, headers, _body = response
headers.delete "transfer-encoding"
headers.delete "content-length" if Webpacker.dev_server.running? && Webpacker.dev_server.https?
headers.delete "content-length" if dev_server.running? && dev_server.https?
response
end

def perform_request(env)
if env["PATH_INFO"].start_with?("/#{public_output_uri_path}") && Webpacker.dev_server.running?
env["HTTP_HOST"] = env["HTTP_X_FORWARDED_HOST"] = env["HTTP_X_FORWARDED_SERVER"] = Webpacker.dev_server.host_with_port
env["HTTP_X_FORWARDED_PROTO"] = env["HTTP_X_FORWARDED_SCHEME"] = Webpacker.dev_server.protocol
unless Webpacker.dev_server.https?
if env["PATH_INFO"].start_with?("/#{public_output_uri_path}") && dev_server.running?
env["HTTP_HOST"] = env["HTTP_X_FORWARDED_HOST"] = env["HTTP_X_FORWARDED_SERVER"] = dev_server.host_with_port
env["HTTP_X_FORWARDED_PROTO"] = env["HTTP_X_FORWARDED_SCHEME"] = dev_server.protocol
unless dev_server.https?
env["HTTPS"] = env["HTTP_X_FORWARDED_SSL"] = "off"
end
env["SCRIPT_NAME"] = ""
Expand All @@ -25,6 +32,6 @@ def perform_request(env)

private
def public_output_uri_path
Webpacker.config.public_output_path.relative_path_from(Webpacker.config.public_path)
config.public_output_path.relative_path_from(config.public_path)
end
end
21 changes: 14 additions & 7 deletions lib/webpacker/helper.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
module Webpacker::Helper
# Returns current Webpacker instance.
# Could be overriden to use multiple Webpacker
# configurations within the same app (e.g. with engines)
def current_webpacker
Copy link
Member

Choose a reason for hiding this comment

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

I would just call Webpacker.instance directly instead of putting into a method. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The goal is to make it possible to override the Webpacker instance, so in the engine's helper we do:

require "webpacker/helper"

module MyEngine
  module ApplicationHelper
    include ::Webpacker::Helper
    
    def current_webpacker
       MyEngine.webpacker # or whatever
    end
  end
end

Webpacker.instance would point to global Webpacker (i.e. ::Webpacker.instance), so that won't work

Webpacker.instance
end

# Computes the relative path for a given Webpacker asset.
# Return relative path using manifest.json and passes it to asset_path helper
# This will use asset_path internally, so most of their behaviors will be the same.
Expand All @@ -11,8 +18,8 @@ module Webpacker::Helper
# # In production mode:
# <%= asset_pack_path 'calendar.css' %> # => "/packs/calendar-1016838bab065ae1e122.css"
def asset_pack_path(name, **options)
unless stylesheet?(name) && Webpacker.dev_server.running? && Webpacker.dev_server.hot_module_replacing?
asset_path(Webpacker.manifest.lookup!(name), **options)
unless stylesheet?(name) && current_webpacker.dev_server.running? && current_webpacker.dev_server.hot_module_replacing?
asset_path(current_webpacker.manifest.lookup!(name), **options)
end
end

Expand All @@ -28,8 +35,8 @@ def asset_pack_path(name, **options)
# # In production mode:
# <%= asset_pack_url 'calendar.css' %> # => "http://example.com/packs/calendar-1016838bab065ae1e122.css"
def asset_pack_url(name, **options)
unless Webpacker.dev_server.running? && Webpacker.dev_server.hot_module_replacing?
asset_url(Webpacker.manifest.lookup!(name), **options)
unless current_webpacker.dev_server.running? && current_webpacker.dev_server.hot_module_replacing?
asset_url(current_webpacker.manifest.lookup!(name), **options)
end
end

Expand All @@ -40,7 +47,7 @@ def asset_pack_url(name, **options)
# <%= image_pack_tag 'application.png', size: '16x10', alt: 'Edit Entry' %>
# <img alt='Edit Entry' src='/packs/application-k344a6d59eef8632c9d1.png' width='16' height='10' />
def image_pack_tag(name, **options)
image_tag(asset_path(Webpacker.manifest.lookup!(name)), **options)
image_tag(asset_path(current_webpacker.manifest.lookup!(name)), **options)
end

# Creates a script tag that references the named pack file, as compiled by webpack per the entries list
Expand Down Expand Up @@ -72,7 +79,7 @@ def javascript_pack_tag(*names, **options)
# <%= stylesheet_pack_tag 'calendar', 'data-turbolinks-track': 'reload' %> # =>
# <link rel="stylesheet" media="screen" href="/packs/calendar-1016838bab065ae1e122.css" data-turbolinks-track="reload" />
def stylesheet_pack_tag(*names, **options)
unless Webpacker.dev_server.running? && Webpacker.dev_server.hot_module_replacing?
unless current_webpacker.dev_server.running? && current_webpacker.dev_server.hot_module_replacing?
stylesheet_link_tag(*sources_from_pack_manifest(names, type: :stylesheet), **options)
end
end
Expand All @@ -83,7 +90,7 @@ def stylesheet?(name)
end

def sources_from_pack_manifest(names, type:)
names.map { |name| Webpacker.manifest.lookup!(pack_name_with_extension(name, type: type)) }
names.map { |name| current_webpacker.manifest.lookup!(pack_name_with_extension(name, type: type)) }
end

def pack_name_with_extension(name, type:)
Expand Down
2 changes: 1 addition & 1 deletion test/compiler_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def test_default_watched_paths
assert_equal Webpacker.compiler.send(:default_watched_paths), [
"app/assets/**/*",
"/etc/yarn/**/*",
"test/test_app/app/javascript/**/*",
"app/javascript/**/*",
"yarn.lock",
"package.json",
"config/webpack/**/*"
Expand Down