-
-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added host to hooks for instanceof check #100
base: master
Are you sure you want to change the base?
Conversation
Hi @nerocui 👋 First of all: thank you, I really appreciate the effort here! Can you provide a reproduction of your issue in codesandbox, so that I better understand your use-case please? 🙏 |
@ZeeCoder Happy new year. Hope you had a good holiday break. Please let me know if this helps. |
Sorry I didn't come back to you yet, I'm extremely busy with personal stuff right now but I just wanted you to know I appreciate the time you've invested here. use-resize-observer/src/utils/useResolvedElement.ts Lines 30 to 37 in 490429f
I think one solution would be to change this: const element: T | null = cbElement
? cbElement
: refOrElement
? refOrElement instanceof Element
? refOrElement
: refOrElement.current
: null; To this: const element: T | null = cbElement
? cbElement
: refOrElement
? "current" in refOrElement
? refOrElement.current
: refOrElement
: null; ☝️ Basically by not using instanceof at all, we circumvent the issue altogether. Could be a minor release. 🤔 |
Would need an additional test case ofc to ensure there'd be no regressions. |
@ZeeCoder Thank you so much for looking into it! Yes getting rid of interaction with global objects would solve this issue. |
I'll update the PR with the recommendation and test cases. |
Alternatively, using const element =
cbElement ||
(refOrElement
? Object.prototype.hasOwnProperty.call(refOrElement, 'current')
? refOrElement.current
: refOrElement
: null) |
@bpinto looks about right, it's a shame that TS doesn't handle it properly though: |
Sorry @ZeeCoder, I have 0 experience with TS but a google search showed me the following solution: Type assertion since we know it's a ref and not an element/state:
I tested it here |
Hi @ZeeCoder , thanks for creating this library. We are using this package, and we'd like to help make it better. Our use case includes passing in child window's window object.
instanceof
checks the prototype, and that's unique on each window. So to make sureinstanceof
works reliably, we need to pass in the host object and compare to the correctElement
. Default case is the globalwindow
object.Hope you could give this PR a look when you are free. Thank you!