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

Using react-reactonlickoutside with React.forwardRef results in a runtime error #297

Open
gaborsar opened this issue Nov 15, 2018 · 8 comments

Comments

@gaborsar
Copy link

gaborsar commented Nov 15, 2018

Using the following component that uses React.forwardRef:

const TestComponent = onClickOutside(
    React.forwardRef((props, ref) => <div {...props} ref={ref} />)
);

Leads to the following error:

Uncaught TypeError: Cannot read property 'isReactComponent' of undefined
    at onClickOutside.render (react-onclickoutside.es.js:329)
    at finishClassComponent (react-dom.development.js:13727)
    at updateClassComponent (react-dom.development.js:13690)
    at beginWork (react-dom.development.js:14489)
    at performUnitOfWork (react-dom.development.js:17014)
    at workLoop (react-dom.development.js:17054)
    at HTMLUnknownElement.callCallback (react-dom.development.js:149)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:199)
    at invokeGuardedCallback (react-dom.development.js:256)
    at replayUnitOfWork (react-dom.development.js:16366)

As styled-components uses React.forwardRef, react-onclickoutside cannot be directly used with styled components.
Related to: #293

@Pomax
Copy link
Owner

Pomax commented Nov 24, 2018

I am very glad they finally added this as a thing, because it solves a problem that is inherent to HOC implementations. That said, this is absolutely worth updating the code for, and then releasing it as a new major version that does away with the whole instance forwarding code this HOC has had to use for years now.

As per https://reactjs.org/docs/forwarding-refs.html#note-for-component-library-maintainers - let's use this! And mark it as breaking change.

/cc @Andarist

@Andarist
Copy link
Collaborator

Sure - doing a major release for this is cool 👍

@ozluy
Copy link

ozluy commented Apr 24, 2019

Any update on that?

@arthur-cen
Copy link

Any updates on this fix?

@kevalbhatt
Copy link

@Andarist could you please update your PR

@adarrra
Copy link

adarrra commented Jun 18, 2019

is there any workaround now to use with styled-components?

@NilSet
Copy link

NilSet commented Jun 30, 2019

#326 should be able to work with forwardRef without a breaking change

@Pomax
Copy link
Owner

Pomax commented Aug 11, 2019

I've not had any free time to look at this before today, but I've left some questions on that PR - there's some odd things going on in it and I'd like to make sure those are all intentional.

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 a pull request may close this issue.

8 participants