Merge pull request #13 from rails-front-end/support-manifests-whose-v…

…alues-include-assets-prefix

Support manifests whose values already include the assets prefix

README.md

@@ -89,6 +89,45 @@ config.external_asset_pipeline.assets_prefix = '/static'
 config.external_asset_pipeline.manifest_filename = '.asset-manifest.json'
 ```
 
+### Usage with a manifest whose values already include the `assets_prefix`
+
+By default, the `external_asset_pipeline` assumes that the manifest file
+contains values which are not already prefixed by the `assets_prefix`. For
+example, it assumes the manifest looks like:
+
+```json
+{
+  "application.css": "application-2ea8c3891d.css",
+  "application.js": "application-7b3dc2436f7956c77987.js"
+}
+```
+
+rather than:
+
+```json
+{
+  "application.css": "/packs/application-2ea8c3891d.css",
+  "application.js": "/packs/application-7b3dc2436f7956c77987.js"
+}
+```
+
+This assumption aligns with the default behavior of the
+[`webpack-assets-manifest` plugin] when no options are passed. However, if your
+manifest values include the assets prefix, as in the latter example (e.g. if you
+set the `publicPath` option to `true` or to `'/packs/'` when instantiating the
+`WebpackAssetsManifest` plugin), then you should set the
+`prepend_assets_prefix_to_manifest_values` configuration option to `false`:
+
+```ruby
+config.external_asset_pipeline.prepend_assets_prefix_to_manifest_values = false
+```
+
+This will instruct the `external_asset_pipeline` to _not_ prepend the assets
+prefix to your manifest values (otherwise, it would return doubly-prefixed paths
+like `/packs//packs/application-2ea8c3891d.css`).
+
+[`webpack-assets-manifest` plugin]: https://github.com/webdeveric/webpack-assets-manifest/blob/v3.1.1/readme.md#publicpath
+
 ### Using with a dev server
 
 You may also connect the `external_asset_pipeline` to a dev server (e.g.

lib/external_asset_pipeline/configuration.rb

@@ -10,6 +10,7 @@ class Configuration
                   :fall_back_to_sprockets,
                   :logger,
                   :manifest_filename,
+                  :prepend_assets_prefix_to_manifest_values,
                   :public_path
 
     alias cache_manifest? cache_manifest
@@ -34,6 +35,11 @@ def initialize
       @fall_back_to_sprockets = false
       @logger = Logger.new(STDOUT)
       @manifest_filename = 'manifest.json'
+      @prepend_assets_prefix_to_manifest_values = true
+    end
+
+    def manifest_value_prefix
+      @prepend_assets_prefix_to_manifest_values ? "#{@assets_prefix}/" : ''
     end
 
     def configure

lib/external_asset_pipeline/manifest.rb

@@ -10,7 +10,7 @@ def initialize(config)
 
     def find(name)
       value = data[name.to_s]
-      { path: "#{@config.assets_prefix}/#{value}" } if value
+      { path: "#{@config.manifest_value_prefix}#{value}" } if value
     end
 
     def fall_back_to_sprockets?

test/external_asset_pipeline/configuration_test.rb

@@ -9,7 +9,7 @@ module ExternalAssetPipeline
   class ConfigurationTest < Minitest::Test
     TEST_APP_PUBLIC_PATH = File.expand_path('../test_app/public', __dir__)
 
-    def test_configure
+    def test_configure # rubocop:disable Metrics/MethodLength
       config = Configuration.new
 
       assert_equal '/packs', config.assets_prefix
@@ -18,13 +18,15 @@ def test_configure
       assert_equal false, config.fall_back_to_sprockets
       assert_equal false, config.fall_back_to_sprockets?
       assert_equal 'manifest.json', config.manifest_filename
+      assert_equal true, config.prepend_assets_prefix_to_manifest_values
       assert_nil config.public_path
 
       result = config.configure do |c|
         c.assets_prefix = '/assets'
         c.cache_manifest = false
         c.fall_back_to_sprockets = true
         c.manifest_filename = '.revisioned-asset-manifest.json'
+        c.prepend_assets_prefix_to_manifest_values = false
         c.public_path = Pathname.new(TEST_APP_PUBLIC_PATH)
       end
 
@@ -35,6 +37,7 @@ def test_configure
       assert_equal true, config.fall_back_to_sprockets
       assert_equal true, config.fall_back_to_sprockets?
       assert_equal '.revisioned-asset-manifest.json', config.manifest_filename
+      assert_equal false, config.prepend_assets_prefix_to_manifest_values
       assert_equal Pathname.new(TEST_APP_PUBLIC_PATH), config.public_path
     end
 
@@ -96,5 +99,15 @@ def test_manifest_path
 
       assert_equal expected_manifest_path, config.manifest_path
     end
+
+    def test_manifest_value_prefix
+      config = Configuration.new
+
+      assert_equal '/packs/', config.manifest_value_prefix
+
+      config.prepend_assets_prefix_to_manifest_values = false
+
+      assert_equal '', config.manifest_value_prefix
+    end
   end
 end

test/external_asset_pipeline/manifest_test.rb

@@ -22,6 +22,30 @@ def test_find
       assert_nil manifest.find('missing-asset.css')
     end
 
+    def test_find_when_manifest_values_include_prefix
+      config = Configuration.new
+      config.public_path = Pathname.new(TEST_APP_PUBLIC_PATH)
+      config.manifest_filename = 'manifest_with_prefixed_values.json'
+
+      manifest = Manifest.new(config)
+
+      # The assets prefix will always be prepended unless
+      # `config.prepend_assets_prefix_to_manifest_values` is set to `false`
+      assert_equal '/packs//packs/application-7b3dc2436f7956c77987.js',
+                   manifest.find('application.js')[:path]
+      assert_equal '/packs//packs/application-2ea8c3891d.css',
+                   manifest.find('application.css')[:path]
+      assert_nil manifest.find('missing-asset.css')
+
+      config.prepend_assets_prefix_to_manifest_values = false
+
+      assert_equal '/packs/application-7b3dc2436f7956c77987.js',
+                   manifest.find('application.js')[:path]
+      assert_equal '/packs/application-2ea8c3891d.css',
+                   manifest.find('application.css')[:path]
+      assert_nil manifest.find('missing-asset.css')
+    end
+
     def test_manifest_caching
       config = Configuration.new
       config.public_path = Pathname.new(TEST_APP_PUBLIC_PATH)

test/test_app/public/packs/manifest_with_prefixed_values.json

@@ -0,0 +1,4 @@
+{
+  "application.css": "/packs/application-2ea8c3891d.css",
+  "application.js": "/packs/application-7b3dc2436f7956c77987.js"
+}