Change attribute handling, fix unowned removal

ryansolid · Jun 19, 2020 · 4aca0d2 · 4aca0d2
1 parent 895e847
commit 4aca0d2
Show file tree

Hide file tree

Showing 10 changed files with 76 additions and 67 deletions.
diff --git a/package.json b/package.json
@@ -1,7 +1,7 @@
 {
   "name": "dom-expressions-build",
   "description": "A Fine-Grained Runtime for Performant DOM Rendering",
-  "version": "0.18.0",
+  "version": "0.19.0",
   "author": "Ryan Carniato",
   "license": "MIT",
   "repository": {

diff --git a/packages/babel-plugin-jsx-dom-expressions/src/dom/element.js b/packages/babel-plugin-jsx-dom-expressions/src/dom/element.js
@@ -99,11 +99,10 @@ export function setAttr(path, elem, name, value, isSVG, dynamic, prevId) {
     if (attribute.alias) name = attribute.alias;
   } else if (isSVG) name = name.replace(/([A-Z])/g, g => `-${g[0].toLowerCase()}`);
 
-  if (isAttribute)
-    return t.callExpression(t.memberExpression(elem, t.identifier("setAttribute")), [
-      t.stringLiteral(name),
-      value
-    ]);
+  if (isAttribute) {
+    registerImportMethod(path, "setAttribute");
+    return t.callExpression(t.identifier("_$setAttribute"), [elem, t.stringLiteral(name), value]);
+  }
   return t.assignmentExpression("=", t.memberExpression(elem, t.identifier(name)), value);
 }
 
@@ -152,7 +151,7 @@ function transformAttributes(path, results) {
       a.node.name.name === "style" &&
       t.isJSXExpressionContainer(a.node.value) &&
       t.isObjectExpression(a.node.value.expression) &&
-      !(a.node.value.expression.properties.some(p => t.isSpreadElement(p)))
+      !a.node.value.expression.properties.some(p => t.isSpreadElement(p))
   );
   if (styleAttribute) {
     let i = 0,
@@ -210,8 +209,13 @@ function transformAttributes(path, results) {
         key = t.isJSXNamespacedName(node.name)
           ? `${node.name.namespace.name}:${node.name.name.name}`
           : node.name.name,
-        reservedNameSpace = t.isJSXNamespacedName(node.name) && reservedNameSpaces[node.name.namespace.name];
-      if (t.isJSXNamespacedName(node.name) && reservedNameSpace && !t.isJSXExpressionContainer(value)) {
+        reservedNameSpace =
+          t.isJSXNamespacedName(node.name) && reservedNameSpaces[node.name.namespace.name];
+      if (
+        t.isJSXNamespacedName(node.name) &&
+        reservedNameSpace &&
+        !t.isJSXExpressionContainer(value)
+      ) {
         node.value = value = t.JSXExpressionContainer(value);
       }
       if (
@@ -237,9 +241,7 @@ function transformAttributes(path, results) {
             );
           } else if (t.isFunction(value.expression)) {
             results.exprs.unshift(
-              t.expressionStatement(
-                t.callExpression(value.expression, [elem])
-              )
+              t.expressionStatement(t.callExpression(value.expression, [elem]))
             );
           }
         } else if (key === "children") {
@@ -374,7 +376,7 @@ function wrappedByText(list, startIndex) {
   let index = startIndex,
     wrapped;
   while (--index >= 0) {
-    const node = list[index]
+    const node = list[index];
     if (!node) continue;
     if (node.text) {
       wrapped = true;
@@ -385,7 +387,7 @@ function wrappedByText(list, startIndex) {
   if (!wrapped) return false;
   index = startIndex;
   while (++index < list.length) {
-    const node = list[index]
+    const node = list[index];
     if (!node) continue;
     if (node.text) return true;
     if (node.id) return false;
@@ -399,18 +401,21 @@ function transformChildren(path, results) {
     nextPlaceholder,
     i = 0;
   const filteredChildren = filterChildren(path.get("children"), true),
-    childNodes = filteredChildren.map((child, index) =>
-      transformNode(child, {
-        skipId: !results.id || !detectExpressions(filteredChildren, index)
-      })
-    // combine adjacent textNodes
-    ).reduce((memo, child) => {
-      const i = memo.length
-      if (child.text && i && memo[i -1].text) {
-        memo[i - 1].template += child.template;
-      } else memo.push(child);
-      return memo;
-    }, []);
+    childNodes = filteredChildren
+      .map(
+        (child, index) =>
+          transformNode(child, {
+            skipId: !results.id || !detectExpressions(filteredChildren, index)
+          })
+        // combine adjacent textNodes
+      )
+      .reduce((memo, child) => {
+        const i = memo.length;
+        if (child.text && i && memo[i - 1].text) {
+          memo[i - 1].template += child.template;
+        } else memo.push(child);
+        return memo;
+      }, []);
 
   childNodes.forEach((child, index) => {
     if (!child) return;
@@ -437,22 +442,13 @@ function transformChildren(path, results) {
       const multi = checkLength(filteredChildren),
         markers = (generate === "dom-ssr" || hydratable) && multi;
       // boxed by textNodes
-      if (
-        markers ||
-        wrappedByText(childNodes, index)
-      ) {
+      if (markers || wrappedByText(childNodes, index)) {
         let exprId, contentId;
         if (markers) tempPath = createPlaceholder(path, results, tempPath, i++, "#")[0].name;
         if (nextPlaceholder) {
           exprId = nextPlaceholder;
         } else {
-          [exprId, contentId] = createPlaceholder(
-            path,
-            results,
-            tempPath,
-            i++,
-            markers ? "/" : ""
-          );
+          [exprId, contentId] = createPlaceholder(path, results, tempPath, i++, markers ? "/" : "");
         }
         if (!markers) nextPlaceholder = exprId;
         results.exprs.push(

diff --git a/packages/babel-plugin-jsx-dom-expressions/test/__dom_fixtures__/SVG/output.js b/packages/babel-plugin-jsx-dom-expressions/test/__dom_fixtures__/SVG/output.js
@@ -1,6 +1,7 @@
 import { template as _$template } from "r-dom";
 import { createComponent as _$createComponent } from "r-dom";
 import { spread as _$spread } from "r-dom";
+import { setAttribute as _$setAttribute } from "r-dom";
 import { effect as _$effect } from "r-dom";
 
 const _tmpl$ = _$template(
@@ -35,10 +36,10 @@ const template2 = (() => {
         _v$3 = state.x,
         _v$4 = state.y,
         _v$5 = props.stroke;
-      _v$ !== _p$._v$ && _el$3.setAttribute("class", (_p$._v$ = _v$));
-      _v$2 !== _p$._v$2 && _el$3.setAttribute("stroke-width", (_p$._v$2 = _v$2));
-      _v$3 !== _p$._v$3 && _el$3.setAttribute("x", (_p$._v$3 = _v$3));
-      _v$4 !== _p$._v$4 && _el$3.setAttribute("y", (_p$._v$4 = _v$4));
+      _v$ !== _p$._v$ && _$setAttribute(_el$3, "class", (_p$._v$ = _v$));
+      _v$2 !== _p$._v$2 && _$setAttribute(_el$3, "stroke-width", (_p$._v$2 = _v$2));
+      _v$3 !== _p$._v$3 && _$setAttribute(_el$3, "x", (_p$._v$3 = _v$3));
+      _v$4 !== _p$._v$4 && _$setAttribute(_el$3, "y", (_p$._v$4 = _v$4));
       _v$5 !== _p$._v$5 && _el$3.style.setProperty("stroke-width", (_p$._v$5 = _v$5));
       return _p$;
     },

diff --git a/packages/babel-plugin-jsx-dom-expressions/test/__dom_fixtures__/customElements/output.js b/packages/babel-plugin-jsx-dom-expressions/test/__dom_fixtures__/customElements/output.js
@@ -1,6 +1,7 @@
 import { template as _$template } from "r-dom";
 import { effect as _$effect } from "r-dom";
 import { currentContext as _$currentContext } from "r-dom";
+import { setAttribute as _$setAttribute } from "r-dom";
 
 const _tmpl$ = _$template(`<my-element></my-element>`, 2),
   _tmpl$2 = _$template(`<my-element><header slot="head">Title</header></my-element>`, 4),
@@ -9,7 +10,7 @@ const _tmpl$ = _$template(`<my-element></my-element>`, 2),
 const template = (() => {
   const _el$ = _tmpl$.cloneNode(true);
 
-  _el$.setAttribute("some-attr", name);
+  _$setAttribute(_el$, "some-attr", name);
 
   _el$.someProp = data;
   _el$._context = _$currentContext();
@@ -25,7 +26,7 @@ const template2 = (() => {
     _p$ => {
       const _v$ = state.name,
         _v$2 = state.data;
-      _v$ !== _p$._v$ && _el$2.setAttribute("some-attr", (_p$._v$ = _v$));
+      _v$ !== _p$._v$ && _$setAttribute(_el$2, "some-attr", (_p$._v$ = _v$));
       _v$2 !== _p$._v$2 && (_el$2.someProp = _p$._v$2 = _v$2);
       return _p$;
     },

diff --git a/packages/babel-plugin-jsx-dom-expressions/test/__dom_hydratable_fixtures__/SVG/output.js b/packages/babel-plugin-jsx-dom-expressions/test/__dom_hydratable_fixtures__/SVG/output.js
@@ -2,6 +2,7 @@ import { template as _$template } from "r-dom";
 import { createComponent as _$createComponent } from "r-dom";
 import { runHydrationEvents as _$runHydrationEvents } from "r-dom";
 import { spread as _$spread } from "r-dom";
+import { setAttribute as _$setAttribute } from "r-dom";
 import { effect as _$effect } from "r-dom";
 import { getNextElement as _$getNextElement } from "r-dom";
 
@@ -37,10 +38,10 @@ const template2 = (() => {
         _v$3 = state.x,
         _v$4 = state.y,
         _v$5 = props.stroke;
-      _v$ !== _p$._v$ && _el$3.setAttribute("class", (_p$._v$ = _v$));
-      _v$2 !== _p$._v$2 && _el$3.setAttribute("stroke-width", (_p$._v$2 = _v$2));
-      _v$3 !== _p$._v$3 && _el$3.setAttribute("x", (_p$._v$3 = _v$3));
-      _v$4 !== _p$._v$4 && _el$3.setAttribute("y", (_p$._v$4 = _v$4));
+      _v$ !== _p$._v$ && _$setAttribute(_el$3, "class", (_p$._v$ = _v$));
+      _v$2 !== _p$._v$2 && _$setAttribute(_el$3, "stroke-width", (_p$._v$2 = _v$2));
+      _v$3 !== _p$._v$3 && _$setAttribute(_el$3, "x", (_p$._v$3 = _v$3));
+      _v$4 !== _p$._v$4 && _$setAttribute(_el$3, "y", (_p$._v$4 = _v$4));
       _v$5 !== _p$._v$5 && _el$3.style.setProperty("stroke-width", (_p$._v$5 = _v$5));
       return _p$;
     },

diff --git a/...abel-plugin-jsx-dom-expressions/test/__dom_hydratable_fixtures__/customElements/output.js b/...abel-plugin-jsx-dom-expressions/test/__dom_hydratable_fixtures__/customElements/output.js
@@ -2,6 +2,7 @@ import { template as _$template } from "r-dom";
 import { effect as _$effect } from "r-dom";
 import { getNextElement as _$getNextElement } from "r-dom";
 import { currentContext as _$currentContext } from "r-dom";
+import { setAttribute as _$setAttribute } from "r-dom";
 
 const _tmpl$ = _$template(`<my-element></my-element>`, 2),
   _tmpl$2 = _$template(`<my-element><header slot="head">Title</header></my-element>`, 4),
@@ -10,7 +11,7 @@ const _tmpl$ = _$template(`<my-element></my-element>`, 2),
 const template = (() => {
   const _el$ = _$getNextElement(_tmpl$);
 
-  _el$.setAttribute("some-attr", name);
+  _$setAttribute(_el$, "some-attr", name);
 
   _el$.someProp = data;
   _el$._context = _$currentContext();
@@ -26,7 +27,7 @@ const template2 = (() => {
     _p$ => {
       const _v$ = state.name,
         _v$2 = state.data;
-      _v$ !== _p$._v$ && _el$2.setAttribute("some-attr", (_p$._v$ = _v$));
+      _v$ !== _p$._v$ && _$setAttribute(_el$2, "some-attr", (_p$._v$ = _v$));
       _v$2 !== _p$._v$2 && (_el$2.someProp = _p$._v$2 = _v$2);
       return _p$;
     },

diff --git a/packages/babel-plugin-jsx-dom-expressions/test/__dom_ssr_fixtures__/SVG/output.js b/packages/babel-plugin-jsx-dom-expressions/test/__dom_ssr_fixtures__/SVG/output.js
@@ -1,6 +1,7 @@
 import { template as _$template } from "r-dom";
 import { createComponent as _$createComponent } from "r-dom";
 import { spread as _$spread } from "r-dom";
+import { setAttribute as _$setAttribute } from "r-dom";
 import { effect as _$effect } from "r-dom";
 import { getNextElement as _$getNextElement } from "r-dom";
 
@@ -36,10 +37,10 @@ const template2 = (() => {
         _v$3 = state.x,
         _v$4 = state.y,
         _v$5 = props.stroke;
-      _v$ !== _p$._v$ && _el$3.setAttribute("class", (_p$._v$ = _v$));
-      _v$2 !== _p$._v$2 && _el$3.setAttribute("stroke-width", (_p$._v$2 = _v$2));
-      _v$3 !== _p$._v$3 && _el$3.setAttribute("x", (_p$._v$3 = _v$3));
-      _v$4 !== _p$._v$4 && _el$3.setAttribute("y", (_p$._v$4 = _v$4));
+      _v$ !== _p$._v$ && _$setAttribute(_el$3, "class", (_p$._v$ = _v$));
+      _v$2 !== _p$._v$2 && _$setAttribute(_el$3, "stroke-width", (_p$._v$2 = _v$2));
+      _v$3 !== _p$._v$3 && _$setAttribute(_el$3, "x", (_p$._v$3 = _v$3));
+      _v$4 !== _p$._v$4 && _$setAttribute(_el$3, "y", (_p$._v$4 = _v$4));
       _v$5 !== _p$._v$5 && _el$3.style.setProperty("stroke-width", (_p$._v$5 = _v$5));
       return _p$;
     },

diff --git a/packages/babel-plugin-jsx-dom-expressions/test/__dom_ssr_fixtures__/customElements/output.js b/packages/babel-plugin-jsx-dom-expressions/test/__dom_ssr_fixtures__/customElements/output.js
@@ -2,6 +2,7 @@ import { template as _$template } from "r-dom";
 import { effect as _$effect } from "r-dom";
 import { getNextElement as _$getNextElement } from "r-dom";
 import { currentContext as _$currentContext } from "r-dom";
+import { setAttribute as _$setAttribute } from "r-dom";
 
 const _tmpl$ = _$template(`<my-element></my-element>`, 2),
   _tmpl$2 = _$template(`<my-element><header slot="head">Title</header></my-element>`, 4),
@@ -10,7 +11,7 @@ const _tmpl$ = _$template(`<my-element></my-element>`, 2),
 const template = (() => {
   const _el$ = _$getNextElement(_tmpl$, true);
 
-  _el$.setAttribute("some-attr", name);
+  _$setAttribute(_el$, "some-attr", name);
 
   _el$.someProp = data;
   _el$._context = _$currentContext();
@@ -26,7 +27,7 @@ const template2 = (() => {
     _p$ => {
       const _v$ = state.name,
         _v$2 = state.data;
-      _v$ !== _p$._v$ && _el$2.setAttribute("some-attr", (_p$._v$ = _v$));
+      _v$ !== _p$._v$ && _$setAttribute(_el$2, "some-attr", (_p$._v$ = _v$));
       _v$2 !== _p$._v$2 && (_el$2.someProp = _p$._v$2 = _v$2);
       return _p$;
     },

diff --git a/packages/dom-expressions/src/runtime.d.ts b/packages/dom-expressions/src/runtime.d.ts
@@ -31,6 +31,7 @@ export function delegateEvents(eventNames: string[]): void;
 export function clearDelegatedEvents(): void;
 export function spread(node: Element, accessor: any, isSVG?: Boolean, skipChildren?: Boolean): void;
 export function assign(node: Element, props: any, isSVG?: Boolean, skipChildren?: Boolean): void;
+export function setAttribute(node : Element, name : string, value : any): void;
 export function classList(
   node: Element,
   value: { [k: string]: boolean },

diff --git a/packages/dom-expressions/src/runtime.js b/packages/dom-expressions/src/runtime.js
@@ -1,5 +1,5 @@
 import { Attributes, SVGAttributes, NonComposedEvents } from "./constants";
-import  { root, effect, memo, currentContext, createComponent } from "rxcore";
+import { root, effect, memo, currentContext, createComponent } from "rxcore";
 import reconcileArrays from "./reconcile";
 
 const eventRegistry = new Set(),
@@ -21,7 +21,7 @@ export function renderToString(code, options = {}) {
   hydration.context = { id: "0", count: 0 };
   return root(() => {
     const rendered = code();
-    if (typeof rendered === "object" && 'then' in rendered) {
+    if (typeof rendered === "object" && "then" in rendered) {
       const timeout = new Promise((_, reject) =>
         setTimeout(() => reject("renderToString timed out"), options.timeoutMs)
       );
@@ -46,7 +46,7 @@ export function renderDOMToString(code, options = {}) {
       return html;
     }
 
-    if (typeof rendered === "object" && 'then' in rendered) {
+    if (typeof rendered === "object" && "then" in rendered) {
       const timeout = new Promise((_, reject) =>
         setTimeout(() => reject("renderToString timed out"), options.timeoutMs)
       );
@@ -99,6 +99,11 @@ export function clearDelegatedEvents() {
   eventRegistry.clear();
 }
 
+export function setAttribute(node, name, value) {
+  if (value === false || value == null) node.removeAttribute(name);
+  else node.setAttribute(name, value);
+}
+
 export function classList(node, value, prev) {
   const classKeys = Object.keys(value);
   for (let i = 0, len = classKeys.length; i < len; i++) {
@@ -200,13 +205,13 @@ export function ssr(template, ...nodes) {
   }
   const t = () => {
     let result = "";
-    for(let i = 0; i < template.length; i++) {
+    for (let i = 0; i < template.length; i++) {
       result += template[i];
       const node = rNodes[i];
       if (node !== undefined) result += resolveSSRNode(node);
     }
     return result;
-  }
+  };
   t.isTemplate = true;
   return t;
 }
@@ -265,13 +270,13 @@ export function escape(html, attr) {
   if (typeof html !== "string") return html;
   return html.replace(attr ? ATTR_REGEX : CONTENT_REGEX, m => {
     switch (m) {
-      case '&':
-        return '&amp;';
-      case '<':
-        return '&lt;';
+      case "&":
+        return "&amp;";
+      case "<":
+        return "&lt;";
       case '"':
-        return '&quot;';
-      }
+        return "&quot;";
+    }
   });
 }
 
@@ -471,7 +476,8 @@ function cleanChildren(parent, current, marker, replacement) {
   const node = replacement || document.createTextNode("");
   if (current.length) {
     node !== current[0] && parent.replaceChild(node, current[0]);
-    for (let i = current.length - 1; i > 0; i--) parent.removeChild(current[i]);
+    for (let i = current.length - 1; i > 0; i--)
+      current[i].parentNode === parent && parent.removeChild(current[i]);
   } else parent.insertBefore(node, marker);
   return [node];
 }