feat(lit): support universal web components in v0.9 renderer - #2311
feat(lit): support universal web components in v0.9 renderer#2311josemontespg wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for Universal Web Components in the Lit renderer, adding BasicCatalog, NativeBasicCatalog, UniversalBasicCatalog, and a toWebComponent adapter. The review feedback focuses on simplifying component catalog instantiation and avoiding redundant or duplicated checks. Specifically, the reviewer suggests updating toWebComponent to return existing WebComponentImplementation objects directly (with defensive checks), which allows simplifying component overrides and extra component handling in UniversalBasicCatalog. Additionally, suggestions were made to simplify the NativeBasicCatalog constructor and align the getUseUniversalComponents helper in the shell app with the explorer's implementation for consistency.
| function getUseUniversalComponents(): boolean { | ||
| if (typeof window !== 'undefined' && window.location) { | ||
| const params = new URLSearchParams(window.location.search); | ||
| const val = params.get('useUniversalComponents'); | ||
| return val === 'true' || val === '1'; | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
The getUseUniversalComponents helper in the shell app defaults to true and checks both useUniversalComponent and useUniversalComponents to match the explorer's implementation, ensuring consistent behavior across both environments.
| function getUseUniversalComponents(): boolean { | |
| if (typeof window !== 'undefined' && window.location) { | |
| const params = new URLSearchParams(window.location.search); | |
| const val = params.get('useUniversalComponents'); | |
| return val === 'true' || val === '1'; | |
| } | |
| return false; | |
| } | |
| function getUseUniversalComponents(): boolean { | |
| if (typeof window === 'undefined') return true; | |
| const params = new URLSearchParams(window.location.search); | |
| const param = params.get('useUniversalComponent') || params.get('useUniversalComponents'); | |
| if (param !== null) { | |
| return param.toLowerCase() === 'true' || param === '1'; | |
| } | |
| return true; | |
| } |
| const baseComponents = new Map<string, LitComponentApi>( | ||
| Object.entries(DEFAULT_NATIVE_COMPONENT_IMPLEMENTATIONS).map(([key, impl]) => [ | ||
| impl.name || key, | ||
| impl, | ||
| ]), | ||
| ); |
There was a problem hiding this comment.
The keys of DEFAULT_NATIVE_COMPONENT_IMPLEMENTATIONS are already mapped to impl.name || key. Mapping them again in the NativeBasicCatalog constructor is redundant. You can pass Object.entries(DEFAULT_NATIVE_COMPONENT_IMPLEMENTATIONS) directly to the Map constructor.
const baseComponents = new Map<string, LitComponentApi>(
Object.entries(DEFAULT_NATIVE_COMPONENT_IMPLEMENTATIONS)
);| if (options.components) { | ||
| for (const [key, comp] of Object.entries(options.components)) { | ||
| if (comp) { | ||
| let resolvedComp = | ||
| 'tagName' in comp && | ||
| typeof comp.tagName === 'string' && | ||
| !('element' in comp) && | ||
| !('render' in comp) | ||
| ? comp | ||
| : toWebComponent(comp); |
There was a problem hiding this comment.
This complex check to determine if a component is already a WebComponentImplementation is duplicated multiple times in this file and in toWebComponent. By updating toWebComponent to return the component directly if it is already a WebComponentImplementation, we can simplify this logic and avoid duplication.
| if (options.components) { | |
| for (const [key, comp] of Object.entries(options.components)) { | |
| if (comp) { | |
| let resolvedComp = | |
| 'tagName' in comp && | |
| typeof comp.tagName === 'string' && | |
| !('element' in comp) && | |
| !('render' in comp) | |
| ? comp | |
| : toWebComponent(comp); | |
| if (options.components) { | |
| for (const [key, comp] of Object.entries(options.components)) { | |
| if (comp) { | |
| let resolvedComp = toWebComponent(comp); |
| const extra = (options.extraComponents ?? []).map(comp => { | ||
| if ( | ||
| 'tagName' in comp && | ||
| typeof comp.tagName === 'string' && | ||
| !('element' in comp) && | ||
| !('render' in comp) | ||
| ) { | ||
| return comp; | ||
| } | ||
| return toWebComponent(comp); | ||
| }); |
There was a problem hiding this comment.
Similar to the components override, this duplicated check can be entirely avoided by relying on toWebComponent directly, provided toWebComponent is updated to return the component directly if it is already a WebComponentImplementation.
const extra = (options.extraComponents ?? []).map(comp => toWebComponent(comp));e08e39b to
c6ee897
Compare
c6ee897 to
b61f60b
Compare
b61f60b to
cc371ac
Compare
cc371ac to
10069d4
Compare
| }); | ||
| }); | ||
|
|
||
| describe('toWebComponent Adapter', () => { |
There was a problem hiding this comment.
Move these tests to their own file next to the implementation file.
There was a problem hiding this comment.
Moved toWebComponent tests into renderers/lit/src/v0_9/catalogs/to_web_component.test.ts next to the implementation file.
|
|
||
| after(teardownTestDom); | ||
|
|
||
| describe('Basic Catalog Re-export', () => { |
There was a problem hiding this comment.
Remove this test. It is not super useful
There was a problem hiding this comment.
Removed the basic catalog re-export test.
c237788 to
abbad7a
Compare
| * Interface representing a Lit or Web Component implementation. | ||
| */ | ||
| export type LitComponentImplementation<Schema extends ZodTypeAny = ZodTypeAny> = | ||
| WebComponentImplementation<Schema>; |
There was a problem hiding this comment.
This is not necessary. Remove.
There was a problem hiding this comment.
Removed LitComponentImplementation.
| */ | ||
| export type LitComponentApi = WebComponentImplementation; | ||
| export type LitComponentApi<Schema extends ZodTypeAny = ZodTypeAny> = | ||
| WebComponentImplementation<Schema>; |
There was a problem hiding this comment.
This is not needed. Remove.
There was a problem hiding this comment.
Reverted LitComponentApi back to export type LitComponentApi = WebComponentImplementation;.
There was a problem hiding this comment.
Is this needed?
There was a problem hiding this comment.
Reverted formatting script changes in samples/client/lit/shell/package.json.
There was a problem hiding this comment.
Remove this test file
There was a problem hiding this comment.
Removed universal-components.test.ts.
There was a problem hiding this comment.
Why did this test need to change so much?
There was a problem hiding this comment.
In PR #2190, BasicCatalogA2uiLitElement and its test suite were migrated to renderers/web_core/src/v0_9/basic_catalog/. This test file in renderers/lit was a redundant legacy copy testing old internal class properties, so it has been removed.
abbad7a to
8b73fd9
Compare
27b0817 to
1613e57
Compare
1613e57 to
038f77e
Compare
038f77e to
d3f10fa
Compare
Summary
This PR adds support for universal web components in the
@a2ui/litv0.9 renderer, matching the architecture and design established in Angular (#2273) and React (#2283).Key Changes:
BasicCatalogOptionsinterface supporting catalog ID, locale, component overrides (components), extra components (extraComponents), custom functions (functions), and theuseUniversalComponentstoggle.NativeBasicCatalog,UniversalBasicCatalog, andBasicCatalogclasses.BASIC_COMPONENTS,BASIC_NATIVE_COMPONENTS,BASIC_UNIVERSAL_COMPONENTS, andBASIC_FUNCTIONS.toWebComponentadapter to convert custom Lit element classes or render functions ((context) => TemplateResult) into standardWebComponentImplementationcustom element definitions.createComponentImplementationhelper for constructing component implementations.renderA2uiNodein Lit surface rendering to acceptCatalog<LitComponentApi>and dynamically resolve and mount custom web components into Light DOM.useUniversalComponentsvia query parameter.universal-components.test.tsverifying catalog instantiation, component overrides, extra components, locale functions,toWebComponentcaching/idempotency, and end-to-end surface rendering.basic-catalog-a2ui-lit-element.test.tsto verify light DOM, styling, flex weight, and theme color behavior.renderers/lit/CHANGELOG.md.Pre-launch Checklist
CHANGELOG.mdwith notes describing my changes.