Add AssetSizeJavascript Check

RELEASING.md

@@ -2,7 +2,7 @@
 
 1. Check the Semantic Versioning page for info on how to version the new release: http://semver.org
 
-2. Create a PR to update the version in `lib/theme_check/version.rb`
+2. Create a PR to update the version in `lib/theme_check/version.rb` and replace the `THEME_CHECK_VERSION` placeholder in the documentation for new rules.
 
 3. Merge your PR to master
 

config/default.yml

@@ -16,6 +16,7 @@ MissingTemplate:
 
 NestedSnippet:
   enabled: true
+  max_nesting_level: 2
 
 RequiredLayoutThemeObject:
   enabled: true
@@ -80,3 +81,7 @@ MissingEnableComment:
 
 ParserBlockingJavaScript:
   enabled: true
+
+AssetSizeJavaScript:
+  enabled: false
+  threshold_in_bytes: 10000

docs/checks/asset_size_javascript.md

@@ -0,0 +1,79 @@
+# Prevent JavaScript Abuse on Server Rendered Themes (`AssetSizeJavaScript`)
+
+For server rendered pages, it is an anti-pattern to execute large JavaScript bundles on every navigation.
+
+This doesn't mean they don't have a reason to exist. For instance, chat widgets are mini applications embedded inside web pages. Designing such an app with server rendered updates would be absurd. However, if only 10% of the users interact with the chat widget, the other 90% should not have to execute the entire bundle on every page load.
+
+The natural solution to this problem is to implement the chat widget using the [Import on Interaction Pattern][ioip].
+
+## Check Details
+
+This rule disallows the use of theme JavaScript files and external scripts to have a compressed size greater than a configured `threshold_in_bytes`.
+
+:-1: Examples of **incorrect** code for this check:
+```liquid
+<!-- Here assets/chat-widget.js is more than 10KB gzipped. -->
+<script src="{{ 'chat-widget.js' | asset_url }}" defer></script>
+
+<!-- The use of jQuery is discouraged in themes -->
+<script src="https://code.jquery.com/jquery-3.6.0.min.js" defer></script>
+```
+
+:+1: Example of **correct** code for this check:
+```liquid
+<script>
+  const chatWidgetButton = document.getElementById('#chat-widget')
+
+  chatWidgetButton.addEventListener('click', e => {
+    e.preventDefault();
+    import("{{ 'chat-widget.js' | asset_url }}")
+      .then(module => module.default)
+      .then(ChatWidget => ChatWidget.init())
+      .catch(err => {
+        console.error(err);
+      });
+  });
+</script>
+```
+
+## Check Options
+
+The default configuration is the following.
+
+```yaml
+AssetSizeJavaScript:
+  enabled: false
+  threshold_in_bytes: 10000
+```
+
+### `threshold_in_bytes`
+
+The `threshold_in_bytes` option (default: `10000`) determines the maximum allowed compressed size in bytes that a single JavaScript file can take.
+
+This includes theme and remote scripts.
+
+## When Not To Use It
+
+When you can't do anything about it, it is preferable to disable this rule using the comment syntax:
+
+```
+{% comment %}theme-check-disable AssetSizeJavaScript{% endcomment %}
+<script src="https://code.jquery.com/jquery-3.6.0.min.js" defer></script>
+{% comment %}theme-check-enable AssetSizeJavaScript{% endcomment %}
+```
+
+This makes disabling the rule an explicit affair and shows that the code is smelly.
+
+## Version
+
+This check has been introduced in Theme Check THEME_CHECK_VERSION.
+
+## Resources
+
+- [The Impact On Interaction Pattern][ioip]
+- [Rule Source][source]
+- [Documentation Source][doc]
+
+[ioip]: https://addyosmani.com/blog/import-on-interaction/
+[codesource]: /lib/theme_check/checks/asset_size_javascript.rb
+[docsource]: /docs/checks/asset_size_javascript.md

lib/theme_check.rb

@@ -8,6 +8,9 @@
 require_relative "theme_check/disabled_checks"
 require_relative "theme_check/liquid_check"
 require_relative "theme_check/locale_diff"
+require_relative "theme_check/asset_file"
+require_relative "theme_check/remote_asset_file"
+require_relative "theme_check/regex_helpers"
 require_relative "theme_check/json_check"
 require_relative "theme_check/json_file"
 require_relative "theme_check/json_helpers"

lib/theme_check/asset_file.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+require "pathname"
+require "zlib"
+
+module ThemeCheck
+  class AssetFile
+    def initialize(relative_path, storage)
+      @relative_path = relative_path
+      @storage = storage
+      @loaded = false
+      @content = nil
+    end
+
+    def path
+      @storage.path(@relative_path)
+    end
+
+    def relative_path
+      @relative_pathname ||= Pathname.new(@relative_path)
+    end
+
+    def content
+      @content ||= @storage.read(@relative_path)
+    end
+
+    def gzipped_size
+      @gzipped_size ||= Zlib.gzip(content).bytesize
+    end
+
+    def name
+      relative_path.to_s
+    end
+  end
+end

lib/theme_check/check.rb

@@ -18,6 +18,7 @@ class Check
     CATEGORIES = [
       :liquid,
       :translation,
+      :performance,
       :json,
     ]
 

lib/theme_check/checks/asset_size_javascript.rb

@@ -0,0 +1,74 @@
+# frozen_string_literal: true
+module ThemeCheck
+  # Reports errors when trying to use too much JavaScript on page load
+  # Encourages the use of the Import on Interaction pattern [1].
+  # [1]: https://addyosmani.com/blog/import-on-interaction/
+  class AssetSizeJavaScript < LiquidCheck
+    include RegexHelpers
+    severity :error
+    category :performance
+    doc "https://github.com/Shopify/theme-check/blob/master/docs/checks/asset_size_javascript.md"
+
+    Script = Struct.new(:src, :match)
+
+    TAG = /#{Liquid::TagStart}.*?#{Liquid::TagEnd}/om
+    VARIABLE = /#{Liquid::VariableStart}.*?#{Liquid::VariableEnd}/om
+    START_OR_END_QUOTE = /(^['"])|(['"]$)/
+    SCRIPT_TAG_SRC = %r{
+      <script
+        [^>]+                              # any non closing tag character
+        src=                               # src attribute start
+        (?<src>
+          '(?:#{TAG}|#{VARIABLE}|[^']+)*'| # any combination of tag/variable or non straight quote inside straight quotes
+          "(?:#{TAG}|#{VARIABLE}|[^"]+)*"  # any combination of tag/variable or non double quotes inside double quotes
+        )
+        [^>]*                              # any non closing character till the end
+      >
+    }omix
+
+    attr_reader :threshold_in_bytes
+
+    def initialize(threshold_in_bytes: 10000)
+      @threshold_in_bytes = threshold_in_bytes
+    end
+
+    def on_document(node)
+      @node = node
+      @source = node.template.source
+      record_offenses
+    end
+
+    def record_offenses
+      scripts(@source).each do |script|
+        file_size = src_to_file_size(script.src)
+        next if file_size.nil?
+        next if file_size <= threshold_in_bytes
+        add_offense(
+          "JavaScript on every page load exceding compressed size threshold (#{threshold_in_bytes} Bytes), consider using the import on interaction pattern.",
+          node: @node,
+          markup: script.src,
+          line_number: @source[0...script.match.begin(:src)].count("\n") + 1
+        )
+      end
+    end
+
+    def scripts(source)
+      matches(source, SCRIPT_TAG_SRC)
+        .map { |m| Script.new(m[:src].gsub(START_OR_END_QUOTE, ""), m) }
+    end
+
+    def src_to_file_size(src)
+      # We're kind of intentionally only looking at {{ 'asset' | asset_url }} or full urls in here.
+      # More complicated liquid statements are not in scope.
+      if src =~ /^#{VARIABLE}$/o && src =~ /asset_url/ && src =~ Liquid::QuotedString
+        asset_id = Regexp.last_match(0).gsub(START_OR_END_QUOTE, "")
+        asset = @theme.assets.find { |a| a.name.ends_with?("/" + asset_id) }
+        return if asset.nil?
+        asset.gzipped_size
+      elsif src =~ %r{^(https?:)?//}
+        asset = RemoteAssetFile.from_src(src)
+        asset.gzipped_size
+      end
+    end
+  end
+end

lib/theme_check/language_server/completion_helper.rb

@@ -20,16 +20,6 @@ def cursor_on_first_word?(content, cursor)
       def first_word(content)
         return content.match(WORD)[0] if content.match?(WORD)
       end
-
-      def matches(s, re)
-        start_at = 0
-        matches = []
-        while (m = s.match(re, start_at))
-          matches.push(m)
-          start_at = m.end(0)
-        end
-        matches
-      end
     end
   end
 end

lib/theme_check/language_server/completion_provider.rb

@@ -4,6 +4,7 @@ module ThemeCheck
   module LanguageServer
     class CompletionProvider
       include CompletionHelper
+      include RegexHelpers
 
       class << self
         def all

lib/theme_check/regex_helpers.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+module ThemeCheck
+  module RegexHelpers
+    def matches(s, re)
+      start_at = 0
+      matches = []
+      while (m = s.match(re, start_at))
+        matches.push(m)
+        start_at = m.end(0)
+      end
+      matches
+    end
+  end
+end

lib/theme_check/remote_asset_file.rb

@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+require "net/http"
+require "pathname"
+
+module ThemeCheck
+  class RemoteAssetFile
+    class << self
+      def cache
+        @cache ||= {}
+      end
+
+      def from_src(src)
+        key = uri(src).to_s
+        cache[key] = RemoteAssetFile.new(src) unless cache.key?(key)
+        cache[key]
+      end
+
+      def uri(src)
+        URI.parse(src.sub(%r{^//}, "https://"))
+      end
+    end
+
+    def initialize(src)
+      @uri = RemoteAssetFile.uri(src)
+      @content = nil
+    end
+
+    def content
+      return @content unless @content.nil?
+
+      res = Net::HTTP.start(@uri.hostname, @uri.port, use_ssl: @uri.scheme == 'https') do |http|
+        req = Net::HTTP::Get.new(@uri)
+        req['Accept-Encoding'] = 'gzip, deflate, br'
+        http.request(req)
+      end
+
+      @content = res.body
+    end
+
+    def gzipped_size
+      @gzipped_size ||= @content.bytesize
+    end
+  end
+end

lib/theme_check/theme.rb

@@ -11,6 +11,12 @@ def initialize(storage)
       @storage = storage
     end
 
+    def assets
+      @assets ||= @storage.files
+        .select { |path| path.starts_with?("assets/") }
+        .map { |path| AssetFile.new(path, @storage) }
+    end
+
     def liquid
       @liquid ||= @storage.files
         .select { |path| LIQUID_REGEX.match?(path) }
@@ -43,7 +49,7 @@ def default_locale
     end
 
     def all
-      @all ||= json + liquid
+      @all ||= json + liquid + assets
     end
 
     def [](name)

test/asset_file_test.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+require "test_helper"
+
+class AssetFileTest < Minitest::Test
+  def setup
+    @asset = make_asset_file("assets/theme.js", "")
+  end
+
+  def test_name
+    assert_equal("assets/theme.js", @asset.name)
+  end
+
+  def test_relative_path
+    assert_equal("assets/theme.js", @asset.relative_path.to_s)
+  end
+
+  def test_content
+    assert_equal("", @asset.content)
+  end
+
+  def test_gzipped_size
+    assert_equal(20, @asset.gzipped_size)
+  end
+
+  private
+
+  def make_asset_file(name, content)
+    storage = make_storage(name => content)
+    ThemeCheck::AssetFile.new(name, storage)
+  end
+end