From b04bcf18c1b2baf7a144b249b1bd59c8cb3b25f0 Mon Sep 17 00:00:00 2001 From: Lukas Harbarth Date: Wed, 11 Mar 2026 17:31:21 +0100 Subject: [PATCH 1/3] refactor(withWebcomponents): extract stable consts & fix unstable effect deps --- .../src/internal/wrapper/withWebComponent.tsx | 132 ++++++++++-------- 1 file changed, 70 insertions(+), 62 deletions(-) diff --git a/packages/base/src/internal/wrapper/withWebComponent.tsx b/packages/base/src/internal/wrapper/withWebComponent.tsx index 21a6cfa66fe..080af220577 100644 --- a/packages/base/src/internal/wrapper/withWebComponent.tsx +++ b/packages/base/src/internal/wrapper/withWebComponent.tsx @@ -26,6 +26,7 @@ export interface WithWebComponentPropTypes { } const definedWebComponents = new Set([]); + /** * ⚠️ __INTERNAL__ use only! This function is not part of the public API. */ @@ -37,35 +38,42 @@ export const withWebComponent = , RefType = Ui eventProperties: string[], ) => { const webComponentsSupported = parseSemVer(version).major >= 19; + const regularKebabNames = regularProperties.map(camelToKebabCase); + const booleanKebabNames = booleanProperties.map(camelToKebabCase); + const eventPropNames = eventProperties.map(createEventPropName); + const knownKeys = new Set([...regularProperties, ...slotProperties, ...booleanProperties, ...eventPropNames]); + const tagNameSuffix: string = getEffectiveScopingSuffixForTag(tagName); + const Component = (tagNameSuffix ? `${tagName}-${tagNameSuffix}` : tagName) as unknown as ComponentType< + CommonProps & { class?: string; ref?: Ref } + >; + // displayName will be assigned in the individual files // eslint-disable-next-line react/display-name return forwardRef((props, wcRef) => { const { className, children, waitForDefine, ...rest } = props; const [componentRef, ref] = useSyncRef(wcRef); - const tagNameSuffix: string = getEffectiveScopingSuffixForTag(tagName); - const Component = (tagNameSuffix ? `${tagName}-${tagNameSuffix}` : tagName) as unknown as ComponentType< - CommonProps & { class?: string; ref?: Ref } - >; const [isDefined, setIsDefined] = useState(definedWebComponents.has(Component)); // regular props (no booleans, no slots and no events) - const regularProps = regularProperties.reduce((acc, name) => { + const regularProps: Record = {}; + for (let i = 0; i < regularProperties.length; i++) { + const name = regularProperties[i]; if (Object.prototype.hasOwnProperty.call(rest, name) && isPrimitiveAttribute(rest[name])) { - return { ...acc, [camelToKebabCase(name)]: rest[name] }; + regularProps[regularKebabNames[i]] = rest[name]; } - return acc; - }, {}); + } // boolean properties - only attach if they are truthy - const booleanProps = booleanProperties.reduce((acc, name) => { + const booleanProps: Record = {}; + for (let i = 0; i < booleanProperties.length; i++) { + const name = booleanProperties[i]; if (webComponentsSupported) { - return { ...acc, [camelToKebabCase(name)]: rest[name] }; + booleanProps[booleanKebabNames[i]] = rest[name]; } else { if (rest[name] === true || rest[name] === 'true') { - return { ...acc, [camelToKebabCase(name)]: true }; + booleanProps[booleanKebabNames[i]] = true; } - return acc; } - }, {}); + } const slots = slotProperties.reduce((acc, name) => { const slotValue = rest[name] as ReactElement; @@ -117,58 +125,57 @@ export const withWebComponent = , RefType = Ui return [...acc, ...slottedChildren]; }, []); - // event binding - useIsomorphicLayoutEffect(() => { - if (webComponentsSupported) { - return () => { - // React can handle events - }; - } - const localRef = ref.current; - const eventRegistry: Record = {}; - if (!waitForDefine || isDefined) { - eventProperties.forEach((eventName) => { - const eventHandler = rest[createEventPropName(eventName)] as EventHandler; - if (typeof eventHandler === 'function') { - eventRegistry[eventName] = eventHandler; - // @ts-expect-error: all custom events can be passed here, so `keyof HTMLElementEventMap` isn't sufficient - localRef?.addEventListener(eventName, eventRegistry[eventName]); - } - }); + // event binding - React 19 supports this natively + if (!webComponentsSupported) { + // React version never changes between renders + // eslint-disable-next-line react-hooks/rules-of-hooks + useIsomorphicLayoutEffect(() => { + const localRef = ref.current; + const eventRegistry: Record = {}; + if (!waitForDefine || isDefined) { + eventProperties.forEach((eventName, i) => { + const eventHandler = rest[eventPropNames[i]] as EventHandler; + if (typeof eventHandler === 'function') { + eventRegistry[eventName] = eventHandler; + // @ts-expect-error: all custom events can be passed here, so `keyof HTMLElementEventMap` isn't sufficient + localRef?.addEventListener(eventName, eventRegistry[eventName]); + } + }); - return () => { - for (const eventName in eventRegistry) { - // @ts-expect-error: all custom events can be passed here, so `keyof HTMLElementEventMap` isn't sufficient - localRef?.removeEventListener(eventName, eventRegistry[eventName]); - } - }; - } - }, [...eventProperties.map((eventName) => rest[createEventPropName(eventName)]), isDefined, waitForDefine]); + return () => { + for (const eventName in eventRegistry) { + // @ts-expect-error: all custom events can be passed here, so `keyof HTMLElementEventMap` isn't sufficient + localRef?.removeEventListener(eventName, eventRegistry[eventName]); + } + }; + } + }, [...eventPropNames.map((propName) => rest[propName]), isDefined, waitForDefine]); + } - const eventHandlers = eventProperties.reduce((events, eventName) => { - const eventHandlerProp = rest[createEventPropName(eventName)]; - if (webComponentsSupported && eventHandlerProp) { - events[`on${eventName}`] = eventHandlerProp; + const eventHandlers: Record = {}; + if (webComponentsSupported) { + for (let i = 0; i < eventProperties.length; i++) { + const eventHandlerProp = rest[eventPropNames[i]]; + if (eventHandlerProp) { + eventHandlers[`on${eventProperties[i]}`] = eventHandlerProp; + } } - return events; - }, {}); + } // In React 19 events aren't correctly attached after hydration const [attachEvents, setAttachEvents] = useState(!webComponentsSupported || !Object.keys(eventHandlers).length); // apply workaround only for React19 and if event props are defined // non web component related props, just pass them - const nonWebComponentRelatedProps = Object.entries(rest) - .filter(([key]) => !regularProperties.includes(key)) - .filter(([key]) => !slotProperties.includes(key)) - .filter(([key]) => !booleanProperties.includes(key)) - .filter(([key]) => !eventProperties.map((eventName) => createEventPropName(eventName)).includes(key)) - .reduce((acc, [key, val]) => { + const nonWebComponentRelatedProps: Record = {}; + for (const key in rest) { + if (Object.prototype.hasOwnProperty.call(rest, key) && !knownKeys.has(key)) { + const val = rest[key]; if (!key.startsWith('aria-') && !key.startsWith('data-') && val === false) { - return acc; + continue; } - acc[key] = val; - return acc; - }, {}); + nonWebComponentRelatedProps[key] = val; + } + } useEffect(() => { if (waitForDefine && !isDefined) { @@ -177,20 +184,22 @@ export const withWebComponent = , RefType = Ui definedWebComponents.add(Component); }); } - }, [Component, waitForDefine, isDefined]); + }, [waitForDefine, isDefined]); - const propsToApply = regularProperties.map((prop) => ({ name: prop, value: props[prop] })); + const regularPropValues = regularProperties.map((prop) => props[prop]); useEffect(() => { void customElements.whenDefined(Component as unknown as string).then(() => { - for (const prop of propsToApply) { - if (prop.value != null && !isPrimitiveAttribute(prop.value)) { + for (let i = 0; i < regularProperties.length; i++) { + const value = regularPropValues[i]; + if (value != null && !isPrimitiveAttribute(value)) { if (ref.current) { - ref.current[prop.name] = prop.value; + ref.current[regularProperties[i]] = value; } } } }); - }, [Component, ...propsToApply]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [Component, ...regularPropValues]); useIsomorphicLayoutEffect(() => { setAttachEvents(true); @@ -203,7 +212,6 @@ export const withWebComponent = , RefType = Ui // compatibility wrapper for ExpandableText - remove in v3 if (tagName === 'ui5-expandable-text') { const renderWhiteSpace = nonWebComponentRelatedProps['renderWhitespace'] ? true : undefined; - // @ts-expect-error: overflowMode is available const { ['overflow-mode']: overflowMode, text, ...restRegularProps } = regularProps; const showOverflowInPopover = nonWebComponentRelatedProps['showOverflowInPopover']; return ( From 5f926f467e814025be885e5f99825f415fc3c6e1 Mon Sep 17 00:00:00 2001 From: Lukas Harbarth Date: Thu, 12 Mar 2026 07:41:41 +0100 Subject: [PATCH 2/3] Update withWebComponent.tsx --- packages/base/src/internal/wrapper/withWebComponent.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/base/src/internal/wrapper/withWebComponent.tsx b/packages/base/src/internal/wrapper/withWebComponent.tsx index 080af220577..57467de9828 100644 --- a/packages/base/src/internal/wrapper/withWebComponent.tsx +++ b/packages/base/src/internal/wrapper/withWebComponent.tsx @@ -199,7 +199,7 @@ export const withWebComponent = , RefType = Ui } }); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [Component, ...regularPropValues]); + }, [...regularPropValues]); useIsomorphicLayoutEffect(() => { setAttachEvents(true); From 429bb8871aeb78782bf58ee324fe392240e4c2af Mon Sep 17 00:00:00 2001 From: Lukas Harbarth Date: Thu, 12 Mar 2026 16:14:44 +0100 Subject: [PATCH 3/3] Update withWebComponent.cy.tsx --- .../internal/wrapper/withWebComponent.cy.tsx | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/packages/base/src/internal/wrapper/withWebComponent.cy.tsx b/packages/base/src/internal/wrapper/withWebComponent.cy.tsx index c9880aae449..802d12e7dcb 100644 --- a/packages/base/src/internal/wrapper/withWebComponent.cy.tsx +++ b/packages/base/src/internal/wrapper/withWebComponent.cy.tsx @@ -5,6 +5,7 @@ import { import type { ButtonDomRef } from '@ui5/webcomponents-react'; import { Bar, Button, Popover, Switch } from '@ui5/webcomponents-react'; import { useReducer, useRef, useState } from 'react'; +import { withWebComponent } from './withWebComponent.js'; describe('withWebComponent', () => { // reset scoping @@ -164,24 +165,19 @@ describe('withWebComponent', () => { cy.findByText('Btn').should('not.have.attr', 'disabled'); }); + // the underlying custom-element will not be updated, as the scoping suffix has to be set before any ui5wc import it('scoping', () => { - const TestComp = () => { - setCustomElementsScopingSuffix('ui5-wcr'); - return ; - }; - - const TestComp2 = () => { - setCustomElementsScopingSuffix('ui5-wcr'); - setCustomElementsScopingRules({ include: [/^ui5-/], exclude: [/^ui5-button/] }); - return ; - }; + setCustomElementsScopingSuffix('ui5-wcr'); + const ScopedButton = withWebComponent('ui5-button', [], [], [], []); - cy.mount(); + cy.mount(Test); cy.get('ui5-button-ui5-wcr').should('be.visible'); cy.get('ui5-button').should('not.exist'); - // now exclude the button - cy.mount(); + setCustomElementsScopingRules({ include: [/^ui5-/], exclude: [/^ui5-button/] }); + const UnscopedButton = withWebComponent('ui5-button', [], [], [], []); + + cy.mount(Test); cy.get('ui5-button').should('be.visible'); cy.get('ui5-button-ui5-wcr').should('not.exist'); });