Skip to content

Recently added d3-strict check balks on parcoords brush interaction with Dev Console open #2244

Closed
@monfera

Description

@monfera

Just checking this report by @alexcjohnson:

Uncaught Error: d3 selection.style called as getter: disallowed as it can fail for unattached elements. Use node.style.attribute instead.
    at Array.selProto.style (strict-d3.js:36)
    at SVGGElement.brushstart (d3.js:9209)
    at SVGGElement.__onmousedown.brush (d3.js:1120)

The test_dashboard seems to work for parcoords

> npm run start-test_dashboard

parcoords
but indeed it shows the error in the console if Dev Tools are open.

It's good to avoid .style('something') or .attr('something') getters in favor of one-directional data flow. If we can read something this way, probably we put that info there to begin with, so it should be in the model / viewModel. I was curious how it got in there.

Turns out, D3 itself is doing this in the d3.svg.brush component which is used for the axis brush:

d3.select("body").style("cursor", eventTarget.style("cursor"));

This one seems to have a good motive, and also safe because the eventTarget must exist in the DOM in order to capture an event, and the event callback, being synchronous, can't be preceded by the target removal, and I'm wondering how we could exempt it.

To clarify, what's meant by unattached elements, simply, not being in the document tree? For reasons of possible leak avoidance and API compactness (I think) D3 doesn't separate createElement and attachment (appendChild or insertBefore) so an .append() will do both. So D3 isn't optimized for creation of entire DOM fragments for subsequent attachment or repeated attachment/removal (would be neat as it can reduce jank via less gc). Upon .remove() the node gets removed from the DOM but can't immediately get garbage collected because the D3 selection in v3 and v4 hangs onto it via a strong reference, so we can still query the detached node (v4):

https://codepen.io/anon/pen/ppVvVB

If the node got .removed and the corresponding selection in the lexical scope is gone (ie. GC happened and we might be in some subsequent function), then we can no longer reference it via that selection and have to perform a new selection, and with the node detached, the node won't be in it.

So it'd be good to know what case resulted in the creation of this rule; not knowing that, these might be some options, each has challenges so looking for input:

  1. figure out a way for the rule to apply selectively for userland code vs D3 (seems hard, unwieldy or error prone)
  2. find a way for D3 to not need this, make a PR for it and hope it gets merged (ifs)
  3. reimplement brushing in our userland code (anti-DRY; bundle size; error prone)
  4. catch the possible error in parcoords (but it's not needed in prod code)
  5. disable the rule (it's a good rule though if it can catch legit problems)
  6. not worry about it (would be sweeping it under the rug, only for completeness) as the strict test is run on initial renders and brushing is interactive, and in test_dashboard it's a problem only if Dev Tools is open and it's set to stop on errors

I tested some other plots with D3 based interactions (sankey, table) but they didn't have this issue. Any chart however might in theory have non-strict-d3 calls on interactions as it's a runtime check.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions