Skip to content
This repository has been archived by the owner on Oct 20, 2021. It is now read-only.

add webpacker assets to local files for storage if webpacker config set #1

Merged
merged 4 commits into from
Jun 6, 2017
Merged
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
3 changes: 3 additions & 0 deletions lib/asset_sync/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class Invalid < StandardError; end
attr_accessor :invalidate
attr_accessor :cdn_distribution_id
attr_accessor :cache_asset_regexps
attr_accessor :include_webpacker_assets

# FOG configuration
attr_accessor :fog_provider # Currently Supported ['AWS', 'Rackspace']
Expand Down Expand Up @@ -65,6 +66,7 @@ def initialize
self.enabled = true
self.run_on_precompile = true
self.cdn_distribution_id = nil
self.include_webpacker_assets = false
self.invalidate = []
self.cache_asset_regexps = []
load_yml! if defined?(::Rails) && yml_exists?
Expand Down Expand Up @@ -167,6 +169,7 @@ def load_yml!
self.invalidate = yml["invalidate"] if yml.has_key?("invalidate")
self.cdn_distribution_id = yml['cdn_distribution_id'] if yml.has_key?("cdn_distribution_id")
self.cache_asset_regexps = yml['cache_asset_regexps'] if yml.has_key?("cache_asset_regexps")
self.include_webpacker_assets = yml['include_webpacker_assets'] if yml.has_key?("include_webpacker_assets")

# TODO deprecate the other old style config settings. FML.
self.aws_access_key_id = yml["aws_access_key"] if yml.has_key?("aws_access_key")
Expand Down
1 change: 1 addition & 0 deletions lib/asset_sync/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class Engine < Rails::Engine
config.google_storage_secret_access_key = ENV['GOOGLE_STORAGE_SECRET_ACCESS_KEY'] if ENV.has_key?('GOOGLE_STORAGE_SECRET_ACCESS_KEY')

config.enabled = (ENV['ASSET_SYNC_ENABLED'] == 'true') if ENV.has_key?('ASSET_SYNC_ENABLED')
config.include_webpacker_assets = (ENV['ASSET_SYNC_INCLUDE_WEBPACKER_ASSETS'] == 'true') if ENV.has_key?('ASSET_SYNC_INCLUDE_WEBPACKER_ASSETS')

config.existing_remote_files = ENV['ASSET_SYNC_EXISTING_REMOTE_FILES'] || "keep"

Expand Down
16 changes: 14 additions & 2 deletions lib/asset_sync/storage.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,13 @@ def get_local_files
if ActionView::Base.respond_to?(:assets_manifest)
log "Using: Rails 4.0 manifest access"
manifest = Sprockets::Manifest.new(ActionView::Base.assets_manifest.environment, ActionView::Base.assets_manifest.dir)
return manifest.assets.values.map { |f| File.join(self.config.assets_prefix, f) }
files = manifest.assets.values.map { |f| File.join(self.config.assets_prefix, f) }

if self.config.include_webpacker_assets && defined?(Webpacker)
files += Dir[File.join(Webpacker::Configuration.paths.fetch(:entry, 'packs'), '/**/**')]
end

return files
elsif File.exist?(self.config.manifest_path)
log "Using: Manifest #{self.config.manifest_path}"
yml = YAML.load(IO.read(self.config.manifest_path))
Expand All @@ -82,7 +88,13 @@ def get_local_files
log "Using: Directory Search of #{path}/#{self.config.assets_prefix}"
Dir.chdir(path) do
to_load = self.config.assets_prefix.present? ? "#{self.config.assets_prefix}/**/**" : '**/**'
Dir[to_load]
files = Dir[to_load]

if self.config.include_webpacker_assets && defined?(Webpacker)
files += Dir[File.join(Webpacker::Configuration.paths.fetch(:entry, 'packs'), '/**/**')]
end

files
end
end

Expand Down
15 changes: 14 additions & 1 deletion lib/tasks/asset_sync.rake
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,24 @@ if Rake::Task.task_defined?("assets:precompile:nondigest")
Rake::Task["assets:sync"].invoke if defined?(AssetSync) && AssetSync.config.run_on_precompile
end
else
# Triggers on webpacker compile task instead of assets:precompile to ensure
Copy link
Author

Choose a reason for hiding this comment

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

@jbusser I took this exactly from AssetSync#341, but I also considered this simpler approach:

if Rake::Task.task_defined?("webpacker:compile")
  Rake::Task["webpacker:compile"].enhance do
    if defined?(AssetSync) && AssetSync.config.run_on_precompile
      Rake::Task["assets:sync"].invoke
    end
  end
else
  Rake::Task["assets:precompile"].enhance do
    if defined?(AssetSync) && AssetSync.config.run_on_precompile
      Rake::Task["assets:sync"].invoke
    end
  end
end

It seems strange to me to check AssetSync.config.include_webpacker_assets, since we're already doing that inside of assets:sync. I also feel like it could get us into a situation where assets just don't get synced at all if Rake::Task.task_defined?("webpacker:compile") returns false, but AssetSync.config.include_webpacker_assets returns true (see line 39).

Anyway, I think it may be better to just keep this part simple to focus on which task we're enhancing and make sure assets get synced either way. Let me know what you think!

Copy link

Choose a reason for hiding this comment

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

Ah, I think I understand what's going on here. I thought your simpler version would work, but I think the implementation we stole that inspired this fork is probably better.

The intention is to ensure that we enhance either the "webpacker:compile" or "assets:precompile" tasks but never both. I have no idea why someone would have the webpacker gem installed and then say "nah, don't sync that like all my other assets," but here we are.

Copy link
Author

@cameronjacoby cameronjacoby Jun 6, 2017

Choose a reason for hiding this comment

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

The intention is to ensure that we enhance either the "webpacker:compile" or "assets:precompile" tasks but never both.

Agreed, and I think the implementation that "inspired" this PR does that with FashionThing's setup (since Rake::Task.task_defined?("webpacker:compile") and AssetSync.config.include_webpacker_assets will both be true), so I'll leave it as is. 👍

# that syncing is done after all assets are compiled
# (webpacker:compile already enhances assets:precompile)
if Rake::Task.task_defined?("webpacker:compile")
Rake::Task["webpacker:compile"].enhance do
if defined?(AssetSync) && AssetSync.config.run_on_precompile && AssetSync.config.include_webpacker_assets
Rake::Task["assets:sync"].invoke
end
end
end

Rake::Task["assets:precompile"].enhance do
# rails 3.1.1 will clear out Rails.application.config if the env vars
# RAILS_GROUP and RAILS_ENV are not defined. We need to reload the
# assets environment in this case.
# Rake::Task["assets:environment"].invoke if Rake::Task.task_defined?("assets:environment")
Rake::Task["assets:sync"].invoke if defined?(AssetSync) && AssetSync.config.run_on_precompile
if defined?(AssetSync) && AssetSync.config.run_on_precompile && !AssetSync.config.include_webpacker_assets
Rake::Task["assets:sync"].invoke
end
end
end
Empty file.
4 changes: 4 additions & 0 deletions spec/unit/asset_sync_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@
expect(AssetSync.config.cdn_distribution_id).to be_nil
end

it "should default include_webpacker_assets to false" do
expect(AssetSync.config.include_webpacker_assets).to be_falsey
end

it "should default invalidate to empty array" do
expect(AssetSync.config.invalidate).to eq([])
end
Expand Down
34 changes: 34 additions & 0 deletions spec/unit/storage_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,40 @@
describe AssetSync::Storage do
include_context "mock Rails without_yml"

describe '#local_files' do
context 'default' do
let(:config) { AssetSync::Config.new }
let(:storage) { AssetSync::Storage.new(config) }

it 'returns directories and files inside public/assets' do
expect(storage.local_files).to eq([
'assets/javascripts',
'assets/javascripts/application.js',
])
end
end

context 'with include_webpacker_assets = true' do
let(:config) do
config = AssetSync::Config.new
config.include_webpacker_assets = true
config
end

let(:webpacker_config_stub) { Class.new }
let(:storage) { AssetSync::Storage.new(config) }

before do
stub_const('Webpacker::Configuration', webpacker_config_stub)
allow(webpacker_config_stub).to receive(:paths).and_return({})
end

it 'includes files inside public/packs' do
expect(storage.local_files).to include('packs/manifest.json')
end
end
end

describe '#upload_files' do
before(:each) do
@local_files = ["local_image2.jpg", "local_image1.jpg", "local_stylesheet1.css", "local_stylesheet2.css"]
Expand Down