Skip to content
This repository has been archived by the owner on Jul 29, 2019. It is now read-only.

GWT interoperability / usage of typeof instead instanceof #4098

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

daRoof
Copy link

@daRoof daRoof commented Aug 29, 2018

When using the timeline from GWT, type determination for provided functions via the instanceof operator does not work.
Changed setOptions method to use typeof instead instanceof.

When using the timeline from GWT, type determination for provided functions via the instanceof operator does not work.
Changed from instanceof to typeof and everything is okay.
@yotamberk
Copy link
Contributor

Thank @daRoof !
This is not the only use-case of using instanceof.
Is there any reason for changing only this function to use typeof instead of instanceof?

In any case:

I've now branched out to a new fork of my own including only Timeline.
I've applied your change here: https://github.com/yotamberk/timeline-plus
and can be used via npm here: https://www.npmjs.com/package/timeline-plus
Awesome fix! Thanks for your contribution! Future PRs will be reviewed and added in the new fork

@daRoof
Copy link
Author

daRoof commented Sep 11, 2018

I am writing a GWT wrapper for the timeline component. Until now, there has only been this one occurance of instanceof that I had problems with.

As far as I understand, the problem with instanceof is caused by complex objects (a function in this case) passed from GWT code to the timeline. I think this happens only when setting options on the timeline.
But I have not included all features of the timeline, yet. So there may be further pull requests from me.

@mojoaxel mojoaxel self-requested a review January 30, 2019 19:57
Copy link
Member

@mojoaxel mojoaxel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants