Skip to content

WEBDEV-9066: Guard custom element registration against duplicate defines - #106

Merged
jbuckner merged 2 commits into
mainfrom
WEBDEV-9066-guard-custom-element-defines
Sep 16, 2026
Merged

jbuckner merged 2 commits into
mainfrom
WEBDEV-9066-guard-custom-element-defines

Conversation

@jbuckner

Copy link
Copy Markdown
Collaborator

Importing two element subpaths on the same page throws and one of them never registers. Repros on the published 0.4.0 in headless Chromium:

Failed to execute 'define' on 'CustomElementRegistry': the name "ia-status-indicator" has already been used with this registry

Each subpath esm.archive.org serves is its own self-contained bundle, so a shared component is built into every entry point that imports it. Two subpaths means two copies and two customElements.define() calls for the same tag. The second throws mid-evaluation, which aborts the rest of that bundle, so it's partway through: some tags from the failed bundle register and the rest are lost. Bundler consumers like offshoot never see it because Vite dedupes to one instance.

12 tags are defined by more than one entry point. ia-status-indicator is in ia-button, ia-dropdown-search-bar, ia-otp-form, ia-radio-player and its own subpath, so any two of those collide.

So this adds src/util/custom-element.ts, a @customElement that skips the define when the tag is already claimed, and points every call site at it. First copy wins. That's safe here: nothing does instanceof on a shared element class and the only class-level imports of one are import type, so the copies are interchangeable. There's an eslint rule to keep Lit's version from coming back.

The demo and story files are swapped too. They aren't published so they can't hit this, but leaving them on the Lit decorator just invites someone to copy the wrong import into a new element.

What I didn't do

There's a second, structural fix: a single-URL entry point would give non-bundler consumers one module instance and no duplicates at all. That's a bigger change and it argues with #81, which drops the package root. Skipping it for now since petabox usage is getting deprecated anyway. The guard is worth having on its own for any non-bundler consumer.

Verifying

Unit tests cover the guard, including that registration keeps working after a duplicate is skipped, which is the part that actually breaks.

The real failure mode needs two separately-built bundles, which no unit test can produce, so I checked that by hand: esbuild dist/.../ia-dropdown-search-bar and dist/.../ia-radio-player into two self-contained bundles and load both on one page.

  • Without the guard: radio-player throws, doesn't register, doesn't upgrade, and ia-transcript-view and ia-waveform-progress are lost with it.
  • With it: both import, all six tags register, both elements attach a shadow root.

694 tests pass, lint 0 errors, prettier clean.

https://webarchive.jira.com/browse/WEBDEV-9066

🤖 Generated with Claude Code

https://claude.ai/code/session_01PoknqYwGpzkxFE3kfcYW81

Each element subpath we serve to non-bundler consumers is its own
self-contained bundle, so a component several elements share gets built into
every entry point that imports it. Two of those subpaths on one page means two
copies of the shared component, and the second customElements.define() for the
same tag throws. It throws while the module is still evaluating, so it takes
the rest of that bundle down with it and the element the page actually wanted
never registers.

ia-status-indicator is the worst of these: ia-button, ia-dropdown-search-bar,
ia-otp-form, ia-radio-player and its own subpath all carry a copy, so any two
of those collide. There are 12 tags in that position.

This swaps Lit's @CustomElement for our own, which skips the define when the
tag is already claimed. First copy wins, which is fine, the copies come from
the same source and nothing narrows a shared element by class identity. An
eslint rule keeps the Lit one from creeping back.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PoknqYwGpzkxFE3kfcYW81
@github-actions

github-actions Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-16 17:14 UTC

@codecov-commenter

codecov-commenter commented Sep 15, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.80%. Comparing base (631605d) to head (e1df5b2).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/util/custom-element.ts 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #106   +/-   ##
=======================================
  Coverage   85.79%   85.80%           
=======================================
  Files          74       75    +1     
  Lines        2619     2627    +8     
  Branches      577      579    +2     
=======================================
+ Hits         2247     2254    +7     
  Misses        195      195           
- Partials      177      178    +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@latonv latonv left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just had one thought about possible logging when we skip a define.

Comment thread src/util/custom-element.ts Outdated
Skipping the define quietly is fine when two subpaths share a component, but
it's also how a page ends up on an element it didn't mean to use: load two
versions of the package, or let the host page claim one of these tag names,
and whatever registered first is what you get. That used to throw, so it was
at least loud. Now it needs saying out loud.

Nothing here can tell a benign duplicate from a version mismatch, since two
builds of the same element differ in nothing observable at this point, so the
message names the tag and lays out both readings.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jbuckner
jbuckner merged commit 4bf26ec into main Sep 16, 2026
4 checks passed
@jbuckner
jbuckner deleted the WEBDEV-9066-guard-custom-element-defines branch September 16, 2026 17:14
@jbuckner jbuckner mentioned this pull request Sep 16, 2026
jbuckner added a commit that referenced this pull request Sep 16, 2026
* origin/main:
  WEBDEV-9063: Typecheck in CI (#107)
  v0.4.1 (#108)
  WEBDEV-9066: Guard custom element registration against duplicate defines (#106)
jbuckner added a commit that referenced this pull request Sep 24, 2026
…onation-thermometer

* origin/main:
  1.1.1 (#118)
  WEBDEV-9132: Fix Safari image-viewer wide-layout breakpoint (#115)
  1.1.0 (#116)
  WEBDEV-9129: Exclude story files from coverage reporting (#113)
  WEBDEV-9121: Migrate histogram-date-range into elements (#112)
  WEBDEV-9130: Exclude nested dist and worktree tests from local runs (#114)
  v1.0.0 (#111)
  WEBDEV-9025: Add a CLAUDE.md covering the publish flow (#94)
  WEBDEV-9063: Typecheck in CI (#107)
  v0.4.1 (#108)
  WEBDEV-9066: Guard custom element registration against duplicate defines (#106)
jbuckner added a commit that referenced this pull request Sep 24, 2026
…onation-form-models

* origin/main:
  1.1.1 (#118)
  WEBDEV-9132: Fix Safari image-viewer wide-layout breakpoint (#115)
  1.1.0 (#116)
  WEBDEV-9129: Exclude story files from coverage reporting (#113)
  WEBDEV-9121: Migrate histogram-date-range into elements (#112)
  WEBDEV-9130: Exclude nested dist and worktree tests from local runs (#114)
  v1.0.0 (#111)
  WEBDEV-9025: Add a CLAUDE.md covering the publish flow (#94)
  WEBDEV-9063: Typecheck in CI (#107)
  v0.4.1 (#108)
  WEBDEV-9066: Guard custom element registration against duplicate defines (#106)
  v0.4.0 (#99)
  WEBDEV-9022: Migrate the image viewer into elements (#93)
jbuckner added a commit that referenced this pull request Sep 24, 2026
…opnav

* origin/main:
  1.1.1 (#118)
  WEBDEV-9132: Fix Safari image-viewer wide-layout breakpoint (#115)
  1.1.0 (#116)
  WEBDEV-9129: Exclude story files from coverage reporting (#113)
  WEBDEV-9121: Migrate histogram-date-range into elements (#112)
  WEBDEV-9130: Exclude nested dist and worktree tests from local runs (#114)
  v1.0.0 (#111)
  WEBDEV-9025: Add a CLAUDE.md covering the publish flow (#94)
  WEBDEV-9063: Typecheck in CI (#107)
  v0.4.1 (#108)
  WEBDEV-9066: Guard custom element registration against duplicate defines (#106)
  v0.4.0 (#99)
  WEBDEV-9022: Migrate the image viewer into elements (#93)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants