-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
composite layer: pass extension props through #3307
Conversation
newProps[key] = propValue; | ||
if (propDef && propDef.type === 'accessor') { | ||
newProps.updateTriggers[key] = this.props.updateTriggers[key]; | ||
// Unwrap sublayer objects before passing to accessor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just my 2 cents:
This adds a "hidden" unwrapObject
method to some layers which feels a little iffy (Layer is already complex and contains a ton of methods some of which are ripe for cleanup).
The comment here didn't help me understand what "unwrap" means or why it is needed. Also could be valuable to point out if this just a hack for extensions or something more generically valuable for CompositeLayers? Should this be documented at some point? At least add some form of TODO?
@@ -100,6 +100,10 @@ export default class PolygonLayer extends CompositeLayer { | |||
}); | |||
} | |||
|
|||
unwrapObject(object) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason this one in particular looks rather hacky...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is. I'll clean up in a follow up PR.
Follow up of #3305
Change List