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

Fix: Now correctly allows to override ElementFinder and ElementArrayFinder methods #5328

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

Conversation

Xotabu4
Copy link

@Xotabu4 Xotabu4 commented Oct 7, 2019

Fix: Now correctly allows to override ElementFinder and ElementArrayFinder methods

Monkey-patching was done in ElementFinder constructor against 'this'. It means this monkey-patching is applied AFTER all methods in inherited objects declared - it will just replace them. Here is list of WebElement methods, that is monkey-patched into ElementFinder:
let WEB_ELEMENT_FUNCTIONS = [
'click', 'sendKeys', 'getTagName', 'getCssValue', 'getAttribute', 'getText', 'getRect',
'isEnabled', 'isSelected', 'submit', 'clear', 'isDisplayed', 'getId', 'takeScreenshot'
];

class Component extends ElementFinder {
  constructor(elementFinder: ElementFinder) {
    super(elementFinder.browser_, elementFinder.elementArrayFinder_);
  }

 async click() {
     console.log('OWN click!')
     await super.click()
 }
}

new Component($('button')).click()

Before this PR code above would not be working - since monkey-patching is overrides methods declared in child classes:

Xotabu4/protractor-element-extend#20

@@ -730,6 +719,16 @@ export class ElementArrayFinder extends WebdriverWebElement {
return this.applyAction_(allowAnimationsTestFn);
}
}
// TODO(juliemr): might it be easier to combine this with our docs and just
// wrap each one explicity with its own documentation?
WEB_ELEMENT_FUNCTIONS.forEach((fnName) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

if fnName can be only string you could add this type

// TODO(juliemr): might it be easier to combine this with our docs and just
// wrap each one explicity with its own documentation?
WEB_ELEMENT_FUNCTIONS.forEach((fnName) => {
ElementArrayFinder.prototype[fnName] = function (...args: any[]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

do you need here () as you did for ElementFinder ((ElementFinder.prototype[fnName]))?

@CrispusDH
Copy link
Contributor

@cnishina could you take a look at it useful PR? :)

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

Successfully merging this pull request may close these issues.

None yet

3 participants