Make assets_preload more strict

Prevent suggesting the usage of preload_tag for third party assets. Fixes Shopify#693.
eloyesp · Feb 23, 2023 · dcb4021 · dcb4021
1 parent 6b80093
commit dcb4021
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 6 deletions.
diff --git a/lib/theme_check/checks/asset_preload.rb b/lib/theme_check/checks/asset_preload.rb
@@ -6,10 +6,12 @@ class AssetPreload < HtmlCheck
     doc docs_url(__FILE__)
 
     def on_link(node)
-      return if node.attributes["rel"]&.downcase != "preload"
+      return unless node.attributes["rel"]&.downcase == "preload"
+      return if node.literal? # literal urls cannot use the preload_tag
+      return unless node.attributes["href"]&.match?(/(?:assets?|image|file)_url\b/i)
       case node.attributes["as"]&.downcase
       when "font"
-        # Ignored as it is not supported
+        # fonts are not supported yet
       when "style"
         add_offense("For better performance, prefer using the preload argument of the stylesheet_tag filter", node: node)
       when "image"

diff --git a/test/checks/asset_preload_test.rb b/test/checks/asset_preload_test.rb
@@ -6,7 +6,7 @@ def test_no_offense_with_link_element
     offenses = analyze_theme(
       ThemeCheck::AssetPreload.new,
       "templates/index.liquid" => <<~END,
-        <link href="a.css" rel="stylesheet">
+        <link href="{{ 'a.css' | asset_url }}" rel="stylesheet">
         <link href="b.com" rel="preconnect">
       END
     )
@@ -23,11 +23,21 @@ def test_no_offense_for_font_preload
     assert_offenses("", offenses)
   end
 
+  def test_no_warning_for_external_assets
+    offenses = analyze_theme(
+      ThemeCheck::AssetPreload.new,
+      "templates/index.liquid" => <<~END,
+        <link rel="preload" as="script" href="https://ajax.googleapis.com/ajax/libs/jquery/3.6.0/jquery.min.js">
+      END
+    )
+    assert_offenses("", offenses)
+  end
+
   def test_reports_stylesheet_preloading
     offenses = analyze_theme(
       ThemeCheck::AssetPreload.new,
       "templates/index.liquid" => <<~END,
-        <link href="a.css" rel="preload" as="style">
+        <link href="{{ 'a.css' | asset_url }}" rel="preload" as="style">
       END
     )
     assert_offenses(<<~END, offenses)
@@ -39,7 +49,7 @@ def test_reports_image_preloading
     offenses = analyze_theme(
       ThemeCheck::AssetPreload.new,
       "templates/index.liquid" => <<~END,
-        <link href="a.png" rel="preload" as="image">
+        <link href="{{ 'a.png' | image_url }}" rel="preload" as="image">
       END
     )
     assert_offenses(<<~END, offenses)
@@ -51,7 +61,7 @@ def test_reports_general_preloading
     offenses = analyze_theme(
       ThemeCheck::AssetPreload.new,
       "templates/index.liquid" => <<~END,
-        <link href="a..js" rel="preload" as="script">
+        <link href="{{ 'script.js' | asset_url  }}" rel="preload" as="script">
       END
     )
     assert_offenses(<<~END, offenses)