Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Infra: Provide solution for duplicate component registration #3

Closed
daneah opened this issue May 4, 2021 · 2 comments · Fixed by #226
Closed

Infra: Provide solution for duplicate component registration #3

daneah opened this issue May 4, 2021 · 2 comments · Fixed by #226
Assignees
Labels
BREAKING feature request infrastructure status/has ticket This issue will be addressed by the maintainers

Comments

@daneah
Copy link
Member

daneah commented May 4, 2021

The problem

Right now Pharos requires using its own @customElement decorator to ensure no runtime errors when a component is registered multiple times. This doesn't cover third-party components we make use of, and doesn't provide deterministic behavior if two differing versions of the component try to register (the first one wins).

The solution

Use the scoped element mixin to provide an axis of freedom in registering components under another name.

Alternatives considered

In the long term scoped custom element registration will help solve this issue natively.

@daneah daneah pinned this issue Jul 14, 2021
@daneah daneah changed the title Provide solution for duplicate component registration Infra: Provide solution for duplicate component registration Jul 15, 2021
@Niznikr
Copy link
Collaborator

Niznikr commented Aug 23, 2021

Lit itself also provides a separate mixin that integrates with the CustomElementRegistry polyfill. Because shadow roots are used as registration scopes in the proposal, we need to perhaps provide a helper for consumers to scope their version of Pharos. This helper would attach a shadow root to a given element and then define Pharos components on it for folks to use and scope their app/page.

@Niznikr Niznikr added status/has ticket This issue will be addressed by the maintainers and removed status/on hold labels Aug 24, 2021
@Niznikr
Copy link
Collaborator

Niznikr commented Sep 2, 2021

There are two possible solutions for us:

  1. We create a <pharos-registry> component that utilizes the @lit-labs/scoped-registry-mixin to scope our components within its shadow root. Consumers could then use the component as such:
<template id="my-template">
  <h1>My page template</h1>
  <pharos-button>hi</pharos-button>
</template>
<pharos-registry></pharos-registry>

And in their JS entry file:

import { PharosRegistry } from '@ithaka/pharos/lib/components/core/pharos-registry';
customElements.define('pharos-registry', PharosRegistry);

document.querySelector('pharos-registry').shadowRoot.append(document.querySelector('#my-template').content);

This would remove the need for consumers to import individual components but at the cost of needing for their template to exist in the shadow DOM which would require changes to references of document.querySelector and integration tests.

  1. Only use @lit-labs/scoped-registry-mixin within our own components that import other Pharos components and remove the @customElement decorator to stop self-registering. Instead we only export the component class and let consumers define a unique tag for their app:
import { PharosButton } from '@ithaka/pharos/lib/components/button/pharos-button';
customElements.define('pharos-button-homepage', PharosButton);

This would place responsibility on the consumer to define a unique tag. Any integration test helpers would need to allow configuration for custom tags because of this new flexibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING feature request infrastructure status/has ticket This issue will be addressed by the maintainers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants