fix: normalize html attributes only when not defined by component (#5215)

Proper fix for #5211

Now props meta are available in component generator and can be used to
figure if prop is defined by component or comes from html attributes.
Added same logic in canvas and component generator.

Also got rid from no longer necessary registeredComponentPropsMetas
store since all props are described by component meta.

Author: TrySound

apps/builder/app/builder/features/settings-panel/props-section/props-section.stories.tsx

@@ -199,10 +199,10 @@ const checkProp = (options = defaultOptions, label?: string): PropMeta => ({
 
 registerComponentLibrary({
   components: {},
-  metas: {},
   templates: {},
-  propsMetas: {
+  metas: {
     Box: {
+      icon: "",
       props: {
         initialText: textProp("", "multi\nline"),
         initialShortText: shortTextProp(),

apps/builder/app/builder/features/settings-panel/shared.tsx

@@ -39,7 +39,7 @@ import {
 import {
   $dataSourceVariables,
   $dataSources,
-  $registeredComponentPropsMetas,
+  $registeredComponentMetas,
   $variableValuesByInstanceSelector,
 } from "~/shared/nano-states";
 import type { BindingVariant } from "~/builder/shared/binding-popover";
@@ -455,62 +455,60 @@ const attributeToMeta = (attribute: Attribute): PropMeta => {
 };
 
 export const $selectedInstancePropsMetas = computed(
-  [$selectedInstance, $registeredComponentPropsMetas, $instanceTags],
-  (instance, componentPropsMetas, instanceTags): Map<string, PropMeta> => {
+  [$selectedInstance, $registeredComponentMetas, $instanceTags],
+  (instance, metas, instanceTags): Map<string, PropMeta> => {
     if (instance === undefined) {
       return new Map();
     }
-    const propsMetas = componentPropsMetas.get(instance.component)?.props ?? {};
+    const meta = metas.get(instance.component);
     const tag = instanceTags.get(instance.id);
-    const metas = new Map<Prop["name"], PropMeta>();
+    const propsMetas = new Map<Prop["name"], PropMeta>();
     // add html attributes only when instance has tag
     if (tag) {
       for (const attribute of [...ariaAttributes].reverse()) {
-        metas.set(attribute.name, attributeToMeta(attribute));
+        propsMetas.set(attribute.name, attributeToMeta(attribute));
       }
       if (attributesByTag["*"]) {
         for (const attribute of [...attributesByTag["*"]].reverse()) {
-          metas.set(attribute.name, attributeToMeta(attribute));
+          propsMetas.set(attribute.name, attributeToMeta(attribute));
         }
       }
       if (attributesByTag[tag]) {
         for (const attribute of [...attributesByTag[tag]].reverse()) {
-          metas.set(attribute.name, attributeToMeta(attribute));
+          propsMetas.set(attribute.name, attributeToMeta(attribute));
         }
       }
     }
-    for (const [name, propMeta] of Object.entries(propsMetas).reverse()) {
+    for (const [name, propMeta] of Object.entries(
+      meta?.props ?? {}
+    ).reverse()) {
       // when component property has the same name as html attribute in react
       // override to deduplicate similar properties
       // for example component can have own "className" and html has "class"
       const htmlName = reactPropsToStandardAttributes[name];
       if (htmlName) {
-        metas.delete(htmlName);
+        propsMetas.delete(htmlName);
       }
-      metas.set(name, propMeta);
+      propsMetas.set(name, propMeta);
     }
-    metas.set(showAttribute, showAttributeMeta);
+    propsMetas.set(showAttribute, showAttributeMeta);
     // ui should render in the following order
     // 1. system properties
     // 2. component properties
     // 3. specific tag attributes
     // 4. global html attributes
     // 5. aria attributes
-    return new Map(Array.from(metas.entries()).reverse());
+    return new Map(Array.from(propsMetas.entries()).reverse());
   }
 );
 
 export const $selectedInstanceInitialPropNames = computed(
-  [
-    $selectedInstance,
-    $registeredComponentPropsMetas,
-    $selectedInstancePropsMetas,
-  ],
-  (selectedInstance, componentPropsMetas, instancePropsMetas) => {
+  [$selectedInstance, $registeredComponentMetas, $selectedInstancePropsMetas],
+  (selectedInstance, metas, instancePropsMetas) => {
     const initialPropNames = new Set<string>();
     if (selectedInstance) {
       const initialProps =
-        componentPropsMetas.get(selectedInstance.component)?.initialProps ?? [];
+        metas.get(selectedInstance.component)?.initialProps ?? [];
       for (const propName of initialProps) {
         // className -> class
         if (instancePropsMetas.has(reactPropsToStandardAttributes[propName])) {

apps/builder/app/canvas/canvas.tsx

@@ -1,7 +1,7 @@
 import { useMemo, useEffect, useState, useLayoutEffect, useRef } from "react";
 import { ErrorBoundary, type FallbackProps } from "react-error-boundary";
 import { useStore } from "@nanostores/react";
-import { type Instances, coreMetas, corePropsMetas } from "@webstudio-is/sdk";
+import { type Instances, coreMetas } from "@webstudio-is/sdk";
 import { coreTemplates } from "@webstudio-is/sdk/core-templates";
 import type { Components } from "@webstudio-is/react-sdk";
 import { wsImageLoader } from "@webstudio-is/image";
@@ -240,13 +240,11 @@ export const Canvas = () => {
     registerComponentLibrary({
       components: {},
       metas: coreMetas,
-      propsMetas: corePropsMetas,
       templates: coreTemplates,
     });
     registerComponentLibrary({
       components: baseComponents,
       metas: baseComponentMetas,
-      propsMetas: {},
       hooks: baseComponentHooks,
       templates: baseComponentTemplates,
     });
@@ -257,22 +255,19 @@ export const Canvas = () => {
         Body,
       },
       metas: {},
-      propsMetas: {},
       templates: {},
     });
     registerComponentLibrary({
       namespace: "@webstudio-is/sdk-components-react-radix",
       components: radixComponents,
       metas: radixComponentMetas,
-      propsMetas: {},
       hooks: radixComponentHooks,
       templates: radixTemplates,
     });
     registerComponentLibrary({
       namespace: "@webstudio-is/sdk-components-animation",
       components: animationComponents,
       metas: animationComponentMetas,
-      propsMetas: {},
       hooks: animationComponentHooks,
       templates: animationTemplates,
     });

apps/builder/app/canvas/features/webstudio-component/webstudio-component.tsx

@@ -301,22 +301,22 @@ const useInstanceProps = (instanceSelector: InstanceSelector) => {
         if (tag !== undefined) {
           instancePropsObject[tagProperty] = tag;
         }
-        const hasTags =
-          Object.keys(metas.get(instance?.component ?? "")?.presetStyle ?? {})
-            .length > 0;
+        const meta = metas.get(instance?.component ?? "");
+        const hasTags = Object.keys(meta?.presetStyle ?? {}).length > 0;
         const index = indexesWithinAncestors.get(instanceId);
         if (index !== undefined) {
           instancePropsObject[indexProperty] = index.toString();
         }
         const instanceProps = propValuesByInstanceSelector.get(instanceKey);
         if (instanceProps) {
           for (const [name, value] of instanceProps) {
-            if (hasTags) {
-              const reactName = standardAttributesToReactProps[name] ?? name;
-              instancePropsObject[reactName] = value;
-            } else {
-              instancePropsObject[name] = value;
+            let propName = name;
+            // convert html attribute only when component has tags
+            // and does not specify own property with this name
+            if (hasTags && !meta?.props?.[propName]) {
+              propName = standardAttributesToReactProps[propName] ?? propName;
             }
+            instancePropsObject[propName] = value;
           }
         }
         return instancePropsObject;

apps/builder/app/shared/nano-states/components.ts

@@ -12,7 +12,6 @@ import {
   getIndexesWithinAncestors,
   type Instance,
   type WsComponentMeta,
-  type WsComponentPropsMeta,
 } from "@webstudio-is/sdk";
 import type { InstanceSelector } from "../tree-utils";
 import { $memoryProps, $props } from "./misc";
@@ -176,15 +175,10 @@ export const $registeredTemplates = atom(
   new Map<string, GeneratedTemplateMeta>()
 );
 
-export const $registeredComponentPropsMetas = atom(
-  new Map<string, WsComponentPropsMeta>()
-);
-
 export const registerComponentLibrary = ({
   namespace,
   components,
   metas,
-  propsMetas,
   hooks,
   templates,
 }: {
@@ -193,7 +187,6 @@ export const registerComponentLibrary = ({
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   components: Record<Instance["component"], ExoticComponent<any>>;
   metas: Record<Instance["component"], WsComponentMeta>;
-  propsMetas: Record<Instance["component"], WsComponentPropsMeta>;
   hooks?: Hook[];
   templates: Record<Instance["component"], TemplateMeta>;
 }) => {
@@ -206,22 +199,10 @@ export const registerComponentLibrary = ({
   }
   $registeredComponents.set(nextComponents);
 
-  const prevPropsMetas = $registeredComponentPropsMetas.get();
-  const nextPropsMetas = new Map(prevPropsMetas);
-  for (const [componentName, propsMeta] of Object.entries(propsMetas)) {
-    nextPropsMetas.set(`${prefix}${componentName}`, propsMeta);
-  }
-
   const prevMetas = $registeredComponentMetas.get();
   const nextMetas = new Map(prevMetas);
   for (const [componentName, meta] of Object.entries(metas)) {
     nextMetas.set(`${prefix}${componentName}`, meta);
-    if (meta.initialProps || meta.props) {
-      nextPropsMetas.set(`${prefix}${componentName}`, {
-        initialProps: meta.initialProps,
-        props: meta.props ?? {},
-      });
-    }
   }
   $registeredComponentMetas.set(nextMetas);
 
@@ -241,6 +222,4 @@ export const registerComponentLibrary = ({
     const nextHooks = [...prevHooks, ...hooks];
     $registeredComponentHooks.set(nextHooks);
   }
-
-  $registeredComponentPropsMetas.set(nextPropsMetas);
 };

apps/builder/app/shared/sync/sync-stores.ts

@@ -43,7 +43,6 @@ import {
   $blockChildOutline,
   $textToolbar,
   $registeredComponentMetas,
-  $registeredComponentPropsMetas,
   $registeredTemplates,
   $modifierKeys,
 } from "~/shared/nano-states";
@@ -147,10 +146,6 @@ export const createObjectPool = () => {
       "registeredComponentMetas",
       $registeredComponentMetas
     ),
-    new NanostoresSyncObject(
-      "registeredComponentPropsMetas",
-      $registeredComponentPropsMetas
-    ),
     new NanostoresSyncObject("registeredTemplates", $registeredTemplates),
     new NanostoresSyncObject("canvasScrollbarWidth", $canvasScrollbarSize),
     new NanostoresSyncObject("systemDataByPage", $systemDataByPage),

packages/react-sdk/src/component-generator.test.tsx

@@ -3,6 +3,7 @@ import { expect, test } from "vitest";
 import stripIndent from "strip-indent";
 import {
   createScope,
+  elementComponent,
   ROOT_INSTANCE_ID,
   SYSTEM_VARIABLE_ID,
   WsComponentMeta,
@@ -1386,7 +1387,9 @@ test("convert attributes to react compatible when render ws:element", () => {
       name: "Page",
       rootInstanceId: "bodyId",
       parameters: [],
-      metas: new Map(),
+      metas: new Map([
+        [elementComponent, { icon: "", presetStyle: { div: [] } }],
+      ]),
       ...renderData(
         <$.Body ws:id="bodyId">
           <ws.element
@@ -1444,6 +1447,46 @@ test("convert attributes to react compatible when render components with tags",
   );
 });
 
+test("ignore props similar to standard attributes when react components defines them", () => {
+  expect(
+    generateWebstudioComponent({
+      classesMap: new Map(),
+      scope: createScope(),
+      name: "Page",
+      rootInstanceId: "bodyId",
+      parameters: [],
+      metas: new Map([
+        [
+          "Vimeo",
+          {
+            icon: "",
+            presetStyle: { div: [] },
+            props: {
+              autoplay: { type: "boolean", control: "boolean", required: true },
+            },
+          },
+        ],
+      ]),
+      ...renderData(
+        <$.Body ws:id="bodyId">
+          <$.Vimeo autoplay={true}></$.Vimeo>
+        </$.Body>
+      ),
+    })
+  ).toEqual(
+    validateJSX(
+      clear(`
+       const Page = () => {
+       return <Body>
+       <Vimeo
+       autoplay={true} />
+       </Body>
+       }
+     `)
+    )
+  );
+});
+
 test("ignore props similar to standard attributes on react components without tags", () => {
   expect(
     generateWebstudioComponent({

packages/react-sdk/src/component-generator.ts

@@ -170,8 +170,8 @@ export const generateJsxElement = ({
     return "";
   }
 
-  const hasTags =
-    Object.keys(metas.get(instance.component)?.presetStyle ?? {}).length > 0;
+  const meta = metas.get(instance.component);
+  const hasTags = Object.keys(meta?.presetStyle ?? {}).length > 0;
 
   let generatedProps = "";
 
@@ -204,7 +204,9 @@ export const generateJsxElement = ({
       continue;
     }
     let name = prop.name;
-    if (instance.component === elementComponent || hasTags) {
+    // convert html attribute only when component has tags
+    // and does not specify own property with this name
+    if (hasTags && !meta?.props?.[prop.name]) {
       name = standardAttributesToReactProps[prop.name] ?? prop.name;
     }