Closed seanblonien closed 1 year ago
We're open to it, but currently don't have the bandwidth to do it ourselves.
@tylermcginnis apologies for not doing this when I opened up the issue, but I believe this library doesn't explicitly use "react-dom" dependency at all -- look at this search for react-dom
in Github search for this repo, and it seems the there is not a single react-dom
import, which is a great start
...but there is an implicit dependency on DOM elements, such as window
. I mean yes, having the react-dom
as a peer dependency does make sense and addresses the required window dependency by indicating these hooks require DOM features, because react-dom
does require window
, so if you use react-dom
, you're guaranteed to have the window available. BUT yea idk, it just seems weird to rely on global objects outside of the specific react-dom
package which is never explicitly used, just seems like a confusing communication of it being a peer-dependency
REGARDLESS - I will put my purity concerns aside and talk about where this could go from here
@uidotdev/usehooks
into separate packages for pure ("core" maybe) and DOM packages
@uidotdev/usehooks-core
that the DOM package consumeswent ahead and did some leg work for this: | Hooks | Compatibility |
---|---|---|
useBattery | DOM | |
useClickAway | DOM | |
useCopyToClipboard | DOM | |
useCounter | pure | |
useDebounce | pure | |
useDefault | pure | |
useDocumentTitle | DOM | |
useFavicon | DOM | |
useGeolocation | DOM | |
useHistoryState | pure | |
useHover | DOM | |
useIdle | DOM | |
useIntersectionObserver | DOM | |
useIsClient | DOM | |
useIsFirstRender | pure | |
useList | pure | |
useLockBodyScroll | DOM | |
useLongPress | DOM | |
useMap | pure | |
useMeasure | DOM | |
useMediaQuery | DOM | |
useMouse | DOM | |
useNetworkState | DOM | |
useObjectState | pure | |
useOrientation | DOM | |
usePreferredLanguage | DOM | |
usePrevious | pure | |
useQueue | pure | |
useRenderCount | pure | |
useRenderInfo | Node | |
useScript | DOM | |
useSet | pure | |
useSpeech | DOM | |
useThrottle | pure | |
useToggle | pure | |
useVisibilityChange | DOM | |
useWindowScroll | DOM | |
useWindowSize | DOM |
also unsure why useRenderInfo
requires node's process
global, so just thought i'd mention it here when I was looking at the hooks (I can create another issue for this weird node dependency if this is an actual bug)
unfortunately, I don't have the time commitment to do this myself, and also quite frankly this is such a large thing I'd imagine it's a feature for a core maintainer anyways. so yea, happy to give ideas, but that's about where my commitment stops in moving this forward
This is really helpful. I'll dive into it soon.
Going to close this again to keep the issues clean, but will refer back to this once I dive into RN support.
Just curious to know if there are any plans to make this
react-native
or rather notreact-dom
dependent one day.