.attr on null selection should not error

[isakbm]

Problem

When a node exists, say it has id="foo" in the DOM, then we can safely do

d3.select("#foo").attr("bar")

however, when the node does NOT exist, then

d3.select("#foo").attr("bar")

will cause the following error

TypeError: Cannot read properties of null (reading 'getAttribute')

The fix to this issue seems relatively straight forward. The culprit is in d3-selection/src/selection/attr.js.

export default function(name, value) {

  var fullname = namespace(name);

  if (arguments.length < 2) {
    var node = this.node();
    return fullname.local
        ? node.getAttributeNS(fullname.space, fullname.local)
        : node.getAttribute(fullname);
  }

  return this.each((value == null
      ? (fullname.local ? attrRemoveNS : attrRemove) : (typeof value === "function"
      ? (fullname.local ? attrFunctionNS : attrFunction)
      : (fullname.local ? attrConstantNS : attrConstant)))(fullname, value));
}

There is simply a missing null check on the lines trying to call either getAttributeNS or getAttribute on node.

Proposed solution

export default function(name, value) {

  var fullname = namespace(name);

  if (arguments.length < 2) {
    var node = this.node();
    if (node == null) return undefined;
    return fullname.local
        ? node.getAttributeNS(fullname.space, fullname.local)
        : node.getAttribute(fullname);
  }

  return this.each((value == null
      ? (fullname.local ? attrRemoveNS : attrRemove) : (typeof value === "function"
      ? (fullname.local ? attrFunctionNS : attrFunction)
      : (fullname.local ? attrConstantNS : attrConstant)))(fullname, value));
}

Could also consider just using ?. operator, however this may not be supported by all javascript runtimes?

[mbostock]

This is intentional. When you’re trying to retrieve the current value of an attribution on the selection, the expectation is that the selection has exactly one element. If you have zero or more than one element selected, the behavior is undefined. If you want to check whether the selection is empty first, you can call selection.empty.

[isakbm]

This is intentional. When you’re trying to retrieve the current value of an attribution on the selection, the expectation is that the selection has exactly one element. If you have zero or more than one element selected, the behavior is undefined. If you want to check whether the selection is empty first, you can call selection.empty.

@mbostock

But how is undefined behaviour better than simply returning undefined?

[mbostock]

In this case, throwing an error helps you understand that you shouldn’t be calling selection.attr when the selection is empty.