visgl / react-map-gl

React friendly API wrapper around MapboxGL JS
http://visgl.github.io/react-map-gl/
Other
7.88k stars 1.35k forks source link

[Feat] better console.log behavior for events #2243

Open logemann opened 1 year ago

logemann commented 1 year ago

Target Use Case

As a first time user with react-map-gl, when using a map with interactiveLayerIds=[SomeLayer..id] and doing console.log(event) in the onClick EventHandler, you see the following output in console:

Bildschirmfoto 2023-07-23 um 23 10 53

Now you start wondering where the crucial "features" property is and try to fix things in your code. Of course nothing will change whatever you do .Then, after copy/pasting an official example (cluster example) into my codebase and also doing console.log(), you see that its the same there too. Now you realize, that console.log() just doesnt print out the "features" property. The only way to print out this property is the following:

console.log(Object.getOwnPropertyNames(event));

Which gets you this:

Bildschirmfoto 2023-07-23 um 23 21 38

So you fought an error which wasnt one in the first place and thats kind of horrible.

Proposal

Dont know a proposal. Would like to see console.log include this important property when its there. Not a JS pro so i cant even figure out why its not there in the first place. Perhaps something todo with the js prototype property?

What would have helped is a hint in the docs for the interactiveLayerIds=[SomeLayer..id] property so that developers dont search for the resulting "features" property in the event via console.log() which is a quite normal quck and dirty way to inspect stuff. After this, most likely i will start sooner firing up a debugger with a breakpoint

Appendix: thanks for this great library.

tordans commented 1 year ago

I agree, this is confusing.

On documentation:

The features are mentioned in https://visgl.github.io/react-map-gl/docs/api-reference/map#onclick. I wonder if an additional note on the console log topic should be added there.

On the issue:

I ran into a similar issue with queryRenderedFeatures which looked like it did not have a geometry field (console log output in this outdated issue https://github.com/charliedotau/mapbox-gl-js-select-features-by-draw/issues/1). I asked about it in the Maplibre Slack https://osmus.slack.com/archives/C01G3D28DAB/p1674586300777679 and though I learned that the issue there was, that geometry is a getter which is console-logged differently:

Harel Mazor vor 9 Monaten This is a getter which is calculated and cached to improve performance. It's not an actual property, it just looks like so. This is one of the reasons I hardly use getters and setters - I prefer a method called "getGeometry()" to indicate that this is not a simple property...

However, now I am confused again, because the features do not show up half-transparent when logging the event… 🤔 .

Possible solution:

Following Harels comment in Slack, maybe this should be a more explicit event.getFeatures()? Something to introduce in a minor release and then slowly deprecating the old interface in the next major release?

lswainemoore commented 9 months ago

Good grief. This got me too for a few hours--actually until I just saw this. It doesn't help that the clusters example is actually broken (see here). I think a note in the documentation would be very welcome.