Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion docs/app/views/docs/pages/multi_file_packages.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions docs/app/views/docs/pages/pinning.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 12 additions & 0 deletions lib/importmap/commands.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
25 changes: 25 additions & 0 deletions test/commands_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading