String coercion and comparison fixes.

Fixes #88. Fixes #87.
d3 · Jan 24, 2019 · fa94c9f · fa94c9f
1 parent 2a44471
commit fa94c9f
Show file tree

Hide file tree

Showing 4 changed files with 187 additions and 46 deletions.
diff --git a/src/transition/attr.js b/src/transition/attr.js
@@ -16,52 +16,56 @@ function attrRemoveNS(fullname) {
 }
 
 function attrConstant(name, interpolate, value1) {
-  var value00,
+  var string00,
+      string1 = value1 + "",
       interpolate0;
   return function() {
-    var value0 = this.getAttribute(name);
-    return value0 === value1 ? null
-        : value0 === value00 ? interpolate0
-        : interpolate0 = interpolate(value00 = value0, value1);
+    var string0 = this.getAttribute(name);
+    return string0 === string1 ? null
+        : string0 === string00 ? interpolate0
+        : interpolate0 = interpolate(string00 = string0, value1);
   };
 }
 
 function attrConstantNS(fullname, interpolate, value1) {
-  var value00,
+  var string00,
+      string1 = value1 + "",
       interpolate0;
   return function() {
-    var value0 = this.getAttributeNS(fullname.space, fullname.local);
-    return value0 === value1 ? null
-        : value0 === value00 ? interpolate0
-        : interpolate0 = interpolate(value00 = value0, value1);
+    var string0 = this.getAttributeNS(fullname.space, fullname.local);
+    return string0 === string1 ? null
+        : string0 === string00 ? interpolate0
+        : interpolate0 = interpolate(string00 = string0, value1);
   };
 }
 
 function attrFunction(name, interpolate, value) {
-  var value00,
-      value10,
+  var string00,
+      string10,
       interpolate0;
   return function() {
-    var value0, value1 = value(this);
+    var string0, value1 = value(this), string1;
     if (value1 == null) return void this.removeAttribute(name);
-    value0 = this.getAttribute(name);
-    return value0 === value1 ? null
-        : value0 === value00 && value1 === value10 ? interpolate0
-        : interpolate0 = interpolate(value00 = value0, value10 = value1);
+    string0 = this.getAttribute(name);
+    string1 = value1 + "";
+    return string0 === string1 ? null
+        : string0 === string00 && string1 === string10 ? interpolate0
+        : (string10 = string1, interpolate0 = interpolate(string00 = string0, value1));
   };
 }
 
 function attrFunctionNS(fullname, interpolate, value) {
-  var value00,
-      value10,
+  var string00,
+      string10,
       interpolate0;
   return function() {
-    var value0, value1 = value(this);
+    var string0, value1 = value(this), string1;
     if (value1 == null) return void this.removeAttributeNS(fullname.space, fullname.local);
-    value0 = this.getAttributeNS(fullname.space, fullname.local);
-    return value0 === value1 ? null
-        : value0 === value00 && value1 === value10 ? interpolate0
-        : interpolate0 = interpolate(value00 = value0, value10 = value1);
+    string0 = this.getAttributeNS(fullname.space, fullname.local);
+    string1 = value1 + "";
+    return string0 === string1 ? null
+        : string0 === string00 && string1 === string10 ? interpolate0
+        : (string10 = string1, interpolate0 = interpolate(string00 = string0, value1));
   };
 }
 
@@ -70,5 +74,5 @@ export default function(name, value) {
   return this.attrTween(name, typeof value === "function"
       ? (fullname.local ? attrFunctionNS : attrFunction)(fullname, i, tweenValue(this, "attr." + name, value))
       : value == null ? (fullname.local ? attrRemoveNS : attrRemove)(fullname)
-      : (fullname.local ? attrConstantNS : attrConstant)(fullname, i, value + ""));
+      : (fullname.local ? attrConstantNS : attrConstant)(fullname, i, value));
 }
diff --git a/src/transition/style.js b/src/transition/style.js
@@ -4,15 +4,15 @@ import {tweenValue} from "./tween";
 import interpolate from "./interpolate";
 
 function styleRemove(name, interpolate) {
-  var value00,
-      value10,
+  var string00,
+      string10,
       interpolate0;
   return function() {
-    var value0 = style(this, name),
-        value1 = (this.style.removeProperty(name), style(this, name));
-    return value0 === value1 ? null
-        : value0 === value00 && value1 === value10 ? interpolate0
-        : interpolate0 = interpolate(value00 = value0, value10 = value1);
+    var string0 = style(this, name),
+        string1 = (this.style.removeProperty(name), style(this, name));
+    return string0 === string1 ? null
+        : string0 === string00 && string1 === string10 ? interpolate0
+        : interpolate0 = interpolate(string00 = string0, string10 = string1);
   };
 }
 
@@ -23,27 +23,30 @@ function styleRemoveEnd(name) {
 }
 
 function styleConstant(name, interpolate, value1) {
-  var value00,
+  var string00,
+      string1 = value1 + "",
       interpolate0;
   return function() {
-    var value0 = style(this, name);
-    return value0 === value1 ? null
-        : value0 === value00 ? interpolate0
-        : interpolate0 = interpolate(value00 = value0, value1);
+    var string0 = style(this, name);
+    return string0 === string1 ? null
+        : string0 === string00 ? interpolate0
+        : interpolate0 = interpolate(string00 = string0, value1);
   };
 }
 
 function styleFunction(name, interpolate, value) {
-  var value00,
-      value10,
+  var string00,
+      string10,
       interpolate0;
   return function() {
-    var value0 = style(this, name),
-        value1 = value(this);
-    if (value1 == null) value1 = (this.style.removeProperty(name), style(this, name));
-    return value0 === value1 ? null
-        : value0 === value00 && value1 === value10 ? interpolate0
-        : interpolate0 = interpolate(value00 = value0, value10 = value1);
+    var string0 = style(this, name),
+        value1 = value(this),
+        string1 = value1 + "";
+    console.log({value1});
+    if (value1 == null) string1 = value1 = (this.style.removeProperty(name), style(this, name));
+    return string0 === string1 ? null
+        : string0 === string00 && string1 === string10 ? interpolate0
+        : (string10 = string1, interpolate0 = interpolate(string00 = string0, value1));
   };
 }
 
@@ -54,5 +57,5 @@ export default function(name, value, priority) {
           .on("end.style." + name, styleRemoveEnd(name))
       : this.styleTween(name, typeof value === "function"
           ? styleFunction(name, i, tweenValue(this, "style." + name, value))
-          : styleConstant(name, i, value + ""), priority);
+          : styleConstant(name, i, value), priority);
 }
diff --git a/test/transition/attr-test.js b/test/transition/attr-test.js
@@ -63,6 +63,88 @@ tape("transition.attr(name, value) creates a namespaced tween to the value retur
   }, 125);
 });
 
+tape("transition.attr(name, constant) is a noop if the string-coerced value matches the current value on tween initialization", function(test) {
+  var root = jsdom().documentElement,
+      selection = d3_selection.select(root).attr("foo", 1),
+      transition = selection.transition().attr("foo", 1);
+
+  d3_timer.timeout(function(elapsed) {
+    root.setAttribute("foo", 2);
+  }, 125);
+
+  d3_timer.timeout(function(elapsed) {
+    test.strictEqual(root.getAttribute("foo"), "2");
+    test.end();
+  }, 250);
+});
+
+tape("transition.attr(ns:name, constant) is a noop if the string-coerced value matches the current value on tween initialization", function(test) {
+  var root = jsdom().documentElement,
+      selection = d3_selection.select(root).attr("svg:foo", 1),
+      transition = selection.transition().attr("svg:foo", 1);
+
+  d3_timer.timeout(function(elapsed) {
+    root.setAttributeNS("http://www.w3.org/2000/svg", "foo", 2);
+  }, 125);
+
+  d3_timer.timeout(function(elapsed) {
+    test.strictEqual(root.getAttributeNS("http://www.w3.org/2000/svg", "foo"), "2");
+    test.end();
+  }, 250);
+});
+
+tape("transition.attr(name, function) is a noop if the string-coerced value matches the current value on tween initialization", function(test) {
+  var root = jsdom().documentElement,
+      selection = d3_selection.select(root).attr("foo", 1),
+      transition = selection.transition().attr("foo", function() { return 1; });
+
+  d3_timer.timeout(function(elapsed) {
+    root.setAttribute("foo", 2);
+  }, 125);
+
+  d3_timer.timeout(function(elapsed) {
+    test.strictEqual(root.getAttribute("foo"), "2");
+    test.end();
+  }, 250);
+});
+
+tape("transition.attr(ns:name, function) is a noop if the string-coerced value matches the current value on tween initialization", function(test) {
+  var root = jsdom().documentElement,
+      selection = d3_selection.select(root).attr("svg:foo", 1),
+      transition = selection.transition().attr("svg:foo", function() { return 1; });
+
+  d3_timer.timeout(function(elapsed) {
+    root.setAttributeNS("http://www.w3.org/2000/svg", "foo", 2);
+  }, 125);
+
+  d3_timer.timeout(function(elapsed) {
+    test.strictEqual(root.getAttributeNS("http://www.w3.org/2000/svg", "foo"), "2");
+    test.end();
+  }, 250);
+});
+
+tape("transition.attr(name, constant) uses interpolateNumber if value is a number", function(test) {
+  var root = jsdom().documentElement,
+      selection = d3_selection.select(root).attr("foo", "15px"),
+      transition = selection.transition().attr("foo", 10);
+
+  d3_timer.timeout(function(elapsed) {
+    test.strictEqual(root.getAttribute("foo"), "NaN");
+    test.end();
+  }, 125);
+});
+
+tape("transition.attr(name, function) uses interpolateNumber if value is a number", function(test) {
+  var root = jsdom().documentElement,
+      selection = d3_selection.select(root).attr("foo", "15px"),
+      transition = selection.transition().attr("foo", () => 10);
+
+  d3_timer.timeout(function(elapsed) {
+    test.strictEqual(root.getAttribute("foo"), "NaN");
+    test.end();
+  }, 125);
+});
+
 tape("transition.attr(name, value) immediately evaluates the specified function with the expected context and arguments", function(test) {
   var document = jsdom("<h1 id='one' fill='cyan'></h1><h1 id='two' fill='magenta'></h1>"),
       one = document.querySelector("#one"),

diff --git a/test/transition/style-test.js b/test/transition/style-test.js
@@ -103,6 +103,36 @@ tape("transition.style(name, value) creates an tween which removes the specified
   });
 });
 
+tape("transition.style(name, constant) is a noop if the string-coerced value matches the current value on tween initialization", function(test) {
+  var root = jsdom().documentElement,
+      selection = d3_selection.select(root).style("opacity", 1),
+      transition = selection.transition().style("opacity", 1);
+
+  d3_timer.timeout(function(elapsed) {
+    root.style.opacity = 0.5;
+  }, 125);
+
+  d3_timer.timeout(function(elapsed) {
+    test.strictEqual(root.style.getPropertyValue("opacity"), "0.5");
+    test.end();
+  }, 250);
+});
+
+tape("transition.style(name, function) is a noop if the string-coerced value matches the current value on tween initialization", function(test) {
+  var root = jsdom().documentElement,
+      selection = d3_selection.select(root).style("opacity", 1),
+      transition = selection.transition().style("opacity", function() { return 1; });
+
+  d3_timer.timeout(function(elapsed) {
+    root.style.opacity = 0.5;
+  }, 125);
+
+  d3_timer.timeout(function(elapsed) {
+    test.strictEqual(root.style.getPropertyValue("opacity"), "0.5");
+    test.end();
+  }, 250);
+});
+
 tape("transition.style(name, value) interpolates numbers", function(test) {
   var root = jsdom().documentElement,
       ease = d3_ease.easeCubic,
@@ -117,6 +147,28 @@ tape("transition.style(name, value) interpolates numbers", function(test) {
   }, 125);
 });
 
+tape("transition.style(name, constant) uses interpolateNumber if value is a number", function(test) {
+  var root = jsdom().documentElement,
+      selection = d3_selection.select(root).style("font-size", "15px"),
+      transition = selection.transition().style("font-size", 10);
+
+  d3_timer.timeout(function(elapsed) {
+    test.strictEqual(root.style.getPropertyValue("font-size"), "NaN");
+    test.end();
+  }, 125);
+});
+
+tape("transition.style(name, function) uses interpolateNumber if value is a number", function(test) {
+  var root = jsdom().documentElement,
+      selection = d3_selection.select(root).style("font-size", "15px"),
+      transition = selection.transition().style("font-size", () => 10);
+
+  d3_timer.timeout(function(elapsed) {
+    test.strictEqual(root.style.getPropertyValue("font-size"), "NaN");
+    test.end();
+  }, 125);
+});
+
 tape("transition.style(name, value) interpolates strings", function(test) {
   var root = jsdom().documentElement,
       ease = d3_ease.easeCubic,