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

react-map-gl doesn't render when autobind acts on ProxyComponent #461

Closed
kunal-mandalia opened this issue Feb 13, 2018 · 7 comments
Closed
Labels

Comments

@kunal-mandalia
Copy link

Given certain react/webpack/hmr setups, components may be treated as Proxy Components.
When the autobind method is invoked in the component constructor e.g. in static-map.js the this argument isn't at the right level of abstraction for methods to bind to - since it's a Proxy Component, you must go to its prototype to see all method names (and then bind using them).

A work around is to check if the obj in autobind has a prototype of pure component (or component) - then we've reached the right level on the proto chain, e.g:

ProxyComponent -> InteractiveMap -> PureComponent

OOTB create-react-app works fine but we've ejected and have custom babel/webpack config causing the Proxy Component during autobind. Unfortunately we're on a monolithic proprietary codebase so I won't be able to share the source generating the issue but I do see that e.g. proxy components are associated with hot module reload.

Not sure what the best way to move forward on this is, I'm hoping you could suggest an idea. Thanks for your work in putting this good library together.

@kunal-mandalia
Copy link
Author

kunal-mandalia commented Feb 13, 2018

Note that manually binding each method in the constructor the old fashioned way fixes the issue - proxy components or not:
this.methodName = this.methodName.bind(this)

I'd be happy to submit a PR doing this across the src components, let me know if that's something you'd want.

@kunal-mandalia
Copy link
Author

kunal-mandalia commented Feb 14, 2018

The issue was with react-hot-loader, see gaearon/react-hot-loader#858. We disabled it and the map renders. It'd be good to see react-mapbox-gl and react-hot-loader play nice.

@ibgreen
Copy link
Contributor

ibgreen commented Feb 14, 2018

@balthazar - I suppose we could revert to manually binding the callbacks and drop our autobind helper.

@balthazar
Copy link
Contributor

Wow this is pretty bad, weirdly didn't have the issue with my personal webpack + hot, but I guess that's a combinaison of things.

@kunal-mandalia What about class properties? I would guess they should work, and are way cleaner than a constructor with method redefinition.

@theKashey
Copy link

PS: you can just replace your autobind with autobind-decorator(which could be just a HOC, not a decorator).
It works a bit differently, binding methods when you access them, and thus stable.

@kunal-mandalia
Copy link
Author

@balthazar I should think they'd work but I haven't tried them myself. The particular issue we were having with react-hot-loader been solved by @theKashey 's merge above (thanks dude!) so if you're confident that no one else is proxying components like hot loader you can consider closing this. Your quick response didn't go unnoticed when I raised it with our CEO, thanks.

@Pessimistress
Copy link
Collaborator

Moved to explicit binding in 3.2.5

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

5 participants