Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Node indices not resolved to objects in links #32

Closed
curran opened this issue May 19, 2016 · 5 comments
Closed

Node indices not resolved to objects in links #32

curran opened this issue May 19, 2016 · 5 comments

Comments

@curran
Copy link
Contributor

curran commented May 19, 2016 •
edited

It seems that d3.forceLink does not replace indices in links (source and target) with references to the corresponding nodes. The documentation in #link_id states that this should be the expected behavior, and I noticed you implemented this in the code, but the behavior is different as of d3.v4.0.0-alpha.40 (or perhaps I'm misusing the API?).

Here's an example in JSBin that shows this behavior, also pasted here:

<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8">
    <title>D3 Force Test</title>
    <script src="https://d3js.org/d3.v4.0.0-alpha.40.min.js"></script>
  </head>
  <body>
    <script>

      var graph = {
        "nodes": ["a", "b", "c"],
        "links": [
          { "source": 0, "target": 1 },
          { "source": 1, "target": 2 }
        ]
      };

      var simulation = d3.forceSimulation(graph.nodes)
        .force("link", d3.forceLink(graph.links));

    </script>
  </body>
</html>

The error manifests as

d3.v4.0.0-alpha.40.min.js:4 Uncaught TypeError: Cannot create property 'vx' on string 'a'

Thanks for all your work on D3 4.0, I'm having lots of fun trying out the beta!
@mbostock
Copy link
Member

mbostock commented May 19, 2016 •
edited

The documentation is out-of-date and I need to update it. You need to use link.id. Specifically: link.id(function(d, i) { return i; }).

EDIT: I was wrong. See subsequent comment.

@curran
Copy link
Contributor Author

curran commented May 19, 2016

Ok, thank you.

I do think it would be nice, though, to have the index as the default id function. Just my 2¢.

All the best.

@curran curran closed this as completed May 19, 2016
@mbostock
Copy link
Member

mbostock commented May 19, 2016 •
edited

Oops, I was wrong, and I overlooked something in your example.

The default node id accessor does use the numeric index as desired:

function id(d, i) {
  return i;
}

The problem isn’t in link.id. The problem is that your nodes are not objects: they are strings. The simulation is trying to assign the various node properties (in particular node.vx, representing the x-component of the node’s velocity) to those strings, and that’s what’s failing with a TypeError: “Cannot create property 'vx' on string 'a'.”

You need to use objects to represent your data, say like this:

var graph = {
  "nodes": [
    {"id": "a"},
    {"id": "b"},
    {"id": "c"}
  ],
  "links": [
    {"source": 0, "target": 1},
    {"source": 1, "target": 2}
  ]
};

The simulation doesn’t wrap the input nodes and links; it assigns properties to them directly.

@curran
Copy link
Contributor Author

curran commented May 19, 2016

Ahhhh got it. Makes perfect sense. Thank you.

@curran curran mentioned this issue May 23, 2016
Closed
3 tasks
@duhaime
Copy link

duhaime commented Sep 27, 2017 •
edited

In case it might help others, I hit this same snag ("Cannot create property 'vx' on string...") because I had updated my simulation's nodes but hadn't updated the links when transitioning between layouts, so needed to add a simulation.force('link').links(links); line before the .force('link', d3.forceLink(links).id(function(d) { return d.id; }) line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@curran @mbostock @duhaime