Skip to content

Conversation

benjamind
Copy link
Contributor

Description

This PR explores a POC for adding prefixing of spectrum-web-component custom element registrations, allowing us to avoid versioning conflicts (at the expense of payload).

Motivation and context

Related issue(s)

  • fixes [Issue Number]

Screenshots (if appropriate)


Author's checklist

  • I have read the CONTRIBUTING and PULL_REQUESTS documents.
  • I have reviewed at the Accessibility Practices for this feature, see: Aria Practices
  • I have added automated tests to cover my changes.
  • I have included a well-written changeset if my change needs to be published.
  • I have included updated documentation if my change required it.

Reviewer's checklist

  • Includes a Github Issue with appropriate flag or Jira ticket number without a link
  • Includes thoughtfully written changeset if changes suggested include patch, minor, or major features
  • Automated tests cover all use cases and follow best practices for writing
  • Validated on all supported browsers
  • All VRTs are approved before the author can update Golden Hash

Manual review test cases

  • Descriptive Test Statement

    1. Go here
    2. Do this action
    3. Expect this result
  • Descriptive Test Statement

    1. Go here
    2. Do this action
    3. Expect this result

Device review

  • Did it pass in Desktop?
  • Did it pass in (emulated) Mobile?
  • Did it pass in (emulated) iPad?

Copy link

changeset-bot bot commented Aug 20, 2025

⚠️ No Changeset found

Latest commit: 6704725

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@benjamind
Copy link
Contributor Author

Initial tachometer results show this is actually faster than the original registration which I find hard to believe, but may be due to my having updated to the latest playwright install.

┌─────────────┬───────────────────────┐
│   Benchmark │ action-bar:basic-test │
├─────────────┼───────────────────────┤
│     Browser │ chrome-headless       │
│             │ 139.0.0.0             │
├─────────────┼───────────────────────┤
│ Sample size │ 50                    │
└─────────────┴───────────────────────┘

┌─────────┬────────────┬───────────────────┬─────────────────┬─────────────────┐
│ Version │ Bytes      │          Avg time │       vs remote │             vs  │
├─────────┼────────────┼───────────────────┼─────────────────┼─────────────────┤
│ remote  │ 614.21 KiB │ 15.17ms - 15.35ms │                 │          slower │
│         │            │                   │        -        │         2% - 3% │
│         │            │                   │                 │ 0.24ms - 0.47ms │
├─────────┼────────────┼───────────────────┼─────────────────┼─────────────────┤
│ <none>  │ 590.44 KiB │ 14.84ms - 14.97ms │          faster │                 │
│         │            │                   │         2% - 3% │        -        │
│         │            │                   │ 0.24ms - 0.47ms │                 │
└─────────┴────────────┴───────────────────┴─────────────────┴─────────────────┘

@graynorton graynorton self-requested a review August 20, 2025 01:19
@rubencarvalho rubencarvalho self-requested a review August 20, 2025 15:54
Copy link
Collaborator

@graynorton graynorton left a comment

Choose a reason for hiding this comment

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

This general approach looks promising to me!

I wanted to see how it might look end-to-end, so I created a Lit Playground example to try it out:

https://lit.dev/playground/#gist=039ebbad69ce634ff3cf6d4d14c7df01

In doing so, I found that there were a couple of issues:

  • We need to provide StaticValues to render into Lit templates in the tag-name position; strings aren't sufficient
  • The logic for rendering dependent tag names needs to combine the prefix from the current component instance with the tagName from the dependency's constructor; as written in this PR, it gets both from the base constructor, so doesn't utilize the custom prefix

In the process of addressing these, I ended up making a bunch of other experimental tweaks, primarily in the interest of improving dev experience.

Take a look at the Playground and see what you think. In particular, I ended up going down a path that tried to adhere closely to Lit's template-safety best practices, so requires use of a tagged template literal when defining prefix and tagName. This is responsible for a bit of complexity in the implementation, so I could imagine deciding that plain strings + unsafeStatic are good enough for our use case, but I wanted to see what the more rigorous approach would look like.

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.

2 participants