Upgrade React to 16.9

[danielkcz]

Something has changed in React that's causing one test to fail. I've spent quite some time to figure it out, but I have no idea. I've extracted the failing portion to sandbox and hoping for an extra pair of eyes to see to the problem...

https://codesandbox.io/s/little-fire-2k5ow?expanddevtools=1&fontsize=14&module=%2Fsrc%2Ferror.test.tsx&previewwindow=tests

I tried changing the following line to createElement(component, props) but it breaks a whole lot of other tests simply because useObserver cannot see into a nested component.

mobx-react-lite/test/observer.test.tsx

Line 22 in 9e80618

return component(props)

cc @mweststrate @urugator @xaviergonz

Fixes #195 #196 #198

[mweststrate]

Sorry, I've been really swamped by our migration lately, let me know if you're still stuck and I'll try to find some time next week

…

On Fri, Aug 9, 2019 at 6:33 PM Daniel K. ***@***.***> wrote: Something has changed in React that's causing one test to fail. I've just spent quite some time to figure it out, but I have no idea. I've extracted the failing portion to sandbox and hoping for an extra pair of eyes to see to the problem... https://codesandbox.io/s/little-fire-2k5ow?fontsize=14&module=%2Fsrc%2Ferror.test.tsx&previewwindow=tests I tried changing the following line to createElement(component, props) but it breaks a whole lot of other tests simply because useObserver cannot see into a nested component. mobx-react-lite/test/observer.test.tsx <https://github.com/mobxjs/mobx-react-lite/blob/9e8061861310c823e788ab7e1b22c6f635ba7d52/test/observer.test.tsx#L22> Line 22 in 9e80618 <http:///mobxjs/mobx-react-lite/commit/9e8061861310c823e788ab7e1b22c6f635ba7d52> return component(props) cc @mweststrate <https://github.com/mweststrate> @urugator <https://github.com/urugator> @xaviergonz <https://github.com/xaviergonz> Fixes #195 <#195> #196 <#196> #198 <#198> ------------------------------ You can view, comment on, or merge this pull request online at: #199 Commit Summary - Upgrade React to 16.9 File Changes - *M* package.json <https://github.com/mobxjs/mobx-react-lite/pull/199/files#diff-0> (10) - *M* yarn.lock <https://github.com/mobxjs/mobx-react-lite/pull/199/files#diff-1> (86) Patch Links: - https://github.com/mobxjs/mobx-react-lite/pull/199.patch - https://github.com/mobxjs/mobx-react-lite/pull/199.diff — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#199?email_source=notifications&email_token=AAN4NBGEDBODFXPSHONHF4LQDWL5FA5CNFSM4IKVSVT2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HEOBWWA>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAN4NBAE6WUMQMJ2HALIOWTQDWL5FANCNFSM4IKVSVTQ> .

[danielkcz]

Yep, still stuck, no idea what's the problem here.

[danielkcz]

Ok, I had a fresh new look at this and something really interesting/strange is happening here. Feels like some partial concurrent mode got into 16.9 or whatnot.

What happens is when an error is thrown from the observed component, it is caught for reaction disposal and then rethrown again - that is fine.

mobx-react-lite/src/useObserver.ts

Line 60 in cc2b30f

dispose()

However, React does another re-render afterward where Reaction is already disposed so the rendering is skipped and ultimately it returns undefined as the initial value of rendering variable.

I will try to isolate this to pure React and possibly report an issue with them.

[danielkcz]

Aha! It seems when I use the code for useObserver from the next branch, it actually passes a test with React 16.9. It's indeed concurrent more related.

So I am thinking we will expedite the release of 2.0 as per #121 (comment) there might be potentially breaking changes. The 1.x will stick to 16.8.

I wonder though if we need to publish mobx-react@7 as well. It's really hard to guess how breaking that change is. I will also remove those deprecated hooks, but they were never reexported in mobx-react.