fix(react): safer react attribute names (#25)

tchak · Apr 25, 2024 · 933922d · 933922d
2 parents 7ebf295 + 76f2297
commit 933922d
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 25 deletions.
diff --git a/.changeset/wet-ants-clean.md b/.changeset/wet-ants-clean.md
@@ -0,0 +1,5 @@
+---
+"@coldwired/react": patch
+---
+
+safer react attributes
diff --git a/packages/actions/src/actions-container.test.tsx b/packages/actions/src/actions-container.test.tsx
@@ -2,7 +2,13 @@ import { describe, it, expect, beforeEach } from 'vitest';
 import { getByText, fireEvent, waitFor } from '@testing-library/dom';
 import { StrictMode, useState } from 'react';
 
-import { NAME_ATTRIBUTE, PROPS_ATTRIBUTE, Manifest, encodeProps } from '@coldwired/react';
+import {
+  NAME_ATTRIBUTE,
+  PROPS_ATTRIBUTE,
+  REACT_COMPONENT_TAG,
+  Manifest,
+  encodeProps,
+} from '@coldwired/react';
 
 import { Actions } from '.';
 
@@ -70,7 +76,7 @@ describe('@coldwired/actions', () => {
 
     actions.update({
       targets: '#frag-1',
-      fragment: `<react-component ${NAME_ATTRIBUTE}="Counter"></react-component>`,
+      fragment: `<${REACT_COMPONENT_TAG} ${NAME_ATTRIBUTE}="Counter"></${REACT_COMPONENT_TAG}>`,
     });
     await actions.ready();
     await waitFor(() => {
@@ -100,7 +106,7 @@ describe('@coldwired/actions', () => {
 
     actions.update({
       targets: '#main',
-      fragment: `<${DEFAULT_TAG_NAME} id="frag-1"><react-component ${NAME_ATTRIBUTE}="Counter"></react-component></${DEFAULT_TAG_NAME}><${DEFAULT_TAG_NAME} id="frag-2">Test</${DEFAULT_TAG_NAME}>`,
+      fragment: `<${DEFAULT_TAG_NAME} id="frag-1"><${REACT_COMPONENT_TAG} ${NAME_ATTRIBUTE}="Counter"></${REACT_COMPONENT_TAG}></${DEFAULT_TAG_NAME}><${DEFAULT_TAG_NAME} id="frag-2">Test</${DEFAULT_TAG_NAME}>`,
     });
     await actions.ready();
     expect(actions.container?.getCache().size).toEqual(2);
@@ -114,7 +120,7 @@ describe('@coldwired/actions', () => {
 
     actions.update({
       targets: '#main',
-      fragment: `<${DEFAULT_TAG_NAME} id="frag-1"><react-component ${NAME_ATTRIBUTE}="Counter"></react-component></${DEFAULT_TAG_NAME}><${DEFAULT_TAG_NAME} id="frag-2">Test 23</${DEFAULT_TAG_NAME}>`,
+      fragment: `<${DEFAULT_TAG_NAME} id="frag-1"><${REACT_COMPONENT_TAG} ${NAME_ATTRIBUTE}="Counter"></${REACT_COMPONENT_TAG}></${DEFAULT_TAG_NAME}><${DEFAULT_TAG_NAME} id="frag-2">Test 23</${DEFAULT_TAG_NAME}>`,
     });
     await actions.ready();
     expect(actions.container?.getCache().size).toEqual(2);
@@ -128,7 +134,7 @@ describe('@coldwired/actions', () => {
 
     actions.update({
       targets: '#main',
-      fragment: `<${DEFAULT_TAG_NAME} id="frag-1"><react-component ${NAME_ATTRIBUTE}="Counter"></react-component></${DEFAULT_TAG_NAME}>`,
+      fragment: `<${DEFAULT_TAG_NAME} id="frag-1"><${REACT_COMPONENT_TAG} ${NAME_ATTRIBUTE}="Counter"></${REACT_COMPONENT_TAG}></${DEFAULT_TAG_NAME}>`,
     });
     await actions.ready();
     await waitFor(() => {
@@ -142,7 +148,7 @@ describe('@coldwired/actions', () => {
 
     actions.update({
       targets: '#main',
-      fragment: `<input name="age" /><${DEFAULT_TAG_NAME} id="frag-1"><react-component ${NAME_ATTRIBUTE}="Counter"></react-component></${DEFAULT_TAG_NAME}>`,
+      fragment: `<input name="age" /><${DEFAULT_TAG_NAME} id="frag-1"><${REACT_COMPONENT_TAG} ${NAME_ATTRIBUTE}="Counter"></${REACT_COMPONENT_TAG}></${DEFAULT_TAG_NAME}>`,
     });
     await actions.ready();
     await waitFor(() => {
@@ -156,7 +162,7 @@ describe('@coldwired/actions', () => {
 
     actions.update({
       targets: '#main',
-      fragment: `<section><input name="age" /></section><${DEFAULT_TAG_NAME} id="frag-1"><react-component ${NAME_ATTRIBUTE}="Counter"></react-component></${DEFAULT_TAG_NAME}>`,
+      fragment: `<section><input name="age" /></section><${DEFAULT_TAG_NAME} id="frag-1"><${REACT_COMPONENT_TAG} ${NAME_ATTRIBUTE}="Counter"></${REACT_COMPONENT_TAG}></${DEFAULT_TAG_NAME}>`,
     });
     await actions.ready();
     await waitFor(() => {
@@ -170,7 +176,7 @@ describe('@coldwired/actions', () => {
 
     actions.update({
       targets: '#main',
-      fragment: `<section><${DEFAULT_TAG_NAME} id="frag-1"><react-component ${NAME_ATTRIBUTE}="Counter" label="My Count"></react-component></${DEFAULT_TAG_NAME}></section>`,
+      fragment: `<section><${DEFAULT_TAG_NAME} id="frag-1"><${REACT_COMPONENT_TAG} ${NAME_ATTRIBUTE}="Counter" label="My Count"></${REACT_COMPONENT_TAG}></${DEFAULT_TAG_NAME}></section>`,
     });
     await actions.ready();
     await waitFor(() => {
@@ -193,7 +199,7 @@ describe('@coldwired/actions', () => {
 
     actions.update({
       targets: '#main',
-      fragment: `<section><${DEFAULT_TAG_NAME} id="frag-1"><react-component ${NAME_ATTRIBUTE}="Counter" ${PROPS_ATTRIBUTE}="${encodeProps({ label: 'My New Count' })}"></react-component></${DEFAULT_TAG_NAME}></section>`,
+      fragment: `<section><${DEFAULT_TAG_NAME} id="frag-1"><${REACT_COMPONENT_TAG} ${NAME_ATTRIBUTE}="Counter" ${PROPS_ATTRIBUTE}="${encodeProps({ label: 'My New Count' })}"></${REACT_COMPONENT_TAG}></${DEFAULT_TAG_NAME}></section>`,
     });
     await actions.ready();
     await waitFor(() => {

diff --git a/packages/react/src/container.test.tsx b/packages/react/src/container.test.tsx
@@ -1,7 +1,7 @@
 import { describe, it, expect } from 'vitest';
 import { useState, StrictMode } from 'react';
 
-import { createContainer, NAME_ATTRIBUTE, type Manifest } from '.';
+import { createContainer, NAME_ATTRIBUTE, REACT_COMPONENT_TAG, type Manifest } from '.';
 
 const DEFAULT_TAG_NAME = 'turbo-fragment';
 
@@ -49,7 +49,7 @@ describe('@coldwired/react', () => {
         fragmentTagName: DEFAULT_TAG_NAME,
         loadingClassName: 'loading',
       });
-      document.body.innerHTML = `<${DEFAULT_TAG_NAME}><react-component ${NAME_ATTRIBUTE}="Counter"></react-component></${DEFAULT_TAG_NAME}><div id="root"></div>`;
+      document.body.innerHTML = `<${DEFAULT_TAG_NAME}><${REACT_COMPONENT_TAG} ${NAME_ATTRIBUTE}="Counter"></${REACT_COMPONENT_TAG}></${DEFAULT_TAG_NAME}><div id="root"></div>`;
       await container.mount(document.getElementById('root')!, StrictMode);
       await container.render(document.body);
 

diff --git a/packages/react/src/react-tree-builder.test.tsx b/packages/react/src/react-tree-builder.test.tsx
@@ -3,7 +3,13 @@ import { renderToStaticMarkup } from 'react-dom/server';
 import type { ReactNode } from 'react';
 import { parseHTMLFragment } from '@coldwired/utils';
 
-import { createReactTree, hydrate, NAME_ATTRIBUTE } from './react-tree-builder';
+import {
+  createReactTree,
+  hydrate,
+  NAME_ATTRIBUTE,
+  REACT_COMPONENT_TAG,
+  REACT_SLOT_TAG,
+} from './react-tree-builder';
 
 function Greeting({ firstName, lastName }: { firstName: string; lastName: string }) {
   return (
@@ -126,10 +132,10 @@ describe('@coldwired/react', () => {
     });
 
     it('render component', () => {
-      const greeting1 = `<react-component ${NAME_ATTRIBUTE}="Greeting" first_name="John" last_name="Doe"></react-component>`;
-      const greeting2 = `<react-component ${NAME_ATTRIBUTE}="Greeting" first-name="Greer" last-name="Pilkington"></react-component>`;
-      const fieldset1 = `<react-component ${NAME_ATTRIBUTE}="FieldSet" lang="fr" legend="Test">Hello World</react-component>`;
-      const fieldset2 = `<react-component ${NAME_ATTRIBUTE}="FieldSet" lang="en"><react-slot ${NAME_ATTRIBUTE}="legend"><span class="blue">Test</span></react-slot>${greeting2}</react-component>`;
+      const greeting1 = `<${REACT_COMPONENT_TAG} ${NAME_ATTRIBUTE}="Greeting" first_name="John" last_name="Doe"></${REACT_COMPONENT_TAG}>`;
+      const greeting2 = `<${REACT_COMPONENT_TAG} ${NAME_ATTRIBUTE}="Greeting" first-name="Greer" last-name="Pilkington"></${REACT_COMPONENT_TAG}>`;
+      const fieldset1 = `<${REACT_COMPONENT_TAG} ${NAME_ATTRIBUTE}="FieldSet" lang="fr" legend="Test">Hello World</${REACT_COMPONENT_TAG}>`;
+      const fieldset2 = `<${REACT_COMPONENT_TAG} ${NAME_ATTRIBUTE}="FieldSet" lang="en"><${REACT_SLOT_TAG} ${NAME_ATTRIBUTE}="legend"><span class="blue">Test</span></${REACT_SLOT_TAG}>${greeting2}</${REACT_COMPONENT_TAG}>`;
 
       const tree = hydrate(
         parseHTMLFragment(`${greeting1}<form>${fieldset1}${fieldset2}</form>`, document),

diff --git a/packages/react/src/react-tree-builder.ts b/packages/react/src/react-tree-builder.ts
@@ -22,8 +22,10 @@ type Component = {
 };
 export type Manifest = Record<string, ComponentType<any>>;
 
-export const NAME_ATTRIBUTE = '$name';
-export const PROPS_ATTRIBUTE = '$props';
+export const NAME_ATTRIBUTE = '@name';
+export const PROPS_ATTRIBUTE = '@props';
+export const REACT_COMPONENT_TAG = 'react-component';
+export const REACT_SLOT_TAG = 'react-slot';
 
 function isElement(node: JSONValue | Element | Component): node is Element {
   return !!(node && typeof node == 'object' && 'tagName' in node && 'attributes' in node);
@@ -53,10 +55,10 @@ export function preload(
     ...new Set(
       Array.from(childNodes).flatMap((childNode) => {
         if (isElementNode(childNode)) {
-          if (childNode.tagName.toLowerCase() == 'react-component') {
+          if (childNode.tagName.toLowerCase() == REACT_COMPONENT_TAG) {
             return childNode.getAttribute(NAME_ATTRIBUTE)!;
           }
-          return Array.from(childNode.querySelectorAll('react-component')).map(
+          return Array.from(childNode.querySelectorAll(REACT_COMPONENT_TAG)).map(
             (element) => element.getAttribute(NAME_ATTRIBUTE)!,
           );
         }
@@ -99,10 +101,10 @@ function hydrateChildNodes(childNodes: NodeListOf<ChildNode>): HydrateResult {
     } else if (isElementNode(childNode)) {
       const tagName = childNode.tagName.toLowerCase();
       const { children, props } = hydrateChildNodes(childNode.childNodes);
-      if (tagName == 'react-component') {
+      if (tagName == REACT_COMPONENT_TAG) {
         const name = childNode.getAttribute(NAME_ATTRIBUTE);
         if (!name) {
-          throw new Error(`Missing "${NAME_ATTRIBUTE}" attribute on <react-component>`);
+          throw new Error(`Missing "${NAME_ATTRIBUTE}" attribute on <${REACT_COMPONENT_TAG}>`);
         }
         result.children.push({
           name,
@@ -111,12 +113,12 @@ function hydrateChildNodes(childNodes: NodeListOf<ChildNode>): HydrateResult {
         });
       } else {
         if (Object.keys(props).length > 0) {
-          throw new Error('<react-slot> only allowed as direct child of <react-component>');
+          throw new Error('<react-slot> only allowed as direct child of <${REACT_COMPONENT_TAG}>');
         }
-        if (tagName == 'react-slot') {
+        if (tagName == REACT_SLOT_TAG) {
           const name = childNode.getAttribute(NAME_ATTRIBUTE);
           if (!name) {
-            throw new Error(`Missing "${NAME_ATTRIBUTE}" attribute on <react-slot>`);
+            throw new Error(`Missing "${NAME_ATTRIBUTE}" attribute on <${REACT_SLOT_TAG}>`);
           }
           if (children.length == 1) {
             const child = children[0];