Keep options when updating packages in importmap

The only option that is relevant to keep is the `preload` option.

Not all cases are kept though. If the option is set to a variable or
complex expression, it is not kept. Only simple values like `true`, `false`,
or a string/array of strings are preserved.

Author: rafaelfranca

lib/importmap/commands.rb

@@ -15,13 +15,7 @@ def self.exit_on_failure?
   option :preload, type: :string, repeatable: true, desc: "Can be used multiple times"
   def pin(*packages)
     for_each_import(packages, env: options[:env], from: options[:from]) do |package, url|
-      puts %(Pinning "#{package}" to #{packager.vendor_path}/#{package}.js via download from #{url})
-
-      packager.download(package, url)
-
-      pin = packager.vendored_pin_for(package, url, options[:preload])
-
-      update_importmap_with_pin(package, pin)
+      pin_package(package, url, options[:preload])
     end
   end
 
@@ -96,7 +90,14 @@ def outdated
   desc "update", "Update outdated package pins"
   def update
     if (outdated_packages = npm.outdated_packages).any?
-      pin(*outdated_packages.map(&:name))
+      package_names = outdated_packages.map(&:name)
+      packages_with_options = packager.extract_existing_pin_options(package_names)
+
+      for_each_import(package_names, env: "production", from: "jspm") do |package, url|
+        options = packages_with_options[package] || {}
+
+        pin_package(package, url, options[:preload])
+      end
     else
       puts "No outdated packages found"
     end
@@ -116,6 +117,16 @@ def npm
       @npm ||= Importmap::Npm.new
     end
 
+    def pin_package(package, url, preload)
+      puts %(Pinning "#{package}" to #{packager.vendor_path}/#{package}.js via download from #{url})
+
+      packager.download(package, url)
+
+      pin = packager.vendored_pin_for(package, url, preload)
+
+      update_importmap_with_pin(package, pin)
+    end
+
     def update_importmap_with_pin(package, pin)
       new_pin = "#{pin}\n"
 

lib/importmap/packager.rb

@@ -4,6 +4,7 @@
 
 class Importmap::Packager
   PIN_REGEX = /#{Importmap::Map::PIN_REGEX}(.*)/.freeze # :nodoc:
+  PRELOAD_OPTION_REGEXP = /preload:\s*(\[[^\]]+\]|true|false|["'][^"']*["'])/.freeze # :nodoc:
 
   Error        = Class.new(StandardError)
   HTTPError    = Class.new(Error)
@@ -67,14 +68,57 @@ def remove(package)
     remove_package_from_importmap(package)
   end
 
+  def extract_existing_pin_options(packages)
+    return {} unless @importmap_path.exist?
+
+    packages = Array(packages)
+
+    all_package_options = build_package_options_lookup(importmap.lines)
+
+    packages.to_h do |package|
+      [package, all_package_options[package] || {}]
+    end
+  end
+
   private
+    def build_package_options_lookup(lines)
+      lines.each_with_object({}) do |line, package_options|
+        match = line.strip.match(PIN_REGEX)
+
+        if match
+          package_name = match[1]
+          options_part = match[2]
+
+          preload_match = options_part.match(PRELOAD_OPTION_REGEXP)
+
+          if preload_match
+            preload = preload_from_string(preload_match[1])
+            package_options[package_name] = { preload: preload }
+          end
+        end
+      end
+    end
+
+    def preload_from_string(value)
+      case value
+      when "true"
+        true
+      when "false"
+        false
+      when /^\[.*\]$/
+        JSON.parse(value)
+      else
+        value.gsub(/["']/, "")
+      end
+    end
+
     def preload(preloads)
       case Array(preloads)
       in []
         ""
-      in ["true"]
+      in ["true"] | [true]
         %(, preload: true)
-      in ["false"]
+      in ["false"] | [false]
         %(, preload: false)
       in [string]
         %(, preload: "#{string}")

test/commands_test.rb

@@ -50,7 +50,105 @@ class CommandsTest < ActiveSupport::TestCase
     assert_equal original, File.read("#{@tmpdir}/dummy/vendor/javascript/md5.js")
   end
 
+  test "update command preserves preload false option" do
+    importmap_config('pin "md5", to: "https://cdn.skypack.dev/[email protected]", preload: false')
+
+    out, _err = run_importmap_command("update")
+
+    assert_includes out, "Pinning"
+
+    updated_content = File.read("#{@tmpdir}/dummy/config/importmap.rb")
+    assert_includes updated_content, "preload: false"
+    assert_includes updated_content, "# @2.3.0"
+  end
+
+  test "update command preserves preload true option" do
+    importmap_config('pin "md5", to: "https://cdn.skypack.dev/[email protected]", preload: true')
+
+    out, _err = run_importmap_command("update")
+
+    assert_includes out, "Pinning"
+
+    updated_content = File.read("#{@tmpdir}/dummy/config/importmap.rb")
+    assert_includes updated_content, "preload: true"
+  end
+
+  test "update command preserves custom preload string option" do
+    importmap_config('pin "md5", to: "https://cdn.skypack.dev/[email protected]", preload: "custom"')
+
+    out, _err = run_importmap_command("update")
+
+    assert_includes out, "Pinning"
+
+    updated_content = File.read("#{@tmpdir}/dummy/config/importmap.rb")
+    assert_includes updated_content, 'preload: "custom"'
+  end
+
+  test "update command removes existing integrity" do
+    importmap_config('pin "md5", to: "https://cdn.skypack.dev/[email protected]", integrity: "sha384-oldintegrity"')
+
+    out, _err = run_importmap_command("update")
+
+    assert_includes out, "Pinning"
+
+    updated_content = File.read("#{@tmpdir}/dummy/config/importmap.rb")
+    assert_not_includes updated_content, "integrity:"
+  end
+
+  test "update command only keeps preload option" do
+    importmap_config('pin "md5", to: "https://cdn.skypack.dev/[email protected]", preload: false, integrity: "sha384-oldintegrity"')
+
+    out, _err = run_importmap_command("update")
+
+    assert_includes out, "Pinning"
+
+    updated_content = File.read("#{@tmpdir}/dummy/config/importmap.rb")
+    assert_includes updated_content, "preload: false"
+    assert_not_includes updated_content, "to:"
+    assert_not_includes updated_content, "integrity:"
+  end
+
+  test "update command handles packages with different quote styles" do
+    importmap_config("pin 'md5', to: 'https://cdn.skypack.dev/[email protected]', preload: false")
+
+    out, _err = run_importmap_command("update")
+
+    assert_includes out, "Pinning"
+
+    updated_content = File.read("#{@tmpdir}/dummy/config/importmap.rb")
+    assert_includes updated_content, "preload: false"
+  end
+
+  test "update command preserves options with version comments" do
+    importmap_config('pin "md5", to: "https://cdn.skypack.dev/[email protected]", preload: false # @2.2.0')
+
+    out, _err = run_importmap_command("update")
+
+    assert_includes out, "Pinning"
+
+    updated_content = File.read("#{@tmpdir}/dummy/config/importmap.rb")
+    assert_includes updated_content, "preload: false"
+    assert_includes updated_content, "# @2.3.0"
+    assert_not_includes updated_content, "# @2.2.0"
+  end
+
+  test "update command handles whitespace variations in pin options" do
+    importmap_config('pin "md5",   to:  "https://cdn.skypack.dev/[email protected]",  preload:  false   ')
+
+    out, _err = run_importmap_command("update")
+
+    assert_includes out, "Pinning"
+
+    updated_content = File.read("#{@tmpdir}/dummy/config/importmap.rb")
+    assert_equal 4, updated_content.lines.size
+    assert_includes updated_content, "preload: false"
+  end
+
   private
+    def importmap_config(content)
+      File.write("#{@tmpdir}/dummy/config/importmap.rb", content)
+    end
+
     def run_importmap_command(command, *args)
       capture_subprocess_io { system("bin/importmap", command, *args, exception: true) }
     end

test/packager_test.rb

@@ -79,4 +79,137 @@ def code() "200" end
     assert_equal %(pin "react", preload: "foo" # @17.0.2), @packager.vendored_pin_for("react", "https://cdn/[email protected]", ["foo"])
     assert_equal %(pin "react", preload: ["foo", "bar"] # @17.0.2), @packager.vendored_pin_for("react", "https://cdn/[email protected]", ["foo", "bar"])
   end
+
+  test "extract_existing_pin_options with preload false" do
+    temp_importmap = create_temp_importmap('pin "package1", preload: false')
+    packager = Importmap::Packager.new(temp_importmap)
+
+    options = extract_options_for_package(packager, "package1")
+
+    assert_equal({ preload: false }, options)
+  end
+
+  test "extract_existing_pin_options with preload true" do
+    temp_importmap = create_temp_importmap('pin "package1", preload: true')
+    packager = Importmap::Packager.new(temp_importmap)
+
+    options = extract_options_for_package(packager, "package1")
+
+    assert_equal({ preload: true }, options)
+  end
+
+  test "extract_existing_pin_options with custom preload string" do
+    temp_importmap = create_temp_importmap('pin "package1", preload: "custom"')
+    packager = Importmap::Packager.new(temp_importmap)
+
+    options = extract_options_for_package(packager, "package1")
+
+    assert_equal({ preload: "custom" }, options)
+  end
+
+  test "extract_existing_pin_options with custom preload array" do
+    temp_importmap = create_temp_importmap('pin "package1", preload: ["custom1", "custom2"]')
+    packager = Importmap::Packager.new(temp_importmap)
+
+    options = extract_options_for_package(packager, "package1")
+
+    assert_equal({ preload: ["custom1", "custom2"] }, options)
+  end
+
+  test "extract_existing_pin_options with to option only" do
+    temp_importmap = create_temp_importmap('pin "package1", to: "custom_path.js"')
+    packager = Importmap::Packager.new(temp_importmap)
+
+    options = extract_options_for_package(packager, "package1")
+
+    assert_equal({}, options)
+  end
+
+  test "extract_existing_pin_options with integrity option only" do
+    temp_importmap = create_temp_importmap('pin "package1", integrity: "sha384-abcdef1234567890"')
+    packager = Importmap::Packager.new(temp_importmap)
+
+    options = extract_options_for_package(packager, "package1")
+
+    assert_equal({}, options)
+  end
+
+  test "extract_existing_pin_options with multiple options" do
+    temp_importmap = create_temp_importmap('pin "package1", to: "path.js", preload: false, integrity: "sha384-abcdef1234567890"')
+    packager = Importmap::Packager.new(temp_importmap)
+
+    options = extract_options_for_package(packager, "package1")
+
+    assert_equal({ preload: false }, options)
+  end
+
+  test "extract_existing_pin_options with version comment" do
+    temp_importmap = create_temp_importmap('pin "package1", preload: false # @2.0.0')
+    packager = Importmap::Packager.new(temp_importmap)
+
+    options = extract_options_for_package(packager, "package1")
+
+    assert_equal({ preload: false }, options)
+  end
+
+  test "extract_existing_pin_options with no options" do
+    temp_importmap = create_temp_importmap('pin "package1"')
+    packager = Importmap::Packager.new(temp_importmap)
+
+    options = extract_options_for_package(packager, "package1")
+
+    assert_equal({}, options)
+  end
+
+  test "extract_existing_pin_options with nonexistent package" do
+    temp_importmap = create_temp_importmap('pin "package1", preload: false')
+    packager = Importmap::Packager.new(temp_importmap)
+
+    options = extract_options_for_package(packager, "nonexistent")
+
+    assert_equal({}, options)
+  end
+
+  test "extract_existing_pin_options with nonexistent file" do
+    packager = Importmap::Packager.new("/nonexistent/path")
+
+    options = extract_options_for_package(packager, "package1")
+
+    assert_nil options
+  end
+
+  test "extract_existing_pin_options handles multiple packages in one call" do
+    temp_importmap = create_temp_importmap(<<~PINS)
+      pin "package1", preload: false
+      pin "package2", preload: true
+      pin "package3", preload: "custom"
+      pin "package4" # no options
+    PINS
+
+    packager = Importmap::Packager.new(temp_importmap)
+
+    result = packager.extract_existing_pin_options(["package1", "package2", "package3", "package4", "nonexistent"])
+
+    assert_equal({
+      "package1" => { preload: false },
+      "package2" => { preload: true },
+      "package3" => { preload: "custom" },
+      "package4" => {},
+      "nonexistent" => {}
+    }, result)
+  end
+
+  private
+
+  def create_temp_importmap(content)
+    temp_file = Tempfile.new(['importmap', '.rb'])
+    temp_file.write(content)
+    temp_file.close
+    temp_file.path
+  end
+
+  def extract_options_for_package(packager, package_name)
+    result = packager.extract_existing_pin_options(package_name)
+    result[package_name]
+  end
 end