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? :)

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants