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

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 subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants