React v16.0 & Popover issue

[PlanetIrata]

Hi, I just upgraded my app to React v16.0, everything is working fine but the component do not render anymore, here is the error reported in the console:

index.js:199 Uncaught TypeError: Failed to execute 'getComputedStyle' on 'Window': 
parameter 1 is not of type 'Element'.
    at Object.resolvePopoverLayout (index.js:199)
    at Object.trackPopover (index.js:429)
    at Object.enter (index.js:328)
    at Object.componentDidUpdate (index.js:153)
    at Object.chainedFunction [as componentDidUpdate] (factory.js:615)
    at commitLifeCycles (react-dom.development.js:11517)
    at commitAllLifeCycles (react-dom.development.js:12294)
    at HTMLUnknownElement.callCallback (react-dom.development.js:1299)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:1338)
    at invokeGuardedCallback (react-dom.development.js:1195)
    at commitAllWork (react-dom.development.js:12415)
    at workLoop (react-dom.development.js:12687)
    at HTMLUnknownElement.callCallback (react-dom.development.js:1299)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:1338)
    at invokeGuardedCallback (react-dom.development.js:1195)
    at performWork (react-dom.development.js:12800)
    at batchedUpdates (react-dom.development.js:13244)
    at performFiberBatchedUpdates (react-dom.development.js:1646)
    at stackBatchedUpdates (react-dom.development.js:1637)
    at batchedUpdates (react-dom.development.js:1651)
    at Object.batchedUpdatesWithControlledComponents [as batchedUpdates] (react-dom.development.js:1664)
    at dispatchEvent (react-dom.development.js:1874)

Thanks for help

[knowbody]

I also tried upgrading to v16 and the blocker for me is:

https://github.com/littlebits/react-popover/blob/334011898aa452233a8f7a9e2b52e5842a424b56/lib/react-layer-mixin.js#L47

[jasonkuhrt]

Thanks for reporting. I'll look into fixing this this week. PR's welcome.

[PlanetIrata]

Thanks @jasonkuhrt for taking time to fix this, the issue I encouter may be related to changes introduced in componentDidUpdate calls & lifecycle which are detailed in the Breaking changes of React v16.0

[knowbody]

actually #129 fixes my issue so I'll wait for the next release

[jasonkuhrt]

@knowbody that release has been made so check it out.

[knowbody]

Thank you!

…

On Fri, 29 Sep 2017 at 01:34, Jason Rose-Kuhrt ***@***.***> wrote: @knowbody <https://github.com/knowbody> that release has been made so check it out. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#135 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADoDp3O_W6J95HE23xNsEfG4xjNAQVvcks5snDr4gaJpZM4PlBm6> .

[jebarjonet]

This is fixing knowbody issue but PlanetIrata one is still occuring to me too since I switched to react v16

[jasonkuhrt]

I started work on transitioning to React 16 last night. It will be a big but good change. For instance no more custom portal implementation to maintain. That said I'm ramping up so not sure how long this is going to take. I'm hoping that a few nights and bus rides will be enough. I'll make a WIP PR soon so I have a place to put comments and progress can be better tracked.

[knowbody]

@jasonkuhrt thanks a lot, I started looking into it this morning too, happy to help on it. Do you want to commit your work so we can put the forces together?

Also how happy would you be to remove the mixin and createClass and move to ES6?

also would you make that upgrade a breaking change and make a 1.0.0 release or would you want to support previous versions of React as well?

Here are some useful resources:

• React Modal, moving to React 16: React16 reactjs/react-modal#513
• here the breaking change to unstable_renderIntoContainer() solution: ReactDOM.render()/unstable_renderIntoContainer() doesn't return instance if called during an update facebook/react#10309 (comment)

[jasonkuhrt]

This is will be a breaking change going to 0.5. 1.0 will be released minimum once Forto is integrated, using flowtype, ... A checklist will be made for the community to help decide what features go into it (ideally).

In regards to backwards compatibility or support (officially), I don't have the bandwidth for that so this will be a line in the sand for users. I think the incentive to upgrade to React 16 is strong enough that most users will probably be ok with this fact.

[jasonkuhrt]

@knowbody Just saw your comment changes.

Also how happy would you be to remove the mixin and createClass and move to ES6?

Yes absolutely.

Here are some useful resources:

Thanks!

Do you want to commit your work so we can put the forces together?

Sounds good to me. I'll try to publish a branch tonight.

[jasonkuhrt]

I'll be cutting a release tonight unless an issue is found. Looking alright so far though.