Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Added host to hooks for instanceof check#100

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
nerocui wants to merge1 commit intoZeeCoder:master
base:master
Choose a base branch
Loading
fromnerocui:dev

Conversation

@nerocui
Copy link

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!

@ZeeCoder
Copy link
Owner

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? 🙏

@nerocui
Copy link
Author

nerocui commentedJan 3, 2023
edited
Loading

@ZeeCoder Happy new year. Hope you had a good holiday break.
I was not able to get it to work with codesandbox, but I uploaded a minimal repro to herehttps://github.com/nerocui/host-repro
It's just a express app that host two static html pages. Essentially it's just showing thatinstanceof returns false if an element is created from a different window. So in order to getinstanceof to work reliably, it needs to be called with the type from the host window.

Please let me know if this helps.

@ZeeCoder
Copy link
Owner

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.
Seems like the issue stems from useResolvedElement's use of instanceof here:

// Ugly ternary. But smaller than an if-else block.
constelement:T|null=cbElement
?cbElement
:refOrElement
?refOrElementinstanceofElement
?refOrElement
:refOrElement.current
:null;

I think one solution would be to change this:

constelement:T|null=cbElement      ?cbElement      :refOrElement      ?refOrElementinstanceofElement        ?refOrElement        :refOrElement.current      :null;

To this:

constelement:T|null=cbElement      ?cbElement      :refOrElement      ?"current"inrefOrElement        ?refOrElement.current        :refOrElement      :null;

☝️ Basically by not using instanceof at all, we circumvent the issue altogether.

Could be a minor release. 🤔

@ZeeCoderZeeCoder added the enhancementNew feature or request labelFeb 18, 2023
@ZeeCoder
Copy link
Owner

Would need an additional test case ofc to ensure there'd be no regressions.
Which is where most of the work lies tbh.

@nerocui
Copy link
Author

@ZeeCoder Thank you so much for looking into it! Yes getting rid of interaction with global objects would solve this issue.

@nerocui
Copy link
Author

I'll update the PR with the recommendation and test cases.

@bpinto
Copy link

bpinto commentedFeb 22, 2023
edited
Loading

Alternatively, usinghasOwnProperty:

constelement=cbElement||(refOrElement        ?Object.prototype.hasOwnProperty.call(refOrElement,'current')          ?refOrElement.current          :refOrElement        :null)

@ZeeCoder
Copy link
Owner

@bpinto looks about right, it's a shame that TS doesn't handle it properly though:
image
I mean there must be a reason why this isn't working. 🤔

bpinto reacted with confused emoji

@bpinto
Copy link

bpinto commentedFeb 26, 2023
edited
Loading

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:

const element =  cbElement ||  (refOrElement    ? Object.prototype.hasOwnProperty.call(refOrElement, 'current')      ? (refOrElement as RefObject<T>).current      : refOrElement as T    : null)

I tested ithere

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

enhancementNew feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nerocui@ZeeCoder@bpinto

[8]ページ先頭

©2009-2025 Movatter.jp