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.

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

Not passing data.links breaks the library while it should throw a custom error #211

Closed
danielcaldas opened this issue Aug 2, 2019 · 4 comments

Comments

@danielcaldas
Copy link
Owner

Describe the bug
A detailed stack can be found below:

graph.helper.js:275 Uncaught TypeError: Cannot read property 'length' of undefined
    at _validateGraphData (graph.helper.js:275)
    at Object.initializeGraphState (graph.helper.js:414)
    at new Graph (Graph.js:546)
    at constructClassInstance (react-dom.development.js:11786)
    at updateClassComponent (react-dom.development.js:15264)

To Reproduce
Steps to reproduce the behavior:

  1. Create a <Graph /> component without links in the data
  2. Mount the graph
  3. An internal error is thrown while ideally, we should show to the developers why the library is breaking (in this case, because we don't pass links data)

Expected behavior
We should see a custom error that will point developers into the right direction, something like

react-d3-graph error :: You are passing invalid data to react-d3-graph. Validation step failed. Maybe you forget to pass links array within the data prop of the <Graph> component?

Screenshots
If applicable, add screenshots to help explain your problem.

Environment:
All

@danielcaldas
Copy link
Owner Author

@LonelyPrincess what do you think it would be the most appropriate message in this case? Do you think the example I provided is clear enough?

@LonelyPrincess
Copy link
Collaborator

LonelyPrincess commented Aug 2, 2019

You're right, it would be good to trigger a custom message to make things clear to the developer in this case. The message you provide suggests that they may have forgotten to include the links property, but if we know for sure they're missing that out, we could be more straightforward about the issue.

react-d3-graph error :: You are passing invalid data to react-d3-graph. You must include a links array in the data object you're passing down to the <Graph> component.

There's something else we may need to consider, though. 🤔 Since passing an empty links array is supported, then providing at least a link is not mandatory. I doubt many people would use this library to display isolate nodes without any links, but since the user is free not to pass any link at all, we could set the data.links property to an empty array by default.

That would prevent this error from happening in the first place, although it would be still a good idea to show a warning to the developer indicating they are missing the links property in case they didn't do it on purpose.

@danielcaldas
Copy link
Owner Author

Fixed with #246

@danielcaldas
Copy link
Owner Author

There's something else we may need to consider, though. 🤔 Since passing an empty links array is supported, then providing at least a link is not mandatory. I doubt many people would use this library to display isolate nodes without any links, but since the user is free not to pass any link at all, we could set the data.links property to an empty array by default.
That would prevent this error from happening in the first place, although it would be still a good idea to show a warning to the developer indicating they are missing the links property in case they didn't do it on purpose.

I like this idea @LonelyPrincess, the change it's not big, but it would handle this in a more graceful manner. Feel free to take over on this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants