Correctly handle self closing jsx elements (#158)

* fix(ts-html-plugin): handle self closing elements as safe avoinding unecessary errors, impr debug expiriance * Create odd-panthers-occur.md Signed-off-by: Arthur Fiorette <[email protected]> * run ci again Signed-off-by: Arthur Fiorette <[email protected]> --------- Signed-off-by: Arthur Fiorette <[email protected]> Co-authored-by: Arthur Fiorette <[email protected]>
kitajs · Mar 25, 2024 · f00a024 · f00a024
1 parent c33d280
commit f00a024
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 24 deletions.
diff --git a/.changeset/odd-panthers-occur.md b/.changeset/odd-panthers-occur.md
@@ -0,0 +1,5 @@
+---
+"@kitajs/ts-html-plugin": patch
+---
+
+Correctly handle self closing jsx elements
diff --git a/packages/ts-html-plugin/src/util.ts b/packages/ts-html-plugin/src/util.ts
@@ -15,7 +15,9 @@ const ESCAPE_HTML_REGEX = /^(\w+\.)?(escapeHtml|e|escape)/i;
 
 /** If the node is a JSX element or fragment */
 export function isJsx(ts: typeof TS, node: TS.Node): node is JsxElement | JsxFragment {
-  return ts.isJsxElement(node) || ts.isJsxFragment(node);
+  return (
+    ts.isJsxElement(node) || ts.isJsxFragment(node) || ts.isJsxSelfClosingElement(node)
+  );
 }
 
 export function recursiveDiagnoseJsxElements(
@@ -71,7 +73,7 @@ export function diagnoseJsxElement(
 ): void {
   const file = node.getSourceFile();
 
-  // Validations that does not applies to fragments
+  // Validations that does not applies to fragments or serlf closing elements
   if (ts.isJsxElement(node)) {
     // Script tags should be ignored
     if (node.openingElement.tagName.getText() === 'script') {
@@ -81,7 +83,7 @@ export function diagnoseJsxElement(
     const safeAttribute = getSafeAttribute(node.openingElement);
 
     // Safe mode warnings
-    if (safeAttribute) {
+    if (safeAttribute && node.children) {
       if (
         // Empty element
         node.children.length === 0 ||
@@ -129,6 +131,12 @@ export function diagnoseJsxElement(
     }
   }
 
+  // If this expression does not have children, we can ignore it
+  // for example it could be a self closing element
+  if (!node.children) {
+    return;
+  }
+
   // Look for expressions
   for (const exp of node.children) {
     if (!ts.isJsxExpression(exp)) {

diff --git a/packages/ts-html-plugin/test/operators.test.ts b/packages/ts-html-plugin/test/operators.test.ts
@@ -7,6 +7,11 @@ it('Operators are evaluated normally', async () => {
   await using server = new TSLangServer(__dirname);
 
   const diagnostics = await server.openWithDiagnostics/* tsx */ `
+
+    function CustomComponent(props: { name: string }) {
+      return ${'<div>{Html.e`${props.name}`}</div>'}
+    }
+
     export default (
       <>
         <div>{boolean ? number : html}</div>
@@ -28,10 +33,16 @@ it('Operators are evaluated normally', async () => {
         <div>{boolean ? number : <div>Safe!</div>}</div>
         <div>{boolean ? number : (<div>Safe!</div>)}</div>
         <div>{boolean ? (number) : (<div><div>Deep safe!</div></div>)}</div>
+        <div>{boolean && (<CustomComponent name="boolean" />)}</div>
+        <div>{boolean && <CustomComponent name="boolean" />}</div>
+        <div>{boolean ? <CustomComponent name="boolean" /> : null}</div>
 
         <div>{number && <div>Safe!</div>}</div>
         <div>{number && (<div>Safe!</div>)}</div>
         <div>{number && (<div><div>Deep safe!</div></div>)}</div>
+        <div>{number && (<CustomComponent name="number" />)}</div>
+        <div>{number && <CustomComponent name="number" />}</div>
+        <div>{number ? <CustomComponent name="number" /> : null}</div>
 
         <div>{<div>Safe!</div> || safeString}</div>
         <div>{(<div>Safe!</div>) || safeString}</div>
@@ -51,73 +62,73 @@ it('Operators are evaluated normally', async () => {
 
   assert.deepStrictEqual(diagnostics.body, [
     {
-      start: { line: 36, offset: 34 },
-      end: { line: 36, offset: 38 },
+      start: { line: 40, offset: 34 },
+      end: { line: 40, offset: 38 },
       text: Xss.message,
       code: Xss.code,
       category: 'error'
     },
     {
-      start: { line: 37, offset: 35 },
-      end: { line: 37, offset: 39 },
+      start: { line: 41, offset: 35 },
+      end: { line: 41, offset: 39 },
       text: Xss.message,
       code: Xss.code,
       category: 'error'
     },
     {
-      start: { line: 38, offset: 40 },
-      end: { line: 38, offset: 44 },
+      start: { line: 42, offset: 40 },
+      end: { line: 42, offset: 44 },
       text: Xss.message,
       code: Xss.code,
       category: 'error'
     },
     {
-      start: { line: 39, offset: 48 },
-      end: { line: 39, offset: 52 },
+      start: { line: 43, offset: 48 },
+      end: { line: 43, offset: 52 },
       text: Xss.message,
       code: Xss.code,
       category: 'error'
     },
 
     {
-      start: { line: 41, offset: 25 },
-      end: { line: 41, offset: 29 },
+      start: { line: 45, offset: 25 },
+      end: { line: 45, offset: 29 },
       text: Xss.message,
       code: Xss.code,
       category: 'error'
     },
     {
-      start: { line: 42, offset: 26 },
-      end: { line: 42, offset: 30 },
+      start: { line: 46, offset: 26 },
+      end: { line: 46, offset: 30 },
       text: Xss.message,
       code: Xss.code,
       category: 'error'
     },
     {
-      start: { line: 43, offset: 37 },
-      end: { line: 43, offset: 41 },
+      start: { line: 47, offset: 37 },
+      end: { line: 47, offset: 41 },
       text: Xss.message,
       code: Xss.code,
       category: 'error'
     },
     {
-      start: { line: 44, offset: 31 },
-      end: { line: 44, offset: 35 },
+      start: { line: 48, offset: 31 },
+      end: { line: 48, offset: 35 },
       text: Xss.message,
       code: Xss.code,
       category: 'error'
     },
 
     {
-      start: { line: 48, offset: 21 },
-      end: { line: 48, offset: 25 },
+      start: { line: 52, offset: 21 },
+      end: { line: 52, offset: 25 },
       text: Xss.message,
       code: Xss.code,
       category: 'error'
     },
     {
-      start: { line: 49, offset: 27 },
-      end: { line: 49, offset: 31 },
+      start: { line: 53, offset: 27 },
+      end: { line: 53, offset: 31 },
       text: Xss.message,
       code: Xss.code,
       category: 'error'

diff --git a/packages/ts-html-plugin/test/util/lang-server.ts b/packages/ts-html-plugin/test/util/lang-server.ts
@@ -112,7 +112,7 @@ export class TSLangServer {
     const fileContent = TEST_HELPERS + '\n' + String.raw(content, ...args).trim();
 
     if (this.debug) {
-      console.log(fileContent);
+      console.log(this.strWithLineNumbers(fileContent));
     }
 
     await this.send({
@@ -203,4 +203,11 @@ export class TSLangServer {
 
     return this.exitPromise;
   }
+
+  strWithLineNumbers(str: string) {
+    return str
+      .split('\n')
+      .map((line, index) => `${index + 1 < 10 ? `0${index + 1}` : index + 1} | ${line}`)
+      .join('\n');
+  }
 }