search

mobxjs / mobx-react-lite

Lightweight React bindings for MobX based on React 16.8 and Hooks
https://mobx.js.org/react-integration.html
MIT License
2.13k stars 91 forks source link

fix(types): deprecated RefForwardingComponent usage #279

Closed nulladdict closed 3 years ago

nulladdict commented 4 years ago

Update react and react-dom types Use ForwardRefRenderFunction instead of RefForwardingComponent

Closes #263

nulladdict commented 4 years ago

I also ran prettier and fixed a small typo

There were no need to add/change tests, since the test present already check observer usage with and without forwardRef option

I did however had to change the base case typings, since TS would complain that baseComponent: React.FunctionComponent<P> overload is not compatible with the implementation

danielkcz commented 4 years ago

I don't think we should do this yet as it would block people with older versions of types package. As long as RefForwardingComponent exists, we should stick to it.

Note that you only changed versions of DEV dependencies. We cannot enforce a specific version to consumers because of global types.

Edit: I wonder if TS has something like a || b similar to JS so we can fallback if the new one not found, but I never heard of such an option.

danielkcz commented 4 years ago

Hm, I see now I wrote a comment that PR fixing deprecation is welcome. My bad, that might have been premature. I am not sure how old that change is.

Thanks for a contribution anyway. Let's leave this open for now and when more complaints about the wrong Ref typing pile up, we can push this through.

nulladdict commented 4 years ago

I think the changes dates back to Feb 20 according to this PR, so it's still pretty fresh and I don't mind leaving it on hold for now

Also, I don't think TS has anything to "fix" non-existing types other than typesVersions property, which unfortunately relies on TS version

danielkcz commented 3 years ago

This PR has conflicts and I will soon move the codebase to mobx repo (https://github.com/mobxjs/mobx/issues/2530). Either way, I am still not sure we are ready to do this since the deprecated type is still available in the latest types package version.