only identity if all values are colors (#676)

* only identity if all values are colors

* comments

src/options.js

@@ -25,9 +25,6 @@ export const first = d => d[0];
 export const second = d => d[1];
 export const constant = x => () => x;
 
-// A few extra color keywords not known to d3-color.
-const colors = new Set(["currentColor", "none"]);
-
 // Some channels may allow a string constant to be specified; to differentiate
 // string constants (e.g., "red") from named fields (e.g., "date"), this
 // function tests whether the given value is a CSS color string and returns a
@@ -37,7 +34,7 @@ const colors = new Set(["currentColor", "none"]);
 export function maybeColorChannel(value, defaultValue) {
   if (value === undefined) value = defaultValue;
   return value === null ? [undefined, "none"]
-    : typeof value === "string" && (colors.has(value) || color(value)) ? [undefined, value]
+    : isColor(value) ? [undefined, value]
     : [value, undefined];
 }
 
@@ -213,10 +210,35 @@ export function isNumeric(values) {
 export function isColors(values) {
   for (const value of values) {
     if (value == null) continue;
-    return typeof value === "string" && (colors.has(value) || color(value) !== null);
+    return isColor(value);
+  }
+}
+
+// Whereas isColors only tests the first defined value and returns undefined for
+// an empty array, this tests all defined values and only returns true if all of
+// them are valid colors. It also returns true for an empty array, and thus
+// should generally be used in conjunction with isColors.
+export function isAllColors(values) {
+  for (const value of values) {
+    if (value == null) continue;
+    if (!isColor(value)) return false;
   }
+  return true;
+}
+
+// Mostly relies on d3-color, with a few extra color keywords. Currently this
+// strictly requires that the value be a string; we might want to apply string
+// coercion here, though note that d3-color instances would need to support
+// valueOf to work correctly with InternMap.
+export function isColor(value) {
+  if (!(typeof value === "string")) return false;
+  value = value.toLowerCase();
+  return value === "currentcolor" || value === "none" || color(value) !== null;
 }
 
+// Like a sort comparator, returns a positive value if the given array of values
+// is in ascending order, a negative value if the values are in descending
+// order. Assumes monotonicity; only tests the first and last values.
 export function order(values) {
   if (values == null) return;
   const first = values[0];

src/scales.js

@@ -1,5 +1,5 @@
 import {parse as isoParse} from "isoformat";
-import {isColors, isOrdinal, isTemporal, order} from "./options.js";
+import {isAllColors, isColors, isOrdinal, isTemporal, order} from "./options.js";
 import {registry, color, position, radius, opacity, symbol, length} from "./scales/index.js";
 import {ScaleLinear, ScaleSqrt, ScalePow, ScaleLog, ScaleSymlog, ScaleQuantile, ScaleThreshold, ScaleIdentity} from "./scales/quantitative.js";
 import {ScaleDiverging, ScaleDivergingSqrt, ScaleDivergingPow, ScaleDivergingLog, ScaleDivergingSymlog} from "./scales/diverging.js";
@@ -202,8 +202,10 @@ function inferScaleType(key, channels, {type, domain, range}) {
   if (registry.get(key) === symbol) return "ordinal";
   for (const {type} of channels) if (type !== undefined) return type;
   if (registry.get(key) === color
-    && (domain !== undefined ? isColors(domain)
-      : channels.some(({value}) => value !== undefined && isColors(value)))) return "identity";
+    && (domain !== undefined
+      ? isColors(domain) && isAllColors(domain)
+      : channels.some(({value}) => value !== undefined && isColors(value))
+        && channels.every(({value}) => value === undefined || isAllColors(value)))) return "identity";
   if ((domain || range || []).length > 2) return asOrdinalType(key);
   if (domain !== undefined) {
     if (isOrdinal(domain)) return asOrdinalType(key);