Skip to content
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

Loosen WebElementCondition instance checks #5968

Merged
merged 2 commits into from
Jun 1, 2018

Conversation

afsmith92
Copy link
Contributor

Update WebElementCondition and Condition with custom
static hasInstance methods so that instanceof checks
are more lenient and don't cause issues with Babel.

Fixes #5560

Update WebElementCondition and Condition with custom
static hasInstance methods so that `instanceof` checks
are more lenient and don't cause issues with Babel.

Fixes SeleniumHQ#5560
* @param {!(IThenable<T>|
* Condition<T>|
* function(!WebDriver): T)} condition - the condition instance
* @returns {*|boolean}
Copy link
Contributor

Choose a reason for hiding this comment

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

@return {boolean}

* @returns {*|boolean}
*/
static [Symbol.hasInstance](condition) {
return condition
Copy link
Contributor

Choose a reason for hiding this comment

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

return !!condition

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, I'd probably be a little stricter:

return !!condition && typeof condition === 'object'

(plus the next two lines)

* function(!WebDriver): T)} condition - the condition instance
* @returns {*|boolean}
*/
static [Symbol.hasInstance](condition) {
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to define this a second time, it will be inherited from Condition

@afsmith92 afsmith92 closed this Jun 1, 2018
@afsmith92 afsmith92 reopened this Jun 1, 2018
@jleyba jleyba merged commit 5ec8094 into SeleniumHQ:master Jun 1, 2018
@afsmith92 afsmith92 deleted the WebElementCondition-fix branch January 17, 2019 00:04
barancev added a commit that referenced this pull request Jan 30, 2019
This reverts commit 5ec8094.

This commit was intended to fix a babel-related issue #5560.

Unfortunately it broke 'x instanceof WebElementCondition' check
that returns true even if 'x' is an instance of the 'Condition' class
(a base class for 'WebElementCondition'). All waits that return
a boolean value or a string are affected.
@barancev
Copy link
Member

I've reverted this change by commit 6158311 because it broke x instanceof WebElementCondition check that returns true even if x is an instance of the Condition class (a base class for WebElementCondition). All waits that return a boolean value or a string are affected.

The issue will be reopened too.

shs96c pushed a commit to shs96c/selenium that referenced this pull request Feb 18, 2019
This reverts commit 5ec8094.

This commit was intended to fix a babel-related issue SeleniumHQ#5560.

Unfortunately it broke 'x instanceof WebElementCondition' check
that returns true even if 'x' is an instance of the 'Condition' class
(a base class for 'WebElementCondition'). All waits that return
a boolean value or a string are affected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants