0.1.4 review fixes: deliberate Enter, cached logo misses, honest edges

Four review findings against the unpublished 0.1.4, each with a test
that fails on the previous code:

- Enter commits only a deliberate choice: an arrowed-to row, the single
  remaining match, or text naming a make exactly. Focus opens the full
  list, and Enter there used to commit the first row alphabetically,
  silently replacing whatever the person had already picked. Text that
  matches the committed selection falls through to a plain form submit.
- A failed logo fetch is a cached miss for the pageview, not a retry:
  rows re-request logos on every render, so a misbehaving logo endpoint
  meant up to fifty requests per keystroke, forever.
- Blur restores the committed text synchronously; a submit button
  clicked in the same tick can no longer carry a value whose visible
  text disagrees with it.
- A server-preselected value missing from the fetched list announces
  itself: onChange fires with the cleared state and the dropped value is
  named in a warning, instead of the form quietly submitting empty.

Also: sideEffects back to true (the UMD global assignment IS the side
effect; false let bundlers tree-shake bare imports into runtime errors),
consumer onChange exceptions log whole via console.error instead of
masquerading as API failures in onError, the required-field validity
message is configurable (requiredMessage), the selected-make chip no
longer flickers on each pick, the README stops claiming the widget
calls /search, and CI runs once per change instead of twice per PR.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015jjKjr2ajZ4etdoMN6eC6k

Author: rameerez

.github/workflows/test.yml

@@ -2,6 +2,8 @@ name: Test
 
 on:
   push:
+    branches: [main]
+    tags: ["v*"]
   pull_request:
 
 permissions:

README.md

@@ -85,7 +85,7 @@ The vehicle identity data is [CC-BY 4.0](https://vehiclesdb.com/attribution) —
 
 ## The API underneath
 
-The widget speaks the documented Dropdown Data endpoints — [`GET /api/v1/makes`](https://vehiclesdb.com/api), `GET /api/v1/makes/{slug}/models`, `GET /api/v1/search`, and make logos — which cost 0 credits and accept publishable keys, so the dropdown never spends your quota. Public keys are deliberately refused everywhere else. Use a private `vdb_sk_…` key from your backend for the rest of the [VehiclesDB API](https://vehiclesdb.com/api): resolving free-text vehicle strings, full records, imagery, and license-plate validation.
+The widget speaks the documented Dropdown Data endpoints: [`GET /api/v1/makes`](https://vehiclesdb.com/api), `GET /api/v1/makes/{slug}/models`, and make logos. (`GET /api/v1/search` is part of the same free surface if you build your own typeahead.) They cost 0 credits and accept publishable keys, so the dropdown never spends your quota. Public keys are deliberately refused everywhere else. Use a private `vdb_sk_…` key from your backend for the rest of the [VehiclesDB API](https://vehiclesdb.com/api): resolving free-text vehicle strings, full records, imagery, and license-plate validation.
 
 ## Development
 

make-model-dropdown.js

@@ -30,6 +30,7 @@
     attribution: true,     // "Vehicle data by VehiclesDB" line (CC-BY credit)
     placeholderMake: "Make",
     placeholderModel: "Model",
+    requiredMessage: "Please select a vehicle make.", // validity message on a required make field
     onChange: null,        // ({ make, model }) => {}
     onError: null          // (error) => {}
   }
@@ -94,9 +95,11 @@
       if (response.status === 404) return null
       if (!response.ok) throw new Error(`VehiclesDB logo API answered ${response.status}`)
       return response.blob()
-    })
+      // ANY failure is a cached miss for the rest of the pageview: the letter
+      // chip renders instead. Every keystroke re-renders up to 50 rows, so a
+      // misbehaving logo endpoint retried per render would be a request storm.
+    }).catch(() => null)
     logoCache.set(cacheKey, promise)
-    promise.catch(() => logoCache.delete(cacheKey))
     return promise
   }
 
@@ -213,6 +216,7 @@
         const data = await fetchJSON(`${this.api}/makes${query(this.config)}`, this.config.key, "makes")
         if (this.destroyed) return
         this.makes = data.makes
+        const preselected = this.makeSelect.value
         fillSelect(this.makeSelect, this.config.placeholderMake, data.makes, this.config)
         if (this.config.includeOther) appendOther(this.makeSelect)
         if (this.combobox) this.combobox.setItems(data.makes)
@@ -234,6 +238,14 @@
         this.combobox?.sync()
 
         if (this.makeSelect.value) this.makeChanged()
+        else if (preselected && preselected !== "other") {
+          // A server-rendered selection that the fetched list does not carry
+          // (filtered out by kind/country/topMakes, or simply unknown) has
+          // just been dropped. Say so: the consumer's own state still holds
+          // the old value, and a silent empty submit is the worst outcome.
+          console.warn(`[VehiclesDropdown] preselected make ${JSON.stringify(preselected)} is not in the fetched list; selection cleared`)
+          this.notify()
+        }
       } catch (error) {
         if (!this.destroyed) this.fail(error)
       }
@@ -302,9 +314,6 @@
         const url = URL.createObjectURL(blob)
         this.objectUrls.add(url)
         return url
-      }).catch(() => {
-        this.logoUrls.delete(cacheKey)
-        return null
       })
       this.logoUrls.set(cacheKey, promise)
       return promise
@@ -386,9 +395,15 @@
         wrap.append(adornment)
       }
 
+      let adornedFor = null
       function showSelected(item) {
         input.value = item.name
         if (!adornment) return
+        // choose() lands here twice per pick (directly, then again via the
+        // change event through sync()); repainting the chip while the cached
+        // logo re-resolves flashes the letter fallback. Same item, no repaint.
+        if (adornedFor === item.name && adornment.childElementCount) return
+        adornedFor = item.name
         adornment.style.display = "inline-flex"
         input.style.paddingLeft = "2.75rem"
         adornment.replaceChildren(chip(item.name))
@@ -503,7 +518,7 @@
       function syncValidity() {
         if (!originalRequired) return
         const item = selectedItem()
-        input.setCustomValidity(item && input.value === item.name ? "" : "Please select a vehicle make.")
+        input.setCustomValidity(item && input.value === item.name ? "" : self.config.requiredMessage)
       }
 
       input.addEventListener("input", () => { render(input.value); syncValidity() })
@@ -513,7 +528,12 @@
         render("")
       })
       input.addEventListener("blur", () => {
-        blurTimer = setTimeout(() => { restoreSelected(); close() }, 100)
+        // Synchronously: list rows preventDefault on mousedown so they never
+        // blur the input, which means a blur is always a real departure — and
+        // a submit button clicked right after must find the visible text and
+        // the form value already agreeing.
+        restoreSelected()
+        blurTimer = setTimeout(close, 100)
       })
       input.addEventListener("keydown", (event) => {
         if (event.key === "Escape") {
@@ -532,9 +552,34 @@
           const next = active < 0 ? (delta > 0 ? 0 : shown.length - 1) : (active + delta + shown.length) % shown.length
           return setActive(next)
         }
-        if (event.key === "Enter" && open && shown.length) {
-          event.preventDefault()
-          choose(shown[active >= 0 ? active : 0])
+        if (event.key === "Enter") {
+          // Commit only a deliberate choice: an arrowed-to row, the single
+          // remaining match, or text that names one exactly. Focusing the
+          // field opens the full list, and Enter there must never quietly
+          // swap a committed selection for the first row alphabetically.
+          if (open && shown.length && active >= 0) {
+            event.preventDefault()
+            return choose(shown[active])
+          }
+          // Text agreeing with the committed selection is a plain form
+          // submit, not a re-pick (a re-pick would clear the chosen model).
+          const committed = selectedItem()
+          if (committed && input.value === committed.name) return close()
+          if (open && shown.length === 1) {
+            event.preventDefault()
+            return choose(shown[0])
+          }
+
+          const needle = input.value.trim().toLowerCase()
+          const exact = needle && items.find((item) => item.name.toLowerCase() === needle)
+          if (exact) {
+            event.preventDefault()
+            return choose(exact)
+          }
+          // No choice made: restore before the form submits, so the value it
+          // carries is the one the field visibly shows.
+          restoreSelected()
+          close()
         }
       })
 
@@ -560,7 +605,10 @@
       try {
         this.config.onChange(this.value)
       } catch (error) {
-        this.fail(error)
+        // A consumer bug is not an API failure: onError promises errors with
+        // .status/.code, and handlers switching on those would misroute this.
+        // Log it whole (stack included) without interrupting the widget.
+        console.error("[VehiclesDropdown] onChange callback failed", error)
       }
     }
 

package.json

@@ -42,7 +42,7 @@
     "README.md",
     "LICENSE"
   ],
-  "sideEffects": false,
+  "sideEffects": true,
   "devDependencies": {
     "jsdom": "^26.1.0"
   }

test/make-model-dropdown.test.js

@@ -141,9 +141,9 @@ test("attach validates its element and credential contract synchronously", () =>
 test("a throwing host callback cannot interrupt dependent model loading", async () => {
   const document = installDOM(`<select id="make"></select><select id="model"></select>`)
   const base = apiBase("callbacks")
-  const warnings = []
-  const originalWarn = console.warn
-  console.warn = (...args) => warnings.push(args)
+  const logged = []
+  const originalError = console.error
+  console.error = (...args) => logged.push(args)
   global.fetch = async (url) => {
     if (url.includes("/models")) return jsonResponse({ models: [{ name: "A3", slug: "a3" }] })
     return jsonResponse({ makes: [{ name: "Audi", slug: "audi" }] })
@@ -160,9 +160,11 @@ test("a throwing host callback cannot interrupt dependent model loading", async
     document.querySelector("#make").dispatchEvent(new Event("change", { bubbles: true }))
     await waitFor(() => document.querySelector("#model").options.length === 2, "models after callback failure")
     assert.equal(document.querySelector("#model").options[1].value, "a3")
-    assert.equal(warnings.length, 1)
+    // The consumer bug lands in console.error whole, and never in onError.
+    assert.equal(logged.length, 1)
+    assert.match(String(logged[0][1]), /consumer failed/)
   } finally {
-    console.warn = originalWarn
+    console.error = originalError
   }
 })
 
@@ -393,8 +395,10 @@ test("combobox stays in sync, restores typed text, and restores labels on destro
 
   input.value = "still not a make"
   input.dispatchEvent(new Event("blur"))
-  await new Promise((resolve) => setTimeout(resolve, 120))
+  // SYNCHRONOUSLY: a submit button clicked in the same tick as the blur must
+  // already see the visible text agreeing with the form value.
   assert.equal(input.value, "Audi")
+  assert.equal(select.value, "audi")
 
   instance.destroy()
   instances = instances.filter((candidate) => candidate !== instance)
@@ -484,33 +488,112 @@ test("logo blobs are credential-scoped and object URLs are instance-owned", asyn
   }
 })
 
-test("a transient logo failure can retry within the same instance", async () => {
+test("a failed logo fetch is a cached miss, not a retry storm", async () => {
   const document = installDOM(`<select id="make"></select>`)
   dom.window.HTMLElement.prototype.scrollIntoView = () => {}
-  const base = apiBase("logo-retry")
+  const base = apiBase("logo-miss")
   let logoCalls = 0
   global.fetch = async (url) => {
     if (new URL(url).pathname.endsWith("/logo")) {
       logoCalls += 1
-      return logoCalls === 1
-        ? new Response("temporary", { status: 503 })
-        : new Response(new Blob(["svg"]), { status: 200, headers: { "Content-Type": "image/svg+xml" } })
+      return new Response("denied", { status: 403 })
     }
     return jsonResponse({ makes: [{ name: "Audi", slug: "audi" }] })
   }
 
-  const originalCreate = URL.createObjectURL
-  URL.createObjectURL = () => "blob:retry"
-  try {
-    attach({ key: "logo-retry-key", apiBase: base, make: "#make", logos: true, attribution: false })
-    await waitFor(() => document.querySelector("#make").options.length === 2)
-    const input = document.querySelector("[role=combobox]")
+  attach({ key: "logo-miss-key", apiBase: base, make: "#make", logos: true, attribution: false })
+  await waitFor(() => document.querySelector("#make").options.length === 2)
+  const input = document.querySelector("[role=combobox]")
+  // Every focus re-renders the list; each row asks for its logo. A failing
+  // logo endpoint must cost ONE request per make for the whole pageview,
+  // never one per render.
+  for (let i = 0; i < 4; i += 1) {
     input.dispatchEvent(new Event("focus"))
-    await waitFor(() => logoCalls === 1)
     await new Promise((resolve) => setTimeout(resolve, 0))
-    input.dispatchEvent(new Event("focus"))
-    await waitFor(() => logoCalls === 2)
+    input.dispatchEvent(new Event("blur"))
+    await new Promise((resolve) => setTimeout(resolve, 0))
+  }
+  await new Promise((resolve) => setTimeout(resolve, 10))
+  assert.equal(logoCalls, 1, `expected a single cached miss, saw ${logoCalls} requests`)
+})
+
+test("focus plus Enter never overwrites a committed selection", async () => {
+  const document = installDOM(`<select id="make"></select>`)
+  dom.window.HTMLElement.prototype.scrollIntoView = () => {}
+  const base = apiBase("enter-guard")
+  global.fetch = async () => jsonResponse({ makes: [
+    { name: "Abarth", slug: "abarth" },
+    { name: "BMW", slug: "bmw" },
+    { name: "Toyota", slug: "toyota" }
+  ] })
+
+  attach({ key: "enter-key", apiBase: base, make: "#make", search: true, attribution: false })
+  await waitFor(() => document.querySelector("#make").options.length === 4)
+  const select = document.querySelector("#make")
+  const input = document.querySelector("[role=combobox]")
+
+  select.value = "toyota"
+  select.dispatchEvent(new Event("change", { bubbles: true }))
+  assert.equal(input.value, "Toyota")
+
+  // The bug this guards: focus opens the full list, and Enter with nothing
+  // active used to commit the first row alphabetically (Abarth).
+  input.dispatchEvent(new Event("focus"))
+  input.dispatchEvent(new KeyboardEvent("keydown", { key: "Enter", bubbles: true }))
+  assert.equal(select.value, "toyota", "Enter with no active row must not pick row zero")
+  assert.equal(input.value, "Toyota")
+
+  // Typing down to a single match plus Enter is still the productive path.
+  input.value = "bm"
+  input.dispatchEvent(new Event("input", { bubbles: true }))
+  input.dispatchEvent(new KeyboardEvent("keydown", { key: "Enter", bubbles: true }))
+  assert.equal(select.value, "bmw")
+  assert.equal(input.value, "BMW")
+
+  // Garbage text plus Enter restores the committed pair before any submit.
+  input.value = "zzzz"
+  input.dispatchEvent(new Event("input", { bubbles: true }))
+  input.dispatchEvent(new KeyboardEvent("keydown", { key: "Enter", bubbles: true }))
+  assert.equal(select.value, "bmw")
+  assert.equal(input.value, "BMW")
+})
+
+test("a dropped server preselection announces itself instead of diverging silently", async () => {
+  const document = installDOM(`
+    <select id="make"><option value="lada" selected>Lada</option></select>
+  `)
+  const base = apiBase("preselect-drop")
+  global.fetch = async () => jsonResponse({ makes: [{ name: "Audi", slug: "audi" }] })
+  const changes = []
+  const warnings = []
+  const originalWarn = console.warn
+  console.warn = (...parts) => warnings.push(parts.join(" "))
+  try {
+    attach({ key: "drop-key", apiBase: base, make: "#make", attribution: false,
+      onChange(value) { changes.push(value) } })
+    await waitFor(() => changes.length === 1)
   } finally {
-    URL.createObjectURL = originalCreate
+    console.warn = originalWarn
   }
+  assert.equal(document.querySelector("#make").value, "")
+  assert.equal(changes[0].make, null, "the consumer must hear that the selection is gone")
+  assert.ok(warnings.some((line) => line.includes("lada")), "the dropped value should be named")
+})
+
+test("a consumer onChange bug is not delivered to onError as an API failure", async () => {
+  const document = installDOM(`<select id="make"></select><select id="model"></select>`)
+  const base = apiBase("onchange-bug")
+  global.fetch = async (url) => new URL(url).pathname.endsWith("/models")
+    ? jsonResponse({ models: [] })
+    : jsonResponse({ makes: [{ name: "Audi", slug: "audi" }] })
+  const apiErrors = []
+  attach({ key: "bug-key", apiBase: base, make: "#make", model: "#model", attribution: false,
+    onChange() { throw new Error("consumer bug") },
+    onError(error) { apiErrors.push(error) } })
+  await waitFor(() => document.querySelector("#make").options.length === 2)
+  const select = document.querySelector("#make")
+  select.value = "audi"
+  select.dispatchEvent(new Event("change", { bubbles: true }))
+  await new Promise((resolve) => setTimeout(resolve, 20))
+  assert.equal(apiErrors.length, 0, "onError is for API failures, not consumer exceptions")
 })