Skip to content

Conversation

@perliedman
Copy link
Member

We store feature properties in our layers for use when a layer is interactive and/or can change feature styles after they're loaded. However, when this feature isn't enabled, storing the properties is unnecessary and can take up a lot of memory.

This PR makes sure properties are only saved when we actually need them.

@jkuebart
Copy link
Contributor

jkuebart commented Feb 15, 2017

Note that other than through getFeatureId(), properties on layers are also visible to event handlers such as click. Only making them available if getFeatureId() is specified might break some client code.

Maybe the condition options.getFeatureId || options.interactive might be strong enough?

@jkuebart
Copy link
Contributor

Adding some unit tests for this project might be worthwhile, and I'd be willing to write one for my assertion above, but I'm afraid setting up the framework from scratch is too daunting to me…

@perliedman
Copy link
Member Author

You're right, should check interactive as well. It sort of hightlights that the interactive and getFeatureId aren't 100% orthogonal: I have a hard time coming up with a use case where you would set interactive: true but not supply getFeatureId.

@jkuebart
Copy link
Contributor

In my case, clicking a feature triggers an update in a separate window without the need for any feedback in the map itself. I can work out which feature was clicked from its properties, but since I don't have a need for setFeatureStyle() I don't supply getFeatureId.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants