diff --git a/CHANGELOG.md b/CHANGELOG.md index e98f7bb..c8c6669 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -295,6 +295,17 @@ not JSON: the entry points are scanned out of the literal now, and an empty array is written back as one. +- **`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 67ff6e6..8edd303 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