maslianok / react-resize-detector

A Cross-Browser, Event-based, Element Resize Detection for React
http://maslianok.github.io/react-resize-detector/
MIT License
1.25k stars 91 forks source link

JFYI: TypeScript #97

Closed Diokuz closed 4 years ago

Diokuz commented 4 years ago

Because of breaking changes in 5, we had an issue in production after automatic update. We failed to prevent it, because we use outdated https://www.npmjs.com/package/@types/react-resize-detector

Consider to use typescript instead of js in this rep.

maslianok commented 4 years ago

Thanks, @Diokuz

It would be great if anyone can help me to update @types/react-resize-detector

jmadankumar commented 4 years ago

@maslianok I can help you with the update of @types/react-resize-detector. As per the documentation we need to remove querySelector, nodeType, and targetDomEl prop types from the typescript definition file. Please confirm whether my understanding is right about the latest changes.

If my understanding is right. Please review the types changes https://github.com/jmadankumar/DefinitelyTyped/commit/bb50fe943fd2a296ae637032836057a97c640f14

maslianok commented 4 years ago

@jmadankumar thanks for your help.

These lines describe all available props https://github.com/maslianok/react-resize-detector/blob/master/src/components/ResizeDetector.js#L200-L217

querySelector, targetDomEl and nodeType are still there. I removed them from the documentation but still keep them in the code to not break any existing logic. I'm not sure whether we should remove it from TS or not. Let's maybe keep them for now? What do you think? (I would vote to remove them in the next major release)

Other than that it looks great! Thanks for your effort 👍

jmadankumar commented 4 years ago

@maslianok As the props are still supported by the library. we should keep the props in the documentation. as well as in the TS. In the next major release, we will remove props from documentation as well as TS.

maslianok commented 4 years ago

Ok, agree. I’ll add the props back and mark them as deprecated.

So now you can create a PR without removing these props

On Wed, 26 Aug 2020 at 20:40, Madan Kumar notifications@github.com wrote:

@maslianok https://github.com/maslianok As the props are still supported by the library. we should keep the props in the documentation. as well as in the TS. In the next major release, we will remove props from documentation as well as TS.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/maslianok/react-resize-detector/issues/97#issuecomment-681025120, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2S6MUJTLKSM4WIK26PPU3SCVCKFANCNFSM4PEQASXQ .

jmadankumar commented 4 years ago

@maslianok I will create PR mark the props deprecated in the TS

jmadankumar commented 4 years ago

@maslianok Created the PR for the types changes https://github.com/DefinitelyTyped/DefinitelyTyped/pull/47098

maslianok commented 4 years ago

Approved! Thank you 👍

jmadankumar commented 4 years ago

@maslianok @types/react-resize-detector v5.0.0 published to NPM