fix: Skip unsafe attribute names on generation (#4216)

## Description

We can also do not skip but 
do `<Dom {...{"1unsafe": value}} />` 
BTW that looks useless.

closes #3573
closes
webstudio-is/webstudio-community#190 (comment)

## Steps for reproduction

1. click button
2. expect xyz

## Code Review

- [ ] hi @kof, I need you to do
  - conceptual review (architecture, feature-correctness)
  - detailed review (read every line)
  - test it on preview

## Before requesting a review

- [ ] made a self-review
- [ ] added inline comments where things may be not obvious (the "why",
not "what")

## Before merging

- [ ] tested locally and on preview environment (preview dev login:
5de6)
- [ ] updated [test
cases](https://github.com/webstudio-is/webstudio/blob/main/apps/builder/docs/test-cases.md)
document
- [ ] added tests
- [ ] if any new env variables are added, added them to `.env` file

Author: istarkov

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

@@ -9,7 +9,10 @@ import {
   Flex,
   Box,
 } from "@webstudio-is/design-system";
-import { descendantComponent } from "@webstudio-is/react-sdk";
+import {
+  descendantComponent,
+  isAttributeNameSafe,
+} from "@webstudio-is/react-sdk";
 import {
   $propValuesByInstanceSelector,
   $propsIndex,
@@ -21,7 +24,6 @@ import { renderControl } from "../controls/combined";
 import { usePropsLogic, type PropAndMeta } from "./use-props-logic";
 import { Row } from "../shared";
 import { serverSyncStore } from "~/shared/sync";
-import { isAttributeNameSafe } from "~/shared/dom-utils";
 
 type Item = {
   name: string;

apps/builder/app/shared/dom-utils.ts

@@ -192,39 +192,3 @@ export const getAllElementsBoundingBox = (elements: Element[]): DOMRect => {
     width: right - left,
   });
 };
-
-/**
- * Copyright (c) Meta Platforms, Inc. and affiliates.
- *
- * This source code is licensed under the MIT license found in the
- * LICENSE file in the root directory of this source tree.
- *
- * https://github.com/facebook/react/blob/main/packages/react-dom-bindings/src/shared/isAttributeNameSafe.js
- */
-const attributeNameStartChar =
-  ":A-Z_a-z\\u00C0-\\u00D6\\u00D8-\\u00F6\\u00F8-\\u02FF\\u0370-\\u037D\\u037F-\\u1FFF\\u200C-\\u200D\\u2070-\\u218F\\u2C00-\\u2FEF\\u3001-\\uD7FF\\uF900-\\uFDCF\\uFDF0-\\uFFFD";
-const attributeNameChar =
-  attributeNameStartChar + "\\-.0-9\\u00B7\\u0300-\\u036F\\u203F-\\u2040";
-
-const validAttributeNameRegex = new RegExp(
-  // eslint-disable-next-line no-misleading-character-class
-  "^[" + attributeNameStartChar + "][" + attributeNameChar + "]*$"
-);
-
-const illegalAttributeNameCache = new Map<string, boolean>();
-const validatedAttributeNameCache = new Map<string, boolean>();
-
-export const isAttributeNameSafe = (attributeName: string) => {
-  if (validatedAttributeNameCache.has(attributeName)) {
-    return true;
-  }
-  if (illegalAttributeNameCache.has(attributeName)) {
-    return false;
-  }
-  if (validAttributeNameRegex.test(attributeName)) {
-    validatedAttributeNameCache.set(attributeName, true);
-    return true;
-  }
-  illegalAttributeNameCache.set(attributeName, true);
-  return false;
-};

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

@@ -948,3 +948,54 @@ test("generate resource prop", () => {
     "
   `);
 });
+
+test("skip unsafe properties", () => {
+  expect(
+    generateWebstudioComponent({
+      classesMap: new Map(),
+      scope: createScope(),
+      name: "Page",
+      rootInstanceId: "body",
+      parameters: [],
+      dataSources: new Map(),
+      indexesWithinAncestors: new Map(),
+
+      instances: toMap([
+        { type: "instance", id: "body", component: "Body", children: [] },
+      ]),
+      props: toMap([
+        {
+          id: "unsafeProp",
+          instanceId: "body",
+          name: "",
+          type: "string",
+          value: "unsafe",
+        },
+
+        {
+          id: "unsafeProp-2",
+          instanceId: "body",
+          name: "1-numeric-unsafe",
+          type: "string",
+          value: "unsafe",
+        },
+
+        {
+          id: "unsafeProp-3",
+          instanceId: "body",
+          name: "click.prevent",
+          type: "string",
+          value: "unsafe",
+        },
+      ]),
+    })
+  ).toEqual(
+    validateJSX(
+      clear(`
+        const Page = () => {
+        return <Body />
+        }
+    `)
+    )
+  );
+});

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

@@ -13,7 +13,7 @@ import {
   decodeDataSourceVariable,
   transpileExpression,
 } from "@webstudio-is/sdk";
-import { indexAttribute, showAttribute } from "./props";
+import { indexAttribute, isAttributeNameSafe, showAttribute } from "./props";
 import { collectionComponent, descendantComponent } from "./core-components";
 import type { IndexesWithinAncestors } from "./instance-utils";
 
@@ -181,6 +181,11 @@ export const generateJsxElement = ({
       dataSources,
       usedDataSources,
     });
+
+    if (isAttributeNameSafe(prop.name) === false) {
+      continue;
+    }
+
     // show prop controls conditional rendering and need to be handled separately
     if (prop.name === showAttribute) {
       // prevent generating unnecessary condition

packages/react-sdk/src/props.test.ts

@@ -1,6 +1,6 @@
-import { test, expect } from "@jest/globals";
+import { test, expect, describe } from "@jest/globals";
 import type { Pages, Prop } from "@webstudio-is/sdk";
-import { normalizeProps } from "./props";
+import { isAttributeNameSafe, normalizeProps } from "./props";
 
 const pagesBase: Pages = {
   meta: {},
@@ -289,3 +289,30 @@ test("normalize page prop with path and hash into string", () => {
     idProp,
   ]);
 });
+
+describe("isAttributeNameSafe", () => {
+  test("should return true for valid attribute names", () => {
+    expect(isAttributeNameSafe("data-test")).toBe(true);
+    expect(isAttributeNameSafe("aria-label")).toBe(true);
+    expect(isAttributeNameSafe("class")).toBe(true);
+    expect(isAttributeNameSafe("ns:class")).toBe(true);
+  });
+
+  test("should return false for invalid attribute names", () => {
+    expect(isAttributeNameSafe("123class")).toBe(false);
+    expect(isAttributeNameSafe("class.name")).toBe(false);
+    expect(isAttributeNameSafe(":bad")).toBe(false);
+    expect(isAttributeNameSafe(" ")).toBe(false);
+    expect(isAttributeNameSafe("hello world")).toBe(false);
+  });
+
+  test("should return true for cached valid attribute names", () => {
+    isAttributeNameSafe("data-cached");
+    expect(isAttributeNameSafe("data-cached")).toBe(true);
+  });
+
+  test("should return false for cached invalid attribute names", () => {
+    isAttributeNameSafe("1-invalid-cached");
+    expect(isAttributeNameSafe("1-invalid-cached")).toBe(false);
+  });
+});

packages/react-sdk/src/props.ts

@@ -129,3 +129,41 @@ export const showAttribute = "data-ws-show" as const;
 export const indexAttribute = "data-ws-index" as const;
 export const collapsedAttribute = "data-ws-collapsed" as const;
 export const textContentAttribute = "data-ws-text-content" as const;
+
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * https://github.com/facebook/react/blob/main/packages/react-dom-bindings/src/shared/isAttributeNameSafe.js
+ */
+const attributeNameStartChar =
+  "A-Z_a-z\\u00C0-\\u00D6\\u00D8-\\u00F6\\u00F8-\\u02FF\\u0370-\\u037D\\u037F-\\u1FFF\\u200C-\\u200D\\u2070-\\u218F\\u2C00-\\u2FEF\\u3001-\\uD7FF\\uF900-\\uFDCF\\uFDF0-\\uFFFD";
+// original: ":A-Z_a-z\\u00C0-\\u00D6\\u00D8-\\u00F6\\u00F8-\\u02FF\\u0370-\\u037D\\u037F-\\u1FFF\\u200C-\\u200D\\u2070-\\u218F\\u2C00-\\u2FEF\\u3001-\\uD7FF\\uF900-\\uFDCF\\uFDF0-\\uFFFD";
+const attributeNameChar =
+  attributeNameStartChar + ":\\-0-9\\u00B7\\u0300-\\u036F\\u203F-\\u2040";
+// original: attributeNameStartChar + "\\-.0-9\\u00B7\\u0300-\\u036F\\u203F-\\u2040";
+
+const validAttributeNameRegex = new RegExp(
+  // eslint-disable-next-line no-misleading-character-class
+  "^[" + attributeNameStartChar + "][" + attributeNameChar + "]*$"
+);
+
+const illegalAttributeNameCache = new Map<string, boolean>();
+const validatedAttributeNameCache = new Map<string, boolean>();
+
+export const isAttributeNameSafe = (attributeName: string) => {
+  if (validatedAttributeNameCache.has(attributeName)) {
+    return true;
+  }
+  if (illegalAttributeNameCache.has(attributeName)) {
+    return false;
+  }
+  if (validAttributeNameRegex.test(attributeName)) {
+    validatedAttributeNameCache.set(attributeName, true);
+    return true;
+  }
+  illegalAttributeNameCache.set(attributeName, true);
+  return false;
+};