From 79417fe701e42a9bfb4f7bd37c100c154bfb0579 Mon Sep 17 00:00:00 2001 From: mhenrixon Date: Sat, 19 Sep 2026 15:05:53 +0200 Subject: [PATCH] fix(cli): pin --vendor leaves a pin to a custom URL alone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --vendor overrides the check that refuses a download; it does not name a destination. The docs already say so in as many words — "--remote wins; it names a destination, where --vendor only overrides a check" — but the code disagreed, because the branch that skips a custom URL runs only while vendor is false: pin "md5", to: "https://cdn.example.com/md5.js", preload: false $ bin/importmap pin md5@2.3.0 --vendor Pinning "md5" to vendor/javascript/md5.js via download from https://ga.jspm.io/… The URL the app had chosen was gone, replaced by a file from whichever CDN the spec happened to resolve to, and `vendor ||= packager.vendored?(package)` kept it that way on every later rewrite. Nothing was printed about it. The skip now happens whether or not --vendor was passed, and the sentence names the flag so the override doesn't look ignored. --from is left as the one way to move such a pin on purpose: it already overrules a custom URL in repin_remote_package, so the guard stands down when a CDN was named, and --vendor --from still vendors from that CDN. ## Test coverage - test/commands_test.rb: --vendor against a custom-URL pin prints the skip and leaves both config/importmap.rb and vendor/javascript untouched; --vendor --from jspm still vendors, so the escape hatch is pinned down too Closes #29 --- CHANGELOG.md | 11 ++++++++ .../views/docs/pages/multi_file_packages.rb | 5 +++- docs/app/views/docs/pages/pinning.rb | 14 +++++++++++ lib/importmap/commands.rb | 12 +++++++++ test/commands_test.rb | 25 +++++++++++++++++++ 5 files changed, 66 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9de4e3..f40f2b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -284,6 +284,17 @@ for it, whether that pin is vendored, recording the CDN in its comment, or remote, carrying it in the URL with no comment at all. +- **`pin --vendor` leaves a pin to a custom URL alone.** `--vendor` is meant to + override the check that refuses a download, not the URL an app chose to pin, + but it reached the vendoring path before the branch that skips custom URLs + could run: `bin/importmap pin md5 --vendor` against + `pin "md5", to: "https://cdn.example.com/md5.js"` downloaded md5 from + whichever CDN the spec resolved to and replaced the line with + `pin "md5" # @2.3.0 (vendored)`, without a word about the URL it had just + dropped. It now reports the skip the way a plain `pin` does and names the + flag in it. Moving such a pin on purpose is still `--from`, which re-resolves + it from the CDN you name. + ## 1.1.0 ### Added diff --git a/docs/app/views/docs/pages/multi_file_packages.rb b/docs/app/views/docs/pages/multi_file_packages.rb index 317e837..9edfce1 100644 --- a/docs/app/views/docs/pages/multi_file_packages.rb +++ b/docs/app/views/docs/pages/multi_file_packages.rb @@ -250,7 +250,10 @@ def overriding_the_check so the override can't quietly vendor something you never asked about. Pass `--remote` and `--vendor` together and [`--remote`](/docs/pinning#pinning-to-a-remote-url) wins — it names a - destination, where `--vendor` only overrides a check. + destination, where `--vendor` only overrides a check. For the same reason + it leaves a pin on a host no CDN answers for alone, reporting it as + [skipped](/docs/pinning#custom-urls) rather than vendoring whatever the + package spec resolved to. Packages an app already vendored before this check existed are not rewritten on their own, and `bin/importmap pristine` downloads them again exactly as diff --git a/docs/app/views/docs/pages/pinning.rb b/docs/app/views/docs/pages/pinning.rb index c491fa9..96870c8 100644 --- a/docs/app/views/docs/pages/pinning.rb +++ b/docs/app/views/docs/pages/pinning.rb @@ -197,6 +197,20 @@ def custom_urls $ ./bin/importmap pin md5 Skipping "md5" pinned to custom URL https://cdn.example.com/md5.js SHELL + md <<~'MD' + `--vendor` doesn't override that. It overrides the check that refuses a + download — not the URL you chose to pin — so it reports the same skip and + names itself in it: + MD + DocsUI::Code(<<~SHELL, lexer: :console) + $ ./bin/importmap pin md5 --vendor + Skipping "md5" pinned to custom URL https://cdn.example.com/md5.js (--vendor doesn't move a pin to another source; pass --from to choose one) + SHELL + md <<~'MD' + Naming a CDN with `--from` is how you move such a pin on purpose: it + re-resolves the package there and rewrites the line, vendoring it if you + also passed `--vendor`. + MD end end diff --git a/lib/importmap/commands.rb b/lib/importmap/commands.rb index 58f3cf6..2edc918 100644 --- a/lib/importmap/commands.rb +++ b/lib/importmap/commands.rb @@ -229,6 +229,8 @@ def pin_package(package, url, preload: nil, remote: false, env: "production", mi if existing_url && !vendor && !regraph repin_remote_package(package, url, existing_url, preload, env: env, from: from, integrity: integrity, locked: locked) + elsif existing_url && from.nil? && packager.provider_for_url(existing_url).nil? + keep_custom_url(package, existing_url) elsif remote pin_remote_package(package, url, preload, integrity: integrity, locked: locked) else @@ -633,6 +635,16 @@ def pin_remote_package(package, url, preload, integrity: nil, locked: false, kep report_lock(package) if locked end + # A URL on a host no provider answers for is the app's own choice, and the + # branch above skips it — but only while `vendor` is false. --vendor is + # about overriding the check that refuses a download, not about moving a + # pin to another source, so it lands here instead of vendoring whatever the + # spec happened to resolve to. Naming a CDN with --from is how a pin is + # moved on purpose, and that still goes through repin_remote_package. + def keep_custom_url(package, existing_url) + puts %(Skipping "#{package}" pinned to custom URL #{existing_url} (--vendor doesn't move a pin to another source; pass --from to choose one)) + end + def repin_remote_package(package, url, existing_url, preload, env:, from: nil, integrity: nil, locked: false) # `url` was already resolved from the requested CDN, so an explicit # --from moves the pin instead of being overruled by its current one. diff --git a/test/commands_test.rb b/test/commands_test.rb index 32015ff..edd37b9 100644 --- a/test/commands_test.rb +++ b/test/commands_test.rb @@ -1307,6 +1307,31 @@ class CommandsTest < ActiveSupport::TestCase assert_includes out, "0 errors, 1 warning\n" end + test "pin command with --vendor leaves a pin to a custom URL untouched" do + importmap_config('pin "md5", to: "https://cdn.example.com/md5.js", preload: false') + + out, _err = run_importmap_command("pin", "md5@2.3.0", "--vendor") + + assert_includes out, 'Skipping "md5" pinned to custom URL https://cdn.example.com/md5.js' + assert_includes out, "--vendor doesn't move a pin to another source" + + content = File.read("#{@tmpdir}/dummy/config/importmap.rb") + assert_includes content, 'pin "md5", to: "https://cdn.example.com/md5.js", preload: false' + assert_not File.exist?("#{@tmpdir}/dummy/vendor/javascript/md5.js") + end + + test "pin command with --vendor and --from vendors a custom-URL pin from the CDN that was named" do + importmap_config('pin "md5", to: "https://cdn.example.com/md5.js", preload: false') + + out, _err = run_importmap_command("pin", "md5@2.2.0", "--vendor", "--from", "jspm") + + assert_includes out, 'Pinning "md5" to vendor/javascript/md5.js' + + content = File.read("#{@tmpdir}/dummy/config/importmap.rb") + assert_includes content, 'pin "md5", preload: false # @2.2.0' + assert File.exist?("#{@tmpdir}/dummy/vendor/javascript/md5.js") + end + private # A registry that can't answer for a package is the case under test, and # the live registry won't produce it on demand. Stub the one method that