Fix an issue in shopify theme dev that was affecting asset loading …

…on local servers
williamlfish · Jun 7, 2023 · 99fc03f · 99fc03f
1 parent e990261
commit 99fc03f
Show file tree

Hide file tree

Showing 8 changed files with 92 additions and 11 deletions.
diff --git a/.changeset/pink-wasps-marry.md b/.changeset/pink-wasps-marry.md
@@ -0,0 +1,6 @@
+---
+'@shopify/cli-kit': patch
+'@shopify/theme': patch
+---
+
+Fix an issue in `shopify theme dev` that was affecting asset loading on local servers, in some shops
diff --git a/packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/dev_server.rb b/packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/dev_server.rb
@@ -168,7 +168,7 @@ def start_server
 
       def middleware_stack
         @app = Proxy.new(ctx, theme, param_builder)
-        @app = CdnFonts.new(@app, theme: theme)
+        @app = CdnFonts.new(ctx, @app, theme: theme)
         @app = LocalAssets.new(ctx, @app, theme)
         @app = HotReload.new(ctx, @app, broadcast_hooks: broadcast_hooks, watcher: watcher, mode: mode,
           script_injector: script_injector)

diff --git a/packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/dev_server/cdn_fonts.rb b/packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/dev_server/cdn_fonts.rb
@@ -6,9 +6,9 @@ class DevServer
       class CdnFonts
         FONTS_PATH = "/fonts"
         FONTS_CDN = "https://fonts.shopifycdn.com"
-        FONTS_REGEX = %r{#{FONTS_CDN}}
 
-        def initialize(app, theme:)
+        def initialize(ctx, app, theme:)
+          @ctx = ctx
           @app = app
           @theme = theme
         end
@@ -69,7 +69,12 @@ def fonts_cdn_uri(path, query)
         end
 
         def replace_font_urls(body)
-          [body.join.gsub(FONTS_REGEX, FONTS_PATH)]
+          fonts_regex = %r{#{FONTS_CDN}|((http:|https:)?//#{shop}/cdn/fonts)}
+          [body.join.gsub(fonts_regex, FONTS_PATH)]
+        end
+
+        def shop
+          @shop ||= ShopifyCLI::Theme::ThemeAdminAPI.new(@ctx).get_shop_or_abort
         end
       end
     end

diff --git a/packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/dev_server/local_assets.rb b/packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/dev_server/local_assets.rb
@@ -5,8 +5,8 @@ module Theme
     class DevServer
       class LocalAssets
         SUPPORTED_EXTENSIONS = [:jpg, :jpeg, :js, :css, :png, :svg].join("|")
-        THEME_REGEX = %r{//cdn\.shopify\.com/s/.+?/(assets/.+?\.(?:#{SUPPORTED_EXTENSIONS}))}
-        VANITY_THEME_REGEX = %r{/cdn/shop/.+?/(assets/.+?\.(?:#{SUPPORTED_EXTENSIONS}))}
+        CDN_REGEX = %r{(//cdn)\.shopify\.com/s/.+?/(assets/.+?\.(?:#{SUPPORTED_EXTENSIONS}))}
+        VANITY_CDN_REGEX = %r{(/cdn)/shop/.+?/(assets/.+?\.(?:#{SUPPORTED_EXTENSIONS}))}
 
         class FileBody
           def initialize(path)
@@ -43,13 +43,17 @@ def call(env)
           end
         end
 
+        def shop_regex
+          %r{(http:|https:)?//#{shop}/(assets/.+?\.(?:#{SUPPORTED_EXTENSIONS}))}
+        end
+
         private
 
         def replace_asset_urls(body)
           replaced_body = body.join
-          [THEME_REGEX, VANITY_THEME_REGEX].each do |regex|
+          [CDN_REGEX, VANITY_CDN_REGEX, shop_regex].each do |regex|
             replaced_body = replaced_body.gsub(regex) do |match|
-              path = Regexp.last_match[1]
+              path = Regexp.last_match[2]
               @target.static_asset_paths.include?(path) ? "/#{path}" : match
             end
           end
@@ -87,6 +91,10 @@ def serve_file(path_info)
             serve_fail(404, "Not found")
           end
         end
+
+        def shop
+          @shop ||= ShopifyCLI::Theme::ThemeAdminAPI.new(@ctx).get_shop_or_abort
+        end
       end
     end
   end

diff --git a/packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/extension/dev_server.rb b/packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/extension/dev_server.rb
@@ -44,7 +44,7 @@ def start(ctx, root, port: 9292, theme: nil, generate_tmp_theme: false, project:
 
         def middleware_stack
           @app = Proxy.new(ctx, theme, param_builder)
-          @app = CdnFonts.new(@app, theme: theme)
+          @app = CdnFonts.new(ctx, @app, theme: theme)
           @app = LocalAssets.new(ctx, @app, extension)
           @app = HotReload.new(ctx, @app, broadcast_hooks: broadcast_hooks, watcher: watcher, mode: mode,
             script_injector: script_injector)

diff --git a/packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/repl/auth_dev_server.rb b/packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/repl/auth_dev_server.rb
@@ -57,7 +57,7 @@ def create_repl_theme
 
         def middleware_stack
           @app = proxy
-          @app = CdnFonts.new(app, theme: theme)
+          @app = CdnFonts.new(ctx, app, theme: theme)
           @app = AuthMiddleware.new(app, proxy, repl) { WebServer.shutdown }
         end
 

diff --git a/packages/cli-kit/assets/cli-ruby/test/shopify-cli/theme/dev_server/cdn_fonts_test.rb b/packages/cli-kit/assets/cli-ruby/test/shopify-cli/theme/dev_server/cdn_fonts_test.rb
@@ -7,6 +7,11 @@ module ShopifyCLI
   module Theme
     class DevServer
       class CdnFontsTest < Minitest::Test
+        def setup
+          super
+          Environment.stubs(:store).returns("my-test-shop.myshopify.com")
+        end
+
         def test_replace_local_assistant_n4_font_in_reponse_body
           original_html = <<~HTML
             <html>
@@ -61,6 +66,28 @@ def test_replace_local_fonts_on_same_line
           assert_equal(expected_html, serve(original_html).body)
         end
 
+        def test_replace_shop_fonts_urls_in_reponse_body
+          original_html = <<~HTML
+            <html>
+              <head>
+              <link rel="preload" as="font" href="//my-test-shop.myshopify.com/cdn/fonts/assistant/assistant_n4.2222.woff2?h1=1111&hmac=0000" type="font/woff2" crossorigin="">
+              <link rel="preload" as="font" href="http://my-test-shop.myshopify.com/cdn/fonts/assistant/assistant_n4.2222.woff2?h1=1111&hmac=0000" type="font/woff2" crossorigin="">
+              <link rel="preload" as="font" href="https://my-test-shop.myshopify.com/cdn/fonts/assistant/assistant_n4.2222.woff2?h1=1111&hmac=0000" type="font/woff2" crossorigin="">
+              </head>
+            </html>
+          HTML
+          expected_html = <<~HTML
+            <html>
+              <head>
+              <link rel="preload" as="font" href="/fonts/assistant/assistant_n4.2222.woff2?h1=1111&hmac=0000" type="font/woff2" crossorigin="">
+              <link rel="preload" as="font" href="/fonts/assistant/assistant_n4.2222.woff2?h1=1111&hmac=0000" type="font/woff2" crossorigin="">
+              <link rel="preload" as="font" href="/fonts/assistant/assistant_n4.2222.woff2?h1=1111&hmac=0000" type="font/woff2" crossorigin="">
+              </head>
+            </html>
+          HTML
+          assert_equal(expected_html, serve(original_html).body)
+        end
+
         def test_dont_replace_other_assets
           original_html = <<~HTML
             <html>
@@ -122,10 +149,11 @@ def test_replace_when_content_type_does_match_text
         private
 
         def serve(response_body = "", path: "/", content_type: "text/html")
+          ctx = TestHelpers::FakeContext.new(root: root)
           app = lambda do |_env|
             [200, { "Content-Type" => content_type }, [response_body]]
           end
-          stack = CdnFonts.new(app, theme: theme)
+          stack = CdnFonts.new(ctx, app, theme: theme)
           request = Rack::MockRequest.new(stack)
           request.get(path)
         end
@@ -136,6 +164,7 @@ def root
 
         def theme
           return @theme if @theme
+
           @theme = Theme.new(nil, root: root)
           @theme.stubs(shop: "my-test-shop.myshopify.com")
           @theme

diff --git a/packages/cli-kit/assets/cli-ruby/test/shopify-cli/theme/dev_server/local_assets_test.rb b/packages/cli-kit/assets/cli-ruby/test/shopify-cli/theme/dev_server/local_assets_test.rb
@@ -8,6 +8,11 @@ module ShopifyCLI
   module Theme
     class DevServer
       class LocalAssetsTest < Minitest::Test
+        def setup
+          super
+          Environment.stubs(:store).returns("my-test-shop.myshopify.com")
+        end
+
         def test_replace_local_assets_in_reponse_body
           original_html = <<~HTML
             <html>
@@ -149,6 +154,34 @@ def test_replace_local_images_in_reponse_body
           assert_equal(expected_html, serve(original_html, theme_mock: theme).body)
         end
 
+        def test_replace_shop_assets_urls_in_reponse_body
+          theme = stub("Theme", static_asset_paths: [
+            "assets/component-list-menu.css",
+          ])
+
+          original_html = <<~HTML
+            <html>
+              <head>
+              <link rel="stylesheet" href="//my-test-shop.myshopify.com/assets/component-list-menu.css?v=11111" media="print" onload="this.media='all'">
+              <link rel="stylesheet" href="http://my-test-shop.myshopify.com/assets/component-list-menu.css?v=11111" media="print" onload="this.media='all'">
+              <link rel="stylesheet" href="https://my-test-shop.myshopify.com/assets/component-list-menu.css?v=11111" media="print" onload="this.media='all'">
+              </head>
+            </html>
+          HTML
+
+          expected_html = <<~HTML
+            <html>
+              <head>
+              <link rel="stylesheet" href="/assets/component-list-menu.css?v=11111" media="print" onload="this.media='all'">
+              <link rel="stylesheet" href="/assets/component-list-menu.css?v=11111" media="print" onload="this.media='all'">
+              <link rel="stylesheet" href="/assets/component-list-menu.css?v=11111" media="print" onload="this.media='all'">
+              </head>
+            </html>
+          HTML
+
+          assert_equal(expected_html, serve(original_html, theme_mock: theme).body)
+        end
+
         private
 
         def serve(response_body, path: "/", theme_mock: nil)