diff --git a/CHANGELOG.md b/CHANGELOG.md index c4c65f6..87a4d45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -243,6 +243,39 @@ ### Fixed +- **One package a CDN can't resolve no longer blocks — or silently moves — the + rest of the batch.** ([#32](https://github.com/zoolutions/importmap-plus/issues/32)) + A CDN answers a batch of packages as a whole, so `bin/importmap update` with + three outdated packages reported `Couldn't find any packages in + ["cheap-ruler", "mapbox-gl", "mermaid"] on jspm`, updated nothing and exited + 0, because jspm's generator can't build `mermaid`. Commenting out the one + package let the other two through. A refused batch of more than one package + is now asked for one package at a time, along the path each would have taken + alone: + + ``` + $ bin/importmap pin md5@2.2.0 mermaid@10.6.0 + jspm couldn't resolve "md5@2.2.0", "mermaid@10.6.0" (No './dist/cytoscape.umd.js' exports subpath defined in cytoscape@3.34.3); asking for each on its own + Pinning "md5" to vendor/javascript/md5.js via download from https://ga.jspm.io/npm:md5@2.2.0/md5.js + jspm couldn't resolve "mermaid@10.6.0" (No './dist/cytoscape.umd.js' exports subpath defined in cytoscape@3.34.3); trying esm.run + Pinning "mermaid" to vendor/javascript/mermaid.js via download from https://cdn.jsdelivr.net/npm/mermaid@10.6.0/+esm + ``` + ```ruby + pin "md5" # @2.2.0 + pin "mermaid" # @10.6.0 (esm.run) + ``` + + Only the package the CDN refused travels the rest of the chain, so a healthy + package is never re-pinned from another CDN — and never has its provenance + comment rewritten — because a sibling failed. An explicit `--from`, or a + provider a pin records, still asks that one CDN and only that one, now once + per package. The batch stays the fast path: it is split only when it is + refused and holds more than one package. + + `pin`, `update` and `pristine` now **exit 1** when a package was left + unresolved, so `bin/importmap update && git commit` can no longer commit an + import map that quietly missed a package. The packages that did resolve are + still written. - **A CDN that fails mid-crawl leaves the pin alone.** Vendoring a graph makes one request per file — 250 of them for `date-fns` — so a 503 that outlives the retries is far likelier than it was for a single download. `pin` and diff --git a/docs/app/views/docs/pages/cli.rb b/docs/app/views/docs/pages/cli.rb index 5b3efd3..10a4303 100644 --- a/docs/app/views/docs/pages/cli.rb +++ b/docs/app/views/docs/pages/cli.rb @@ -111,6 +111,7 @@ def exit_codes [ [ :code, "outdated" ], "an unlocked package is outdated" ], [ [ :code, "audit" ], "a vulnerability is known for a pinned version" ], [ [ :code, "pin / update" ], "a package was skipped — its directory is in the way, or its CDN failed partway through a crawl — while the rest were pinned; the skipped pin is left exactly as it was" ], + [ [ :code, "pin / update / pristine" ], "no CDN could resolve a package, its reason printed; the packages that did resolve are still written" ], [ [ :code, "update" ], "a named package has no pin, or names are combined with --all; nothing is updated in either case" ], [ [ :code, "pristine" ], "a package couldn't be restored the way its pin describes, or a dependency of an esm.run bundle was skipped; the rest are restored" ], [ [ :code, "lock / unlock" ], "a named package has no pin, has no version to lock at, or was given with a version" ], diff --git a/docs/app/views/docs/pages/pinning.rb b/docs/app/views/docs/pages/pinning.rb index 8edd303..9524bba 100644 --- a/docs/app/views/docs/pages/pinning.rb +++ b/docs/app/views/docs/pages/pinning.rb @@ -98,8 +98,29 @@ def choosing_a_cdn pin comment, so `update` and `pristine` go straight back to it and never retry the jspm that couldn't build it. Nothing new is stored anywhere else. - If no CDN in the chain has the package, each one's reason is printed and the - command says so: + A CDN answers a batch of packages as a whole, so one spec its generator + can't build would be a "no" for every package beside it. When that happens + each package is asked for on its own, along the path it would have taken + alone — so only the package the CDN actually refused moves to the next one: + MD + DocsUI::Code(<<~SHELL, lexer: :console) + $ ./bin/importmap pin md5@2.2.0 mermaid@10.6.0 + jspm couldn't resolve "md5@2.2.0", "mermaid@10.6.0" (No './dist/cytoscape.umd.js' exports subpath defined in cytoscape@3.34.3); asking for each on its own + Pinning "md5" to vendor/javascript/md5.js via download from https://ga.jspm.io/npm:md5@2.2.0/md5.js + jspm couldn't resolve "mermaid@10.6.0" (No './dist/cytoscape.umd.js' exports subpath defined in cytoscape@3.34.3); trying esm.run + Pinning "mermaid" to vendor/javascript/mermaid.js via download from https://cdn.jsdelivr.net/npm/mermaid@10.6.0/+esm + SHELL + DocsUI::Code(<<~RUBY, filename: "config/importmap.rb") + pin "md5" # @2.2.0 + pin "mermaid" # @10.6.0 (esm.run) + RUBY + md <<~'MD' + `md5` keeps the CDN it would have been pinned from alone — a sibling failing + is no reason to change a package's provenance. + + If no CDN in the chain has the package, each one's reason is printed, the + command says so and it exits 1. The packages that did resolve are still + pinned: MD DocsUI::Code(<<~SHELL, lexer: :console) $ ./bin/importmap pin no-such-package @@ -123,7 +144,9 @@ def choosing_a_cdn md <<~'MD' `--from` is a choice you made, so it is asked once and never falls back: the CDN you named either has the package or reports why it hasn't. The same goes - for a package whose pin already records a CDN. + for a package whose pin already records a CDN. A batch it refuses is still + split — each package is asked of that same CDN on its own, so the ones it + has are pinned and only the ones it hasn't are reported. The CDN is recorded in the pin comment when it isn't jspm, and later commands go back to it — an unpkg download stays on unpkg through `update` and diff --git a/docs/app/views/docs/pages/updating.rb b/docs/app/views/docs/pages/updating.rb index cfe7064..2e1b7da 100644 --- a/docs/app/views/docs/pages/updating.rb +++ b/docs/app/views/docs/pages/updating.rb @@ -46,6 +46,12 @@ def update against the CDN. Every other package still updates, and the command exits 1 so a script knows it didn't do all it was asked. + A package its CDN can't resolve is reported with the CDN's own reason and + left where it is. It doesn't hold back the rest: the CDN is asked for each + package on its own once it refuses the batch, so every package it can build + still updates, from the CDN its pin names. The command exits 1 to say it + didn't do all it was asked. + A package is re-resolved together with the dependencies its CDN lists for it, so those move as well, keeping their own pin options. Each package comes back from the CDN its pin comment names ([Provenance](/docs/provenance)); a remote @@ -173,7 +179,8 @@ def pristine and skipped — `Couldn't restore "pdfjs-dist": it can't be vendored as a single file (workers)` — and the rest of the packages are still restored; the command exits non-zero to say it didn't do all of it, as it does when a - dependency of an esm.run bundle, pinned on the way, had to be skipped. + dependency of an esm.run bundle, pinned on the way, had to be skipped, or + when the CDN a pin names couldn't resolve the package at all. MD end end diff --git a/docs/app/views/docs/pages/upgrading.rb b/docs/app/views/docs/pages/upgrading.rb index 78c31d3..2524b8e 100644 --- a/docs/app/views/docs/pages/upgrading.rb +++ b/docs/app/views/docs/pages/upgrading.rb @@ -51,6 +51,7 @@ def what_changes [ "A failed CDN request is a raw backtrace.", "Requests are retried three times with a growing pause; the failure then names the URL." ], [ "A download that imports sibling files, spawns a worker or fetches a .wasm is vendored anyway and 404s in the browser.", "It is pinned to its CDN URL instead, the pin says why, and pin --vendor overrides. See below." ], [ "pin asks jspm and nothing else; a package its generator can't build reports \"Couldn't find any packages\".", "jspm, then esm.run, then jsDelivr, until one answers. The CDN that did is recorded on the pin. --from turns the fallback off." ], + [ "One package a CDN refuses fails the whole command, which still exits 0.", "The refused batch is asked for one package at a time, so the rest are pinned from the CDN they would have come from alone. pin, update and pristine exit 1 when a package was left unresolved." ], [ "pin foo takes whatever version jspm has indexed.", "The npm registry decides the version, then every CDN is asked for that one." ], [ "A CDN that hands back a UMD bundle is vendored, and the import fails to link in the browser.", "It is pinned to its CDN URL with (remote: not an ES module). pin --vendor overrides." ] ] diff --git a/lib/importmap/batch_resolver.rb b/lib/importmap/batch_resolver.rb new file mode 100644 index 0000000..4bc7362 --- /dev/null +++ b/lib/importmap/batch_resolver.rb @@ -0,0 +1,133 @@ +require "importmap/provider_chain" + +# Resolves a group of specs against a CDN, batch first and then one at a time. +# +# The batch is the fast path and stays: one round trip, and jspm resolves the +# specs' shared dependencies as a single graph, which is what upstream does. +# But a CDN answers a batch as a whole — one spec its generator can't build is +# "no" for all of them — so a refused batch of more than one spec says nothing +# about the specs in it. Each is then asked on its own, along the path it would +# have taken alone: through the chain when the group named no CDN, of the same +# provider once when it did. A package that was always going to resolve is +# pinned from the CDN it was always going to be pinned from, whatever its +# siblings did. +# +# Specs nobody could resolve are collected rather than raised: each pin is +# independent of the others, so the rest still land and the exit code says the +# command didn't do all it was asked. +class Importmap::BatchResolver + # The specs no CDN answered for, in the order they were asked. + attr_reader :unresolved + + # +on_miss+ is the command's own "couldn't find" reporter, so the sentence a + # single spec gets is written in exactly one place. + def initialize(packager, on_miss:, chain: Importmap::ProviderChain.new) + @packager = packager + @on_miss = on_miss + @chain = chain + @unresolved = [] + end + + # Yields [ package, url ] for everything that resolved, as it resolves, so a + # spec that fails later can't discard the work of one that succeeded earlier. + def each_import(specs, env:, from:, fallback:, &block) + @named = specs.map { |spec| @packager.package_key_for(spec) } + @seen = {} + + if specs.one? + resolve_one(specs.first, env: env, from: from, fallback: fallback, &block) + elsif (response = resolve_batch(specs, env: env, from: from, fallback: fallback)) + emit(specs, response, &block) + else + specs.each { |spec| resolve_one(spec, env: env, from: from, fallback: fallback, &block) } + end + end + + private + # The batch a group with no CDN of its own sends goes to jspm alone. Handing + # the whole list to the next CDN is what moved a healthy package's provenance + # when a sibling failed; the specs travel the rest of the chain separately. + def resolve_batch(specs, env:, from:, fallback:) + provider = fallback ? Importmap::ProviderChain::DEFAULT : from + error = nil + + response = + begin + @packager.import(*specs, env: env, from: provider) + rescue Importmap::Packager::Error => raised + error = raised + nil + end + + return response if response + + report_split(specs, provider, error&.message || @packager.last_import_error, fallback) + nil + end + + def report_split(specs, provider, reason, fallback) + detail = Importmap::ProviderChain.tidy_reason(reason) + miss = if fallback + %(#{provider} couldn't resolve #{quoted(specs)}) + else + %(Couldn't find any packages in #{specs.inspect} on #{provider}) + end + + puts miss + (detail ? %( (#{detail})) : "") + "; asking for each on its own" + end + + def resolve_one(spec, env:, from:, fallback:, &block) + source = fallback ? Importmap::ProviderChain.to_sentence : from + + if (response = request_one(spec, env: env, from: from, fallback: fallback)) + emit([ spec ], response, &block) + else + # A chain that came up empty has already said why each CDN couldn't. + @on_miss.call([ spec ], source, reason: fallback ? nil : @packager.last_import_error) + @unresolved << spec + end + rescue Importmap::Packager::Error => error + puts %(Couldn't resolve "#{spec}" from #{source}: #{error.message}) + @unresolved << spec + end + + def request_one(spec, env:, from:, fallback:) + if fallback + @chain.resolve(@packager, [ spec ], env: env) { |_provider, response| response } + else + @packager.import(spec, env: env, from: from) + end + end + + # Split responses overlap: each carries the dependencies of its own spec. + # A spec the user named answers for itself, so a sibling's response naming + # it is a dependency edge and not the answer asked for. Anything else is + # taken from the first response that carried it, because rewriting it to a + # second URL would move a package the user never mentioned. + def emit(specs, response, &block) + asked_for = specs.map { |spec| @packager.package_key_for(spec) } + + response[:imports].each do |package, url| + next if @named.include?(package) && !asked_for.include?(package) + + if (kept = @seen[package]) + report_kept(package, kept, url, specs.first) unless kept == url + else + @seen[package] = url + block.call(package, url) + end + end + end + + def report_kept(package, kept, url, spec) + puts %(Keeping "#{package}" at #{version(kept)} ("#{spec}" resolved it to #{version(url)})) + end + + def version(url) + @packager.extract_package_version_from(url) || url + end + + def quoted(specs) + specs.map { |spec| %("#{spec}") }.join(", ") + end +end diff --git a/lib/importmap/commands.rb b/lib/importmap/commands.rb index 2edc918..68a6b0d 100644 --- a/lib/importmap/commands.rb +++ b/lib/importmap/commands.rb @@ -4,6 +4,7 @@ require "importmap/vendored_graph" require "importmap/npm" require "importmap/provider_chain" +require "importmap/batch_resolver" require "importmap/integrity" require "importmap/doctor" @@ -39,7 +40,7 @@ def pin(*packages) vendor: requested.include?(package) && options[:vendor]) end - exit 1 if skipped.any? + exit 1 if skipped.any? || resolver.unresolved.any? end desc "lock [*PACKAGES]", "Lock packages at their pinned version" @@ -84,7 +85,7 @@ def pristine # skipped: an esm.run bundle's dependency is pinned on the way, and one the # CDN failed on is skipped the same way pin skips it. - exit 1 if unrestored.any? || skipped.any? + exit 1 if unrestored.any? || skipped.any? || resolver.unresolved.any? end desc "json", "Show the full importmap in json" @@ -166,7 +167,7 @@ def update(*packages) end end - exit 1 if unchecked_packages.any? || skipped.any? + exit 1 if unchecked_packages.any? || skipped.any? || resolver.unresolved.any? end desc "packages", "Print out packages with version numbers" @@ -526,28 +527,15 @@ def vendored_minified?(package) # another CDN would be a rewrite, not a restore. def for_each_import_grouped_by_provider(packages, env:, from: nil, fallback: false, &block) packages.group_by { |spec| from || vendored_provider_for(spec) || Importmap::ProviderChain::DEFAULT }.each do |provider, group| - if fallback && from.nil? && Importmap::ProviderChain.default?(provider) - for_each_import_with_fallback(group, env: env, &block) - else - for_each_import(group, env: env, from: provider, &block) - end + resolver.each_import(group, env: env, from: provider, + fallback: fallback && from.nil? && Importmap::ProviderChain.default?(provider), &block) end end - # A package nobody has pinned yet names no CDN, so it is asked of each in - # turn: jspm's generator giving up on a package says something about the - # generator, not about the package, and the next CDN often has it. A - # provider a pin already records, or one --from names, is a choice somebody - # made, and a choice is asked once and reported on. - def for_each_import_with_fallback(packages, env:, &block) - response = Importmap::ProviderChain.new.resolve(packager, packages, env: env) { |_provider, answer| answer } - - if response - response[:imports].each(&block) - else - # Every CDN has already said why it couldn't, so the summary repeats none of it. - handle_package_not_found(packages, Importmap::ProviderChain.to_sentence, reason: nil) - end + # Shared by every group of a single run, so the exit code can ask it once + # whether anything was left unresolved. + def resolver + @resolver ||= Importmap::BatchResolver.new(packager, on_miss: method(:handle_package_not_found)) end # A spec with no version means the latest, and the registry is what knows diff --git a/test/batch_resolver_test.rb b/test/batch_resolver_test.rb new file mode 100644 index 0000000..25a2c9e --- /dev/null +++ b/test/batch_resolver_test.rb @@ -0,0 +1,181 @@ +require "test_helper" +require "importmap/batch_resolver" + +class Importmap::BatchResolverTest < ActiveSupport::TestCase + # Stands in for Importmap::Packager. Answers are keyed by the exact request + # the resolver makes — [ provider, specs ] — because the whole point of the + # class under test is which requests it makes, not only what comes back. + # The two pure helpers come from the real Packager so the fake can't drift. + class FakePackager + attr_reader :asked, :last_import_error + + delegate :package_key_for, :extract_package_version_from, to: :@packager + + def initialize(answers) + @answers = answers + @asked = [] + @packager = Importmap::Packager.new + end + + def import(*specs, env:, from:) + @asked << [ from, specs ] + @last_import_error = nil + + case (answer = @answers[[ from, specs ]]) + when Exception then raise answer + when String then (@last_import_error = answer) && nil + else answer + end + end + end + + MD5 = "https://ga.jspm.io/npm:md5@2.2.0/md5.js".freeze + MERMAID = "https://cdn.jsdelivr.net/npm/mermaid@10.6.0/+esm".freeze + BOTH = [ "md5@2.2.0", "mermaid@10.6.0" ].freeze + + test "a batch every spec resolves in makes one request and yields every import" do + packager = FakePackager.new([ "jspm", BOTH ] => { imports: { "md5" => MD5, "mermaid" => "https://ga.jspm.io/npm:mermaid@10.6.0/dist/mermaid.js" } }) + + assert_equal [ [ "md5", MD5 ], [ "mermaid", "https://ga.jspm.io/npm:mermaid@10.6.0/dist/mermaid.js" ] ], + collect(packager, BOTH, fallback: true) + + assert_equal [ [ "jspm", BOTH ] ], packager.asked + assert_empty resolver_for(packager).unresolved + end + + test "a refused fallback batch asks for each spec on its own and only the refused one changes CDN" do + packager = FakePackager.new( + [ "jspm", BOTH ] => "Error: Unable to resolve https://ga.jspm.io/npm:mermaid@10.6.0/", + [ "jspm", [ "md5@2.2.0" ] ] => { imports: { "md5" => MD5 } }, + [ "jspm", [ "mermaid@10.6.0" ] ] => "Error: Unable to resolve https://ga.jspm.io/npm:mermaid@10.6.0/", + [ "esm.run", [ "mermaid@10.6.0" ] ] => { imports: { "mermaid" => MERMAID } }) + + imports = nil + out, _err = capture_io { imports = collect(packager, BOTH, fallback: true) } + + assert_equal [ [ "md5", MD5 ], [ "mermaid", MERMAID ] ], imports + assert_includes out, %(jspm couldn't resolve "md5@2.2.0", "mermaid@10.6.0" (Unable to resolve https://ga.jspm.io/npm:mermaid@10.6.0/); asking for each on its own\n) + + assert_equal [ [ "jspm", BOTH ], [ "jspm", [ "md5@2.2.0" ] ], + [ "jspm", [ "mermaid@10.6.0" ] ], [ "esm.run", [ "mermaid@10.6.0" ] ] ], packager.asked + assert_empty @resolver.unresolved + end + + test "a refused named-provider batch asks each spec of that provider and no other" do + packager = FakePackager.new( + [ "jspm", BOTH ] => "Error: Unable to resolve", + [ "jspm", [ "md5@2.2.0" ] ] => { imports: { "md5" => MD5 } }, + [ "jspm", [ "mermaid@10.6.0" ] ] => "Error: Unable to resolve") + + imports = nil + out, _err = capture_io { imports = collect(packager, BOTH, from: "jspm", fallback: false) } + + assert_equal [ [ "md5", MD5 ] ], imports + assert_equal [ "mermaid@10.6.0" ], @resolver.unresolved + assert_includes out, %(Couldn't find any packages in ["md5@2.2.0", "mermaid@10.6.0"] on jspm (Unable to resolve); asking for each on its own\n) + assert_equal [ [ [ "mermaid@10.6.0" ], "jspm", "Error: Unable to resolve" ] ], @misses + + assert_equal %w[ jspm jspm jspm ], packager.asked.map(&:first) + end + + test "a single-spec group makes the same requests it would have made before" do + packager = FakePackager.new([ "jspm", [ "mermaid@10.6.0" ] ] => "Error: nope", + [ "esm.run", [ "mermaid@10.6.0" ] ] => { imports: { "mermaid" => MERMAID } }) + + out, _err = capture_io { assert_equal [ [ "mermaid", MERMAID ] ], collect(packager, [ "mermaid@10.6.0" ], fallback: true) } + + assert_equal [ [ "jspm", [ "mermaid@10.6.0" ] ], [ "esm.run", [ "mermaid@10.6.0" ] ] ], packager.asked + assert_not_includes out, "asking for each on its own" + end + + test "a spec no CDN has is reported once, recorded, and doesn't stop the rest" do + packager = FakePackager.new( + [ "jspm", BOTH ] => "Error: Unable to resolve", + [ "jspm", [ "md5@2.2.0" ] ] => { imports: { "md5" => MD5 } }, + [ "jspm", [ "mermaid@10.6.0" ] ] => "Error: no build", + [ "esm.run", [ "mermaid@10.6.0" ] ] => "Error: Not Found", + [ "jsdelivr", [ "mermaid@10.6.0" ] ] => "Error: Not Found") + + imports = nil + out, _err = capture_io { imports = collect(packager, BOTH, fallback: true) } + + assert_equal [ [ "md5", MD5 ] ], imports + assert_equal [ "mermaid@10.6.0" ], @resolver.unresolved + assert_includes out, %(jspm couldn't resolve "mermaid@10.6.0" (no build); trying esm.run\n) + assert_includes out, %(esm.run couldn't resolve "mermaid@10.6.0" (Not Found); trying jsdelivr\n) + assert_includes out, %(jsdelivr couldn't resolve "mermaid@10.6.0" (Not Found)\n) + assert_equal [ [ [ "mermaid@10.6.0" ], "jspm, esm.run or jsdelivr", nil ] ], @misses + end + + # Each pin is independent of the others, the stance update already takes on a + # package the registry couldn't answer for. + test "a spec whose CDN can't be reached is reported and the specs after it still resolve" do + packager = FakePackager.new( + [ "jspm", BOTH ] => "Error: Unable to resolve", + [ "jspm", [ "mermaid@10.6.0" ] ] => Importmap::Packager::HTTPError.new("Unexpected response code (502)"), + [ "jspm", [ "md5@2.2.0" ] ] => { imports: { "md5" => MD5 } }) + + imports = nil + out, _err = capture_io { imports = collect(packager, [ "mermaid@10.6.0", "md5@2.2.0" ], from: "jspm", fallback: false) } + + assert_equal [ [ "md5", MD5 ] ], imports + assert_equal [ "mermaid@10.6.0" ], @resolver.unresolved + assert_includes out, %(Couldn't resolve "mermaid@10.6.0" from jspm: Unexpected response code (502)\n) + end + + test "a dependency two responses agree on is yielded once" do + packager = FakePackager.new( + [ "jspm", BOTH ] => "Error: Unable to resolve", + [ "jspm", [ "md5@2.2.0" ] ] => { imports: { "md5" => MD5, "crypt" => "https://ga.jspm.io/npm:crypt@0.0.2/index.js" } }, + [ "jspm", [ "mermaid@10.6.0" ] ] => { imports: { "mermaid" => MERMAID, "crypt" => "https://ga.jspm.io/npm:crypt@0.0.2/index.js" } }) + + imports = nil + out, _err = capture_io { imports = collect(packager, BOTH, from: "jspm", fallback: false) } + + assert_equal [ [ "md5", MD5 ], [ "crypt", "https://ga.jspm.io/npm:crypt@0.0.2/index.js" ], [ "mermaid", MERMAID ] ], imports + assert_not_includes out, "Keeping" + end + + test "a dependency two responses disagree on keeps the first and says so" do + packager = FakePackager.new( + [ "jspm", BOTH ] => "Error: Unable to resolve", + [ "jspm", [ "md5@2.2.0" ] ] => { imports: { "md5" => MD5, "crypt" => "https://ga.jspm.io/npm:crypt@0.0.2/index.js" } }, + [ "jspm", [ "mermaid@10.6.0" ] ] => { imports: { "mermaid" => MERMAID, "crypt" => "https://ga.jspm.io/npm:crypt@0.1.0/index.js" } }) + + imports = nil + out, _err = capture_io { imports = collect(packager, BOTH, from: "jspm", fallback: false) } + + assert_equal [ [ "md5", MD5 ], [ "crypt", "https://ga.jspm.io/npm:crypt@0.0.2/index.js" ], [ "mermaid", MERMAID ] ], imports + assert_includes out, %(Keeping "crypt" at @0.0.2 ("mermaid@10.6.0" resolved it to @0.1.0)\n) + end + + # md5 is one of the specs the user named, so its pin comes from its own + # response — a sibling's response mentioning it is a dependency edge, not an + # answer to what the user asked for. + test "a spec the user named is taken from its own response, never a sibling's" do + packager = FakePackager.new( + [ "jspm", BOTH ] => "Error: Unable to resolve", + [ "jspm", [ "md5@2.2.0" ] ] => "Error: no build", + [ "jspm", [ "mermaid@10.6.0" ] ] => { imports: { "mermaid" => MERMAID, "md5" => "https://ga.jspm.io/npm:md5@2.3.0/md5.js" } }) + + imports = nil + capture_io { imports = collect(packager, BOTH, from: "jspm", fallback: false) } + + assert_equal [ [ "mermaid", MERMAID ] ], imports + assert_equal [ "md5@2.2.0" ], @resolver.unresolved + end + + private + def collect(packager, specs, from: nil, fallback: false) + [].tap do |imports| + resolver_for(packager).each_import(specs, env: "production", from: from, fallback: fallback) do |package, url| + imports << [ package, url ] + end + end + end + + def resolver_for(packager) + @misses ||= [] + @resolver ||= Importmap::BatchResolver.new(packager, on_miss: ->(packages, source, reason:) { @misses << [ packages, source, reason ] }) + end +end diff --git a/test/commands_test.rb b/test/commands_test.rb index edd37b9..f0c2cdd 100644 --- a/test/commands_test.rb +++ b/test/commands_test.rb @@ -1060,7 +1060,7 @@ class CommandsTest < ActiveSupport::TestCase pin "photoswipe/lightbox", to: "https://cdn.jsdelivr.net/npm/photoswipe@5.3.0/dist/photoswipe-lightbox.esm.js" PINS - out, _err = run_importmap_command("update") + out, _err = run_importmap_command_expecting_failure("update") # The remote pin says jsdelivr, so the subpath is asked of jsdelivr. Taking # the package's skypack instead grouped the two into one request, and one @@ -1126,7 +1126,7 @@ class CommandsTest < ActiveSupport::TestCase test "pin asks only the CDN it was told to and says why that one couldn't" do importmap_config("") - out, _err = run_importmap_command("pin", "mermaid@10.6.0", "--from", "jspm") + out, _err = run_importmap_command_expecting_failure("pin", "mermaid@10.6.0", "--from", "jspm") assert_not_includes out, "trying esm.run" assert_includes out, %(Couldn't find any packages in ["mermaid@10.6.0"] on jspm) @@ -1135,6 +1135,40 @@ class CommandsTest < ActiveSupport::TestCase assert_not_includes File.read("#{@tmpdir}/dummy/config/importmap.rb"), "mermaid" end + # jspm answers a batch as a whole, so the spec its generator can't build used + # to be "no" for every spec beside it. md5 resolves on jspm alone and must be + # pinned from jspm: a sibling failing is no reason to move its provenance. + test "pin splits a batch jspm refused and only the refused package changes CDN" do + importmap_config("") + + out, _err = run_importmap_command("pin", "md5@2.2.0", "mermaid@10.6.0") + + assert_match(/jspm couldn't resolve "md5@2\.2\.0", "mermaid@10\.6\.0" \(.+\); asking for each on its own/, out) + + content = File.read("#{@tmpdir}/dummy/config/importmap.rb") + assert_includes content, %(pin "md5" # @2.2.0\n) + assert_includes content, %(pin "mermaid" # @10.6.0 (esm.run)\n) + + assert File.exist?("#{@tmpdir}/dummy/vendor/javascript/md5.js") + assert File.exist?("#{@tmpdir}/dummy/vendor/javascript/mermaid.js") + end + + # --from is still asked once per spec and never falls back: the split is about + # which specs a refusal is evidence against, not about the choice of CDN. + test "pin splits a batch a named CDN refused without trying another one" do + importmap_config("") + + out, _err = run_importmap_command_expecting_failure("pin", "md5@2.2.0", "mermaid@10.6.0", "--from", "jspm") + + assert_match(/Couldn't find any packages in \["md5@2\.2\.0", "mermaid@10\.6\.0"\] on jspm( \(.+\))?; asking for each on its own/, out) + assert_includes out, %(Couldn't find any packages in ["mermaid@10.6.0"] on jspm) + assert_not_includes out, "trying esm.run" + + content = File.read("#{@tmpdir}/dummy/config/importmap.rb") + assert_includes content, %(pin "md5" # @2.2.0\n) + assert_not_includes content, "mermaid" + end + test "pin resolves a package with no version from the npm registry" do importmap_config("") @@ -1199,7 +1233,7 @@ class CommandsTest < ActiveSupport::TestCase test "pristine asks the CDN the pin names and doesn't fall back" do importmap_config(%(pin "mermaid" # @10.6.0)) - out, _err = run_importmap_command("pristine") + out, _err = run_importmap_command_expecting_failure("pristine") assert_not_includes out, "trying esm.run" assert_includes out, %(Couldn't find any packages in ["mermaid@10.6.0"] on jspm)