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 breaks when RHL is in place #858

Closed
elado opened this issue Feb 14, 2018 · 5 comments
Closed

react-map-gl breaks when RHL is in place #858

elado opened this issue Feb 14, 2018 · 5 comments
Assignees
Labels

Comments

@elado
Copy link

elado commented Feb 14, 2018

Description

Having react-map-gl in a project with RHL, throws errors.

Here's a simple repo
https://github.com/elado/bug-react-hot-loader-react-map-gl

Error is related to bound methods and the way the proxy is applied:

Uncaught TypeError: Cannot set property '_mapboxMap' of undefined
    at _mapboxMapLoaded (static-map.js:216)
    at commitAttachRef (react-dom.development.js:8836)
    at commitAllLifeCycles (react-dom.development.js:9951)
    at HTMLUnknownElement.callCallback (react-dom.development.js:542)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:581)
    at invokeGuardedCallback (react-dom.development.js:438)
    at commitRoot (react-dom.development.js:10050)
    at performWorkOnRoot (react-dom.development.js:11017)
    at performWork (react-dom.development.js:10967)
    at requestWork (react-dom.development.js:10878)
    at scheduleWorkImpl (react-dom.development.js:10732)
    at scheduleWork (react-dom.development.js:10689)
    at scheduleTopLevelUpdate (react-dom.development.js:11193)
    at Object.updateContainer (react-dom.development.js:11231)
    at react-dom.development.js:15226
    at Object.unbatchedUpdates (react-dom.development.js:11102)
    at renderSubtreeIntoContainer (react-dom.development.js:15225)
    at Object.render (react-dom.development.js:15290)
    at Object../src/index.js (index.js:7)
    at __webpack_require__ (bootstrap f8f3573aa0535663a8b1:678)

This seems to be related to autobind
https://github.com/uber/react-map-gl/blob/master/src/components/static-map.js#L79
https://github.com/uber/react-map-gl/blob/b50de38ab0dfed9e976e1d2e6b0c040629bd82b5/src/utils/autobind.js

But in any case, the expectation is that RHL won't break the code.

Environment

Latest create-react-app - react 16.2, react-hot-loader 4.0.0-beta.22, react-map-gl ^3.2.4

Reproducible Demo

https://github.com/elado/bug-react-hot-loader-react-map-gl

@theKashey theKashey self-assigned this Feb 14, 2018
@theKashey theKashey added the WIP label Feb 14, 2018
@theKashey
Copy link
Collaborator

Autobind uses .getPrototypeOf to get methods, and then hooks them. In this case autobind will hook into ProxyComponent's prototype, missing all the "real" handlers.
To say the truth - the used autobind is just far from perfect ("standard" @autoBind decorator in examples do the same better), but React-hot-loader should mitigate this side effect.

@kunal-mandalia
Copy link

+1 we've had to disable react-hot-loader as a result. I flagged this issue with react-mapbox-gl at visgl/react-map-gl#461

@elado
Copy link
Author

elado commented Feb 14, 2018

@theKashey, don't know much about the current architecture, so a question - does RHL have to wrap dependencies with Proxy? would it make sense/be possible to exclude it from touching anything in node_modules?

@theKashey
Copy link
Collaborator

This is not related to React-Hot-Loader, this is more related to the React or even Javascript itself.
React-Hot-Loader will inherit from and stand-for all the original components, to be able to "swap" realisation on hot-load, and present to React a single version of a class.
It even replaces stateless functional components by statefull class-based ones 🎉

Meanwhile, I've got it sorted, going to open PR in React-Hot-Loader in an hour. Just have to write some tests beforehand.

gregberge pushed a commit that referenced this issue Feb 18, 2018
* safe define displayName, fix #845

* stand children did not exists, fix #843

* improve TS documentation

* add SSR+HRM example

* subrender - fix deferred updates

* transfer original class methods into the Proxy prototype, fixes #858

* fix tests
@gregberge
Copy link
Collaborator

Should be fixed in beta.23.

This issue was closed.
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