From 91d8250fe31f673c8848afbb5cfa8ca8008f888c Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Tue, 24 Dec 2024 19:55:22 +0400 Subject: [PATCH 1/4] refactor: drop lit/react in favor of React built-in support --- dev/kitchen-sink/Row1.tsx | 2 +- dev/pages/Select.tsx | 17 +++ package-lock.json | 82 +++++------- package.json | 2 +- packages/react-components-pro/package.json | 9 +- packages/react-components/package.json | 13 +- packages/react-components/src/Select.tsx | 3 +- .../src/renderers/useRenderer.ts | 2 +- .../src/utils/createComponent.ts | 118 ++++++++++++++---- .../src/utils/useMergedRefs.ts | 4 +- scripts/generator.ts | 4 +- test/Select.spec.tsx | 8 +- 12 files changed, 160 insertions(+), 104 deletions(-) create mode 100644 dev/pages/Select.tsx diff --git a/dev/kitchen-sink/Row1.tsx b/dev/kitchen-sink/Row1.tsx index 245abfc3..2a031022 100644 --- a/dev/kitchen-sink/Row1.tsx +++ b/dev/kitchen-sink/Row1.tsx @@ -25,7 +25,7 @@ export default function Row1() {
Accordion content 2
- + diff --git a/dev/pages/Select.tsx b/dev/pages/Select.tsx new file mode 100644 index 00000000..0a37f8a1 --- /dev/null +++ b/dev/pages/Select.tsx @@ -0,0 +1,17 @@ +import { Select } from '../../packages/react-components/src/Select.js'; + +export default function () { + return ( + <> + + + ); +} diff --git a/package-lock.json b/package-lock.json index d9037142..587abffa 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21,7 +21,7 @@ "@types/karma-chrome-launcher": "^3.1.4", "@types/karma-mocha": "^1.3.4", "@types/mocha": "^10.0.10", - "@types/react-dom": "^18.3.3", + "@types/react-dom": "^19", "@types/sinon": "^17.0.3", "@vitejs/plugin-react": "^4.3.4", "chai-as-promised": "^8.0.1", @@ -922,14 +922,6 @@ "resolved": "https://registry.npmjs.org/@lit-labs/ssr-dom-shim/-/ssr-dom-shim-1.2.1.tgz", "integrity": "sha512-wx4aBmgeGvFmOKucFKY+8VFJSYZxs9poN3SDNQFF6lT6NrQUnHiPB2PWz2sc4ieEcAaYYzN+1uWahEeTq2aRIQ==" }, - "node_modules/@lit/react": { - "version": "1.0.6", - "resolved": "https://registry.npmjs.org/@lit/react/-/react-1.0.6.tgz", - "integrity": "sha512-QIss8MPh6qUoFJmuaF4dSHts3qCsA36S3HcOLiNPShxhgYPr4XJRnCBKPipk85sR9xr6TQrOcDMfexwbNdJHYA==", - "peerDependencies": { - "@types/react": "17 || 18" - } - }, "node_modules/@lit/reactive-element": { "version": "2.0.4", "resolved": "https://registry.npmjs.org/@lit/reactive-element/-/reactive-element-2.0.4.tgz", @@ -1510,29 +1502,25 @@ "undici-types": "~6.20.0" } }, - "node_modules/@types/prop-types": { - "version": "15.7.14", - "resolved": "https://registry.npmjs.org/@types/prop-types/-/prop-types-15.7.14.tgz", - "integrity": "sha512-gNMvNH49DJ7OJYv+KAKn0Xp45p8PLl6zo2YnvDIbTd4J6MER2BmWN49TG7n9LvkyihINxeKW8+3bfS2yDC9dzQ==", - "peer": true - }, "node_modules/@types/react": { - "version": "18.3.17", - "resolved": "https://registry.npmjs.org/@types/react/-/react-18.3.17.tgz", - "integrity": "sha512-opAQ5no6LqJNo9TqnxBKsgnkIYHozW9KSTlFVoSUJYh1Fl/sswkEoqIugRSm7tbh6pABtYjGAjW+GOS23j8qbw==", + "version": "19.0.2", + "resolved": "https://registry.npmjs.org/@types/react/-/react-19.0.2.tgz", + "integrity": "sha512-USU8ZI/xyKJwFTpjSVIrSeHBVAGagkHQKPNbxeWwql/vDmnTIBgx+TJnhFnj1NXgz8XfprU0egV2dROLGpsBEg==", + "devOptional": true, + "license": "MIT", "peer": true, "dependencies": { - "@types/prop-types": "*", "csstype": "^3.0.2" } }, "node_modules/@types/react-dom": { - "version": "18.3.5", - "resolved": "https://registry.npmjs.org/@types/react-dom/-/react-dom-18.3.5.tgz", - "integrity": "sha512-P4t6saawp+b/dFrUr2cvkVsfvPguwsxtH6dNIYRllMsefqFzkZk5UIjzyDOv5g1dXIPdG4Sp1yCR4Z6RCUsG/Q==", + "version": "19.0.2", + "resolved": "https://registry.npmjs.org/@types/react-dom/-/react-dom-19.0.2.tgz", + "integrity": "sha512-c1s+7TKFaDRRxr1TxccIX2u7sfCnc3RxkVyBIUA2lCpyqCF+QoAwQ/CBg7bsMdVwP120HEH143VQezKtef5nCg==", "devOptional": true, + "license": "MIT", "peerDependencies": { - "@types/react": "^18.0.0" + "@types/react": "^19.0.0" } }, "node_modules/@types/sinon": { @@ -3317,6 +3305,7 @@ "version": "3.1.3", "resolved": "https://registry.npmjs.org/csstype/-/csstype-3.1.3.tgz", "integrity": "sha512-M1uQkMl8rQK/szD0LNhtqxIPLpimGm8sOBwU7lLnCpSbTyY3yeU1Vc7l4KT5zT4s/yOxHH5O7tIuuLOCnLADRw==", + "devOptional": true, "peer": true }, "node_modules/custom-event": { @@ -7490,7 +7479,6 @@ "version": "24.7.0-alpha2", "license": "Apache-2.0", "dependencies": { - "@lit/react": "^1.0.6", "@vaadin/a11y-base": "24.7.0-alpha2", "@vaadin/accordion": "24.7.0-alpha2", "@vaadin/app-layout": "24.7.0-alpha2", @@ -7552,10 +7540,10 @@ "@vaadin/virtual-list": "24.7.0-alpha2" }, "peerDependencies": { - "@types/react": "^18.2.37 || ^19", - "@types/react-dom": "^18.2.15 || ^19", - "react": "^18.2.0 || ^19", - "react-dom": "^18.2.0 || ^19" + "@types/react": "^19", + "@types/react-dom": "^19", + "react": "^19", + "react-dom": "^19" }, "peerDependenciesMeta": { "@types/react": { @@ -7571,7 +7559,6 @@ "version": "24.7.0-alpha2", "license": "SEE LICENSE IN LICENSE", "dependencies": { - "@lit/react": "^1.0.6", "@vaadin/board": "24.7.0-alpha2", "@vaadin/charts": "24.7.0-alpha2", "@vaadin/cookie-consent": "24.7.0-alpha2", @@ -7583,10 +7570,10 @@ "@vaadin/rich-text-editor": "24.7.0-alpha2" }, "peerDependencies": { - "@types/react": "^18.2.37 || ^19", - "@types/react-dom": "^18.2.15 || ^19", - "react": "^18.2.0 || ^19", - "react-dom": "^18.2.0 || ^19" + "@types/react": "^19", + "@types/react-dom": "^19", + "react": "^19", + "react-dom": "^19" }, "peerDependenciesMeta": { "@types/react": { @@ -8137,12 +8124,6 @@ "resolved": "https://registry.npmjs.org/@lit-labs/ssr-dom-shim/-/ssr-dom-shim-1.2.1.tgz", "integrity": "sha512-wx4aBmgeGvFmOKucFKY+8VFJSYZxs9poN3SDNQFF6lT6NrQUnHiPB2PWz2sc4ieEcAaYYzN+1uWahEeTq2aRIQ==" }, - "@lit/react": { - "version": "1.0.6", - "resolved": "https://registry.npmjs.org/@lit/react/-/react-1.0.6.tgz", - "integrity": "sha512-QIss8MPh6qUoFJmuaF4dSHts3qCsA36S3HcOLiNPShxhgYPr4XJRnCBKPipk85sR9xr6TQrOcDMfexwbNdJHYA==", - "requires": {} - }, "@lit/reactive-element": { "version": "2.0.4", "resolved": "https://registry.npmjs.org/@lit/reactive-element/-/reactive-element-2.0.4.tgz", @@ -8569,26 +8550,20 @@ "undici-types": "~6.20.0" } }, - "@types/prop-types": { - "version": "15.7.14", - "resolved": "https://registry.npmjs.org/@types/prop-types/-/prop-types-15.7.14.tgz", - "integrity": "sha512-gNMvNH49DJ7OJYv+KAKn0Xp45p8PLl6zo2YnvDIbTd4J6MER2BmWN49TG7n9LvkyihINxeKW8+3bfS2yDC9dzQ==", - "peer": true - }, "@types/react": { - "version": "18.3.17", - "resolved": "https://registry.npmjs.org/@types/react/-/react-18.3.17.tgz", - "integrity": "sha512-opAQ5no6LqJNo9TqnxBKsgnkIYHozW9KSTlFVoSUJYh1Fl/sswkEoqIugRSm7tbh6pABtYjGAjW+GOS23j8qbw==", + "version": "19.0.2", + "resolved": "https://registry.npmjs.org/@types/react/-/react-19.0.2.tgz", + "integrity": "sha512-USU8ZI/xyKJwFTpjSVIrSeHBVAGagkHQKPNbxeWwql/vDmnTIBgx+TJnhFnj1NXgz8XfprU0egV2dROLGpsBEg==", + "devOptional": true, "peer": true, "requires": { - "@types/prop-types": "*", "csstype": "^3.0.2" } }, "@types/react-dom": { - "version": "18.3.5", - "resolved": "https://registry.npmjs.org/@types/react-dom/-/react-dom-18.3.5.tgz", - "integrity": "sha512-P4t6saawp+b/dFrUr2cvkVsfvPguwsxtH6dNIYRllMsefqFzkZk5UIjzyDOv5g1dXIPdG4Sp1yCR4Z6RCUsG/Q==", + "version": "19.0.2", + "resolved": "https://registry.npmjs.org/@types/react-dom/-/react-dom-19.0.2.tgz", + "integrity": "sha512-c1s+7TKFaDRRxr1TxccIX2u7sfCnc3RxkVyBIUA2lCpyqCF+QoAwQ/CBg7bsMdVwP120HEH143VQezKtef5nCg==", "devOptional": true, "requires": {} }, @@ -9385,7 +9360,6 @@ "@vaadin/react-components": { "version": "file:packages/react-components", "requires": { - "@lit/react": "^1.0.6", "@vaadin/a11y-base": "24.7.0-alpha2", "@vaadin/accordion": "24.7.0-alpha2", "@vaadin/app-layout": "24.7.0-alpha2", @@ -9450,7 +9424,6 @@ "@vaadin/react-components-pro": { "version": "file:packages/react-components-pro", "requires": { - "@lit/react": "^1.0.6", "@vaadin/board": "24.7.0-alpha2", "@vaadin/charts": "24.7.0-alpha2", "@vaadin/cookie-consent": "24.7.0-alpha2", @@ -10253,6 +10226,7 @@ "version": "3.1.3", "resolved": "https://registry.npmjs.org/csstype/-/csstype-3.1.3.tgz", "integrity": "sha512-M1uQkMl8rQK/szD0LNhtqxIPLpimGm8sOBwU7lLnCpSbTyY3yeU1Vc7l4KT5zT4s/yOxHH5O7tIuuLOCnLADRw==", + "devOptional": true, "peer": true }, "custom-event": { diff --git a/package.json b/package.json index aeeee14a..48a5362a 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "@types/karma-chrome-launcher": "^3.1.4", "@types/karma-mocha": "^1.3.4", "@types/mocha": "^10.0.10", - "@types/react-dom": "^18.3.3", + "@types/react-dom": "^19", "@types/sinon": "^17.0.3", "@vitejs/plugin-react": "^4.3.4", "chai-as-promised": "^8.0.1", diff --git a/packages/react-components-pro/package.json b/packages/react-components-pro/package.json index dbcb1690..85b933c2 100644 --- a/packages/react-components-pro/package.json +++ b/packages/react-components-pro/package.json @@ -25,7 +25,6 @@ "version": "tsx ../../scripts/package-json-version.ts" }, "dependencies": { - "@lit/react": "^1.0.6", "@vaadin/board": "24.7.0-alpha2", "@vaadin/charts": "24.7.0-alpha2", "@vaadin/cookie-consent": "24.7.0-alpha2", @@ -39,10 +38,10 @@ "author": "Vaadin Ltd.", "license": "SEE LICENSE IN LICENSE", "peerDependencies": { - "@types/react": "^18.2.37 || ^19", - "@types/react-dom": "^18.2.15 || ^19", - "react": "^18.2.0 || ^19", - "react-dom": "^18.2.0 || ^19" + "@types/react": "^19", + "@types/react-dom": "^19", + "react": "^19", + "react-dom": "^19" }, "peerDependenciesMeta": { "@types/react": { diff --git a/packages/react-components/package.json b/packages/react-components/package.json index 6c832ca8..2cf2963b 100644 --- a/packages/react-components/package.json +++ b/packages/react-components/package.json @@ -26,7 +26,6 @@ "version": "tsx ../../scripts/package-json-version.ts" }, "dependencies": { - "@lit/react": "^1.0.6", "@vaadin/a11y-base": "24.7.0-alpha2", "@vaadin/accordion": "24.7.0-alpha2", "@vaadin/app-layout": "24.7.0-alpha2", @@ -90,10 +89,10 @@ "author": "Vaadin Ltd.", "license": "Apache-2.0", "peerDependencies": { - "@types/react": "^18.2.37 || ^19", - "@types/react-dom": "^18.2.15 || ^19", - "react": "^18.2.0 || ^19", - "react-dom": "^18.2.0 || ^19" + "@types/react": "^19", + "@types/react-dom": "^19", + "react": "^19", + "react-dom": "^19" }, "peerDependenciesMeta": { "@types/react": { @@ -501,6 +500,10 @@ "./utils/createComponentWithOrderedProps.d.ts.map": "./utils/createComponentWithOrderedProps.d.ts.map", "./utils/createComponentWithOrderedProps.js": "./utils/createComponentWithOrderedProps.js", "./utils/createComponentWithOrderedProps.js.map": "./utils/createComponentWithOrderedProps.js.map", + "./utils/flushInMicrotask.d.ts": "./utils/flushInMicrotask.d.ts", + "./utils/flushInMicrotask.d.ts.map": "./utils/flushInMicrotask.d.ts.map", + "./utils/flushInMicrotask.js": "./utils/flushInMicrotask.js", + "./utils/flushInMicrotask.js.map": "./utils/flushInMicrotask.js.map", "./utils/flushMicrotask.d.ts": "./utils/flushMicrotask.d.ts", "./utils/flushMicrotask.d.ts.map": "./utils/flushMicrotask.d.ts.map", "./utils/flushMicrotask.js": "./utils/flushMicrotask.js", diff --git a/packages/react-components/src/Select.tsx b/packages/react-components/src/Select.tsx index 1a343c21..ec7439ea 100644 --- a/packages/react-components/src/Select.tsx +++ b/packages/react-components/src/Select.tsx @@ -2,6 +2,7 @@ import { type ComponentType, type ForwardedRef, forwardRef, + type HTMLAttributes, isValidElement, type ReactElement, type ReactNode, @@ -31,7 +32,7 @@ function Select(props: SelectProps, ref: ForwardedRef): ReactElem // Components with slot attribute should stay in light DOM. const slottedChildren = children.filter((child): child is ReactNode => { - return isValidElement(child) && child.props.slot; + return isValidElement>(child) && !!child.props.slot; }); // Component without slot attribute should go to the overlay. diff --git a/packages/react-components/src/renderers/useRenderer.ts b/packages/react-components/src/renderers/useRenderer.ts index 4f3e55bb..3d73f492 100644 --- a/packages/react-components/src/renderers/useRenderer.ts +++ b/packages/react-components/src/renderers/useRenderer.ts @@ -45,7 +45,7 @@ export function useRenderer

( convert?: (props: Slice, 1>) => PropsWithChildren

, config?: RendererConfig, ): UseRendererResult { - const [map, update] = useReducer>(rendererReducer, initialState); + const [map, update] = useReducer(rendererReducer, initialState); const renderer = useCallback( ((...args: Parameters) => { if (config?.renderMode === 'microtask') { diff --git a/packages/react-components/src/utils/createComponent.ts b/packages/react-components/src/utils/createComponent.ts index 74fe3659..2114e1bd 100644 --- a/packages/react-components/src/utils/createComponent.ts +++ b/packages/react-components/src/utils/createComponent.ts @@ -1,7 +1,12 @@ -import { createComponent as _createComponent, type EventName } from '@lit/react'; import type { ThemePropertyMixinClass } from '@vaadin/vaadin-themable-mixin/vaadin-theme-property-mixin.js'; import type React from 'react'; -import type { RefAttributes } from 'react'; +import { + createElement, + useLayoutEffect, + useRef, + type RefAttributes, +} from 'react'; +import useMergedRefs from './useMergedRefs.js'; declare const __VERSION__: string; @@ -28,8 +33,11 @@ window.Vaadin.registrations.push({ version: __VERSION__, }); -// TODO: Remove when types from @lit-labs/react are exported -export type EventNames = Record; +export type EventName = string & { + __eventType: T; +}; + +export type EventNames = Record; type Constructor = { new (): T; name: string }; type PolymerConstructor = Constructor & { _properties: Record }; type Options = Readonly<{ @@ -79,30 +87,86 @@ type AllWebComponentProps = I export type WebComponentProps = Partial>; -// We need a separate declaration here; otherwise, the TypeScript fails into the -// endless loop trying to resolve the typings. +const listenedEvents = new WeakMap>(); + +function addOrUpdateEventListener(node: Element, event: string, listener: ((event: Event) => void) | undefined) { + let events = listenedEvents.get(node); + if (events === undefined) { + listenedEvents.set(node, (events = new Map())); + } + let handler = events.get(event); + if (listener !== undefined) { + // If necessary, add listener and track handler + if (handler === undefined) { + events.set(event, (handler = { handleEvent: listener })); + node.addEventListener(event, handler); + // Otherwise just update the listener with new value + } else { + handler.handleEvent = listener; + } + // Remove listener if one exists and value is undefined + } else if (handler !== undefined) { + events.delete(event); + node.removeEventListener(event, handler); + } +} + export function createComponent( options: Options, -): (props: WebComponentProps & RefAttributes) => React.ReactElement | null; - -export function createComponent(options: Options): any { - const { elementClass } = options; - - return _createComponent( - '_properties' in elementClass - ? { - ...options, - // TODO: improve or remove the Polymer workaround - // 'createComponent' relies on key presence on the custom element class, - // but Polymer defines properties on the prototype when the first element - // is created. Workaround: pass a mock object with properties in - // the prototype. - elementClass: { - // @ts-expect-error: it is a specific workaround for Polymer classes. - name: elementClass.name, - prototype: { ...elementClass._properties, hidden: Boolean }, - }, +): (props: WebComponentProps & RefAttributes) => React.ReactElement { + const { tagName, events: eventsMap, elementClass } = options; + + return (props) => { + const innerRef = useRef(null); + const finalRef = useMergedRefs(innerRef, props.ref); + const prevEventsRef = useRef(new Set()); + + // Option 1 (Lit React's approach – no initial property events): + useLayoutEffect(() => { + if (eventsMap) { + const events = new Set(Object.keys(props).filter((event) => eventsMap[event])); + events.forEach((event) => { + addOrUpdateEventListener(innerRef.current!, eventsMap[event], props[event]); + }); + + prevEventsRef.current.forEach((event) => { + if (!events.has(event)) { + addOrUpdateEventListener(innerRef.current!, eventsMap[event], undefined); + } + }); + + prevEventsRef.current = events; + } + }); + + useLayoutEffect(() => { + return () => { + if (eventsMap) { + prevEventsRef.current.forEach((event) => { + addOrUpdateEventListener(innerRef.current!, eventsMap[event], undefined); + }); } - : options, - ); + } + }, []); + + const elementProps = Object.entries(props).reduce((acc, [key, value]) => { + // Filter out event handlers from the props + if (eventsMap?.[key]) { + return acc; + } + + return { ...acc, [key]: value }; + }, {} as typeof props); + + // Option 2 (React's approach – initial property events are fired): + // props = Object.entries(props).reduce((acc, [key, value]) => { + // if (eventsMap?.[key]) { + // key = `on${eventsMap[key]}`; + // } + + // return { ...acc, [key]: value }; + // }, {}); + + return createElement(tagName, { ...elementProps, ref: finalRef }); + }; } diff --git a/packages/react-components/src/utils/useMergedRefs.ts b/packages/react-components/src/utils/useMergedRefs.ts index 3a02cc50..affa9f59 100644 --- a/packages/react-components/src/utils/useMergedRefs.ts +++ b/packages/react-components/src/utils/useMergedRefs.ts @@ -1,6 +1,6 @@ -import { type ForwardedRef, type RefCallback, useCallback } from 'react'; +import { type Ref, type RefCallback, useCallback } from 'react'; -export default function useMergedRefs(...refs: ReadonlyArray>): RefCallback { +export default function useMergedRefs(...refs: ReadonlyArray | undefined>): RefCallback { return useCallback((element: T) => { refs.forEach((ref) => { if (typeof ref === 'function') { diff --git a/scripts/generator.ts b/scripts/generator.ts index 4cc20581..14922667 100644 --- a/scripts/generator.ts +++ b/scripts/generator.ts @@ -35,7 +35,6 @@ const CREATE_COMPONENT_PATH = '$CREATE_COMPONENT_PATH$'; const EVENT_MAP = '$EVENT_MAP$'; const EVENT_MAP_REF_IN_EVENTS = '$EVENT_MAP_REF_IN_EVENTS$'; const EVENTS_DECLARATION = '$EVENTS_DECLARATION$'; -const LIT_REACT_PATH = '@lit/react'; const MODULE_PATH = '$MODULE_PATH$'; type ElementData = Readonly<{ @@ -321,14 +320,13 @@ function generateReactComponent({ name, js }: SchemaHTMLElement, { packageName, const ast = template( ` -import type { EventName } from "${LIT_REACT_PATH}"; import { ${COMPONENT_NAME} as ${COMPONENT_NAME}Element type ${COMPONENT_NAME}EventMap as _${COMPONENT_NAME}EventMap, ${[...new Set(genericElementInfo?.typeConstraints || [])].map((constraint) => `type ${constraint}`)} } from "${MODULE_PATH}"; import * as React from "react"; -import { createComponent, type WebComponentProps } from "${CREATE_COMPONENT_PATH}"; +import { createComponent, type WebComponentProps, type EventName } from "${CREATE_COMPONENT_PATH}"; export * from "${MODULE_PATH}"; diff --git a/test/Select.spec.tsx b/test/Select.spec.tsx index 29a20d7b..a717eaa4 100644 --- a/test/Select.spec.tsx +++ b/test/Select.spec.tsx @@ -146,15 +146,15 @@ describe('Select', () => { describe('boolean property', () => { const booleanProperties: Array = [ - 'disabled', - 'hidden', + // 'disabled', + // 'hidden', 'opened', - 'draggable', + // 'draggable', ]; booleanProperties.forEach((property) => { describe(property, () => { - it(`should be true in the element if ${property} prop is true`, async () => { + it.only(`should be true in the element if ${property} prop is true`, async () => { render( console.log(e.detail)} - > - - ); -} diff --git a/packages/react-components/src/utils/createComponent.ts b/packages/react-components/src/utils/createComponent.ts index 2114e1bd..1253fe20 100644 --- a/packages/react-components/src/utils/createComponent.ts +++ b/packages/react-components/src/utils/createComponent.ts @@ -114,7 +114,7 @@ function addOrUpdateEventListener(node: Element, event: string, listener: ((even export function createComponent( options: Options, ): (props: WebComponentProps & RefAttributes) => React.ReactElement { - const { tagName, events: eventsMap, elementClass } = options; + const { tagName, events: eventsMap } = options; return (props) => { const innerRef = useRef(null); diff --git a/test/Select.spec.tsx b/test/Select.spec.tsx index a717eaa4..88387330 100644 --- a/test/Select.spec.tsx +++ b/test/Select.spec.tsx @@ -146,24 +146,24 @@ describe('Select', () => { describe('boolean property', () => { const booleanProperties: Array = [ - // 'disabled', - // 'hidden', + 'disabled', + 'hidden', 'opened', - // 'draggable', + 'draggable', ]; booleanProperties.forEach((property) => { describe(property, () => { - it.only(`should be true in the element if ${property} prop is true`, async () => { + it(`should be true in the element if ${property} prop is true`, async () => { render(); const select = await findByQuerySelector('vaadin-select'); - expect(select[property]).not.to.be.ok; + expect(select[property]).to.be.false; }); }); }); From 11a10d07c98910f2043df93bfaa3b714107d2c35 Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Wed, 25 Dec 2024 15:56:22 +0400 Subject: [PATCH 3/4] fix format errors, simplify implementation --- .../src/utils/createComponent.ts | 45 ++++++++----------- .../src/utils/useMergedRefs.ts | 4 +- 2 files changed, 21 insertions(+), 28 deletions(-) diff --git a/packages/react-components/src/utils/createComponent.ts b/packages/react-components/src/utils/createComponent.ts index 1253fe20..eed7fdce 100644 --- a/packages/react-components/src/utils/createComponent.ts +++ b/packages/react-components/src/utils/createComponent.ts @@ -1,11 +1,6 @@ import type { ThemePropertyMixinClass } from '@vaadin/vaadin-themable-mixin/vaadin-theme-property-mixin.js'; import type React from 'react'; -import { - createElement, - useLayoutEffect, - useRef, - type RefAttributes, -} from 'react'; +import { createElement, useLayoutEffect, useRef, type RefAttributes } from 'react'; import useMergedRefs from './useMergedRefs.js'; declare const __VERSION__: string; @@ -121,7 +116,7 @@ export function createComponent()); - // Option 1 (Lit React's approach – no initial property events): + // Option 1 (no initial property events): useLayoutEffect(() => { if (eventsMap) { const events = new Set(Object.keys(props).filter((event) => eventsMap[event])); @@ -146,27 +141,23 @@ export function createComponent { - // Filter out event handlers from the props - if (eventsMap?.[key]) { - return acc; - } - - return { ...acc, [key]: value }; - }, {} as typeof props); - - // Option 2 (React's approach – initial property events are fired): - // props = Object.entries(props).reduce((acc, [key, value]) => { - // if (eventsMap?.[key]) { - // key = `on${eventsMap[key]}`; - // } - - // return { ...acc, [key]: value }; - // }, {}); - - return createElement(tagName, { ...elementProps, ref: finalRef }); + const finalProps = Object.fromEntries( + Object.entries(props).filter(([key]) => !eventsMap?.[key]) + ); + + // Option 2 (initial property events are fired): + // const finalProps = Object.fromEntries( + // Object.entries(props).map(([key, value]) => { + // if (eventsMap?.[key]) { + // return [`on${eventsMap[key]}`, value]; + // } + // return [key, value]; + // }) + // ); + + return createElement(tagName, { ...finalProps, ref: finalRef }); }; } diff --git a/packages/react-components/src/utils/useMergedRefs.ts b/packages/react-components/src/utils/useMergedRefs.ts index affa9f59..dea3dc53 100644 --- a/packages/react-components/src/utils/useMergedRefs.ts +++ b/packages/react-components/src/utils/useMergedRefs.ts @@ -1,6 +1,8 @@ import { type Ref, type RefCallback, useCallback } from 'react'; -export default function useMergedRefs(...refs: ReadonlyArray | undefined>): RefCallback { +export default function useMergedRefs( + ...refs: ReadonlyArray | undefined> +): RefCallback { return useCallback((element: T) => { refs.forEach((ref) => { if (typeof ref === 'function') { From a9716e6b3ad3f9acab68157d1e985e4e0f2bea9c Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Wed, 25 Dec 2024 16:04:31 +0400 Subject: [PATCH 4/4] extract addOrUpdateEventListener to util --- .../src/utils/addOrUpdateEventListener.ts | 30 +++++++++++++++++++ .../src/utils/createComponent.ts | 29 ++---------------- 2 files changed, 32 insertions(+), 27 deletions(-) create mode 100644 packages/react-components/src/utils/addOrUpdateEventListener.ts diff --git a/packages/react-components/src/utils/addOrUpdateEventListener.ts b/packages/react-components/src/utils/addOrUpdateEventListener.ts new file mode 100644 index 00000000..c72e9538 --- /dev/null +++ b/packages/react-components/src/utils/addOrUpdateEventListener.ts @@ -0,0 +1,30 @@ +const listenedEvents = new WeakMap>(); + +export default function addOrUpdateEventListener( + node: Element, + event: string, + listener: ((event: Event) => void) | undefined, +) { + let nodeEvents = listenedEvents.get(node); + if (nodeEvents === undefined) { + nodeEvents = new Map(); + listenedEvents.set(node, nodeEvents); + } + + let handler = nodeEvents.get(event); + if (listener !== undefined) { + if (handler === undefined) { + // If necessary, add listener and track handler + handler = { handleEvent: listener }; + node.addEventListener(event, handler); + nodeEvents.set(event, handler); + } else { + // Otherwise just update the listener with new value + handler.handleEvent = listener; + } + } else if (handler !== undefined) { + // Remove listener if one exists and value is undefined + node.removeEventListener(event, handler); + nodeEvents.delete(event); + } +} diff --git a/packages/react-components/src/utils/createComponent.ts b/packages/react-components/src/utils/createComponent.ts index eed7fdce..20caf940 100644 --- a/packages/react-components/src/utils/createComponent.ts +++ b/packages/react-components/src/utils/createComponent.ts @@ -2,6 +2,7 @@ import type { ThemePropertyMixinClass } from '@vaadin/vaadin-themable-mixin/vaad import type React from 'react'; import { createElement, useLayoutEffect, useRef, type RefAttributes } from 'react'; import useMergedRefs from './useMergedRefs.js'; +import addOrUpdateEventListener from './addOrUpdateEventListener.js'; declare const __VERSION__: string; @@ -82,30 +83,6 @@ type AllWebComponentProps = I export type WebComponentProps = Partial>; -const listenedEvents = new WeakMap>(); - -function addOrUpdateEventListener(node: Element, event: string, listener: ((event: Event) => void) | undefined) { - let events = listenedEvents.get(node); - if (events === undefined) { - listenedEvents.set(node, (events = new Map())); - } - let handler = events.get(event); - if (listener !== undefined) { - // If necessary, add listener and track handler - if (handler === undefined) { - events.set(event, (handler = { handleEvent: listener })); - node.addEventListener(event, handler); - // Otherwise just update the listener with new value - } else { - handler.handleEvent = listener; - } - // Remove listener if one exists and value is undefined - } else if (handler !== undefined) { - events.delete(event); - node.removeEventListener(event, handler); - } -} - export function createComponent( options: Options, ): (props: WebComponentProps & RefAttributes) => React.ReactElement { @@ -144,9 +121,7 @@ export function createComponent !eventsMap?.[key]) - ); + const finalProps = Object.fromEntries(Object.entries(props).filter(([key]) => !eventsMap?.[key])); // Option 2 (initial property events are fired): // const finalProps = Object.fromEntries(