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

Cannot read property 'x' of undefined error #42

Closed
almirfilho opened this issue Mar 11, 2017 · 9 comments
Closed

Cannot read property 'x' of undefined error #42

almirfilho opened this issue Mar 11, 2017 · 9 comments
Assignees
Labels

Comments

@almirfilho
Copy link

Before anything, I'd like to thank you very much for this package!

I don't have an idea why this is happening, but sometimes (apparently randomly) this error occur when calling .getClusterExpansionZoom() method.

Cannot read property 'x' of undefined

Object.getChildren
index.js:84

The getChildren() method tries to read from origin.x, but origin variable is undefined.
The origin var, as we can see, is created in the following line:

var origin = this.trees[clusterZoom + 1].points[clusterId];

It is undefined because (as in one of my debuggings) clusterZoom = 12 and clusterId = 21, and this.trees[13].points only have 19 entries (0 to 18), thus, this.tree[13].points[21] returns undefined.

Any ideas why is that error occurring? Thanks! =)

@mourner
Copy link
Member

mourner commented Mar 11, 2017

Hmm, hard to say. Can you reproduce this easily? E.g. within a few clicks? If so, can you set up a simple minimal test case? This shouldn't happen randomly because Supercluster is deterministic.

@mourner mourner added the bug label Mar 11, 2017
@mourner mourner self-assigned this Mar 11, 2017
@almirfilho
Copy link
Author

Here @mourner, try to run this. The issue is with the zoom level, I guess.

var supercluster = require('supercluster');
var superCluster = supercluster({radius: 50});
var points = [
  {"properties":null,"geometry":{"coordinates":[-157.738807,21.403821]}},
  {"properties":null,"geometry":{"coordinates":[-157.7339888,21.3993326]}},
  {"properties":null,"geometry":{"coordinates":[-157.734548,21.40033]}},
  {"properties":null,"geometry":{"coordinates":[-157.708681,21.379721]}},
  {"properties":null,"geometry":{"coordinates":[-157.711043,21.385541]}},
  {"properties":null,"geometry":{"coordinates":[-157.7172689,21.393636]}},
  {"properties":null,"geometry":{"coordinates":[-157.7443502,21.4165303]}},
  {"properties":null,"geometry":{"coordinates":[-157.74334,21.413519]}},
  {"properties":null,"geometry":{"coordinates":[-157.709435,21.383292]}},
  {"properties":null,"geometry":{"coordinates":[-157.712649,21.388786]}},
  {"properties":null,"geometry":{"coordinates":[-157.729243,21.398487]}},
  {"properties":null,"geometry":{"coordinates":[-157.729243,21.398487]}},
  {"properties":null,"geometry":{"coordinates":[-157.709189,21.379953]}},
  {"properties":null,"geometry":{"coordinates":[-157.71043,21.384883]}},
  {"properties":null,"geometry":{"coordinates":[-157.709665,21.383786]}},
  {"properties":null,"geometry":{"coordinates":[-157.720091,21.395693]}},
  {"properties":null,"geometry":{"coordinates":[-157.716762,21.3888]}},
  {"properties":null,"geometry":{"coordinates":[-157.711711,21.387149]}},
  {"properties":null,"geometry":{"coordinates":[-157.741526,21.408911]}},
  {"properties":null,"geometry":{"coordinates":[-157.7153218,21.3912362]}},
  {"properties":null,"geometry":{"coordinates":[-157.71704,21.3895139]}},
  {"properties":null,"geometry":{"coordinates":[-157.711032,21.386012]}},
  {"properties":null,"geometry":{"coordinates":[-157.7165599,21.388456]}},
  {"properties":null,"geometry":{"coordinates":[-157.735521,21.399968]}},
  {"properties":null,"geometry":{"coordinates":[-157.7096139,21.381616]}},
  {"properties":null,"geometry":{"coordinates":[-157.764096,21.422697]}},
  {"properties":null,"geometry":{"coordinates":[-157.7447369,21.41656]}},
  {"properties":null,"geometry":{"coordinates":[-157.716935,21.392532]}},
  {"properties":null,"geometry":{"coordinates":[-157.712677,21.38497]}},
  {"properties":null,"geometry":{"coordinates":[-157.713332,21.380847]}},
  {"properties":null,"geometry":{"coordinates":[-157.716933,21.38936]}},
  {"properties":null,"geometry":{"coordinates":[-157.71542,21.3905439]}},
  {"properties":null,"geometry":{"coordinates":[-157.743339,21.41022]}},
  {"properties":null,"geometry":{"coordinates":[-157.7460247,21.4165045]}},
  {"properties":null,"geometry":{"coordinates":[-157.7387108,21.4014343]}},
  {"properties":null,"geometry":{"coordinates":[-157.712387,21.38135]}},
  {"properties":null,"geometry":{"coordinates":[-157.762416,21.4276899]}},
  {"properties":null,"geometry":{"coordinates":[-157.747666,21.414808]}},
  {"properties":null,"geometry":{"coordinates":[-157.739207,21.4247]}},
  {"properties":null,"geometry":{"coordinates":[-157.774142,21.406839]}},
  {"properties":null,"geometry":{"coordinates":[-157.7178999,21.390093]}},
  {"properties":null,"geometry":{"coordinates":[-157.747462,21.389257]}},
  {"properties":null,"geometry":{"coordinates":[-157.744486,21.4108139]}},
  {"properties":null,"geometry":{"coordinates":[-157.71512,21.38752]}},
  {"properties":null,"geometry":{"coordinates":[-157.743692,21.425881]}},
  {"properties":null,"geometry":{"coordinates":[-157.770683,21.412724]}},
  {"properties":null,"geometry":{"coordinates":[-157.7426186,21.4086938]}},
  {"properties":null,"geometry":{"coordinates":[-157.738378,21.426421]}},
  {"properties":null,"geometry":{"coordinates":[-157.722777,21.392695]}},
  {"properties":null,"geometry":{"coordinates":[-157.766212,21.411992]}}
];
superCluster.load(points);

var bbox = [
  -157.78133979848454,
  21.3464786240286,
  -157.66633979563304,
  21.460381588705975
];
var zoom = 12;
var clusters = superCluster.getClusters(bbox, zoom);
var clusterId = clusters[6].properties.cluster_id;

// OK with zoom = 12
console.log(superCluster.getClusterExpansionZoom(clusterId, zoom));
// ERROR with zoom = 11
console.log(superCluster.getClusterExpansionZoom(clusterId, 11));

@mourner
Copy link
Member

mourner commented Mar 13, 2017

Oh, yeah, the zoom provided must match the zoom you're getting the cluster from. Would providing a more meaningful error resolve the issue?

@almirfilho
Copy link
Author

Or, don't you think that it should return the expansion zoom anyway? (13 for that case) @mourner

@mourner
Copy link
Member

mourner commented Mar 14, 2017

No, because clusterId is only valid for a specific zoom value — combined they form a unique identification of a cluster. But I'm thinking about combining them somehow into one number variable to avoid confusion like this and make the API easier.

@mourner mourner closed this as completed Mar 14, 2017
@almirfilho
Copy link
Author

@mourner Agreed. I think tou could use uuids for that. Anyway, I think it would be nice to have this information you just gave me in the Readme, and a better error to avoid confusion of course. Let me know if you need help with that. And thanks again for this package!

@mourner
Copy link
Member

mourner commented Mar 14, 2017

@almirfilho opened an issue for this: #43. I think we should definitely do this because it will make the API simpler (no need to pass zoom).

@myflowpl
Copy link

I run into the same problem.
But in my case i accidentally set the zoom as string, then '14'+1 = '141' instead of 15.
May be we can do something like this:

var origin = this.trees[parseInt(clusterZoom) + 1].points[clusterId];

and help others avoid this kind of silly mistakes :)

@chriscalip
Copy link

For anyone stuck using
"@mapbox/mapbox-gl-supported": "^1.3.0", and have to do "supercluster": "2.3.0"
How to fix?

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

No branches or pull requests

4 participants