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

Can't return an ElementFinder from an async function #3831

Closed
massimocode opened this issue Dec 15, 2016 · 10 comments
Closed

Can't return an ElementFinder from an async function #3831

massimocode opened this issue Dec 15, 2016 · 10 comments

Comments

@massimocode
Copy link
Contributor

massimocode commented Dec 15, 2016

The ElementFinder may have a .then member. If it does, functions passed to the .then member will be passed a value of type any. The current type annotations for this member do not accurately represent this and cause errors when using async/await.

The errors can be seen by pasting the following code into a TypeScript file:

import { browser, by, element, ElementFinder, ExpectedConditions } from "protractor";

// The ElementFinder we return here is just an ordinary element
// ERROR: [ts] An async function or method must have a valid awaitable return type.
async function getElementWhenVisible(): Promise<ElementFinder> {
    let myElement = element(by.css(".foo"));
    await browser.wait(ExpectedConditions.visibilityOf(myElement));
    // ERROR: [ts] Return expression in async function does not have a valid callable 'then' member.
    return myElement;
}

// elementWithActionResult is an ElementFinder whose parent ElementArrayFinder
// has action results and therefore has a then function. This type of usage does not
// have any issues and compiles fine.
function getActionResultOldSchool(elementWithActionResult: ElementFinder): PromiseLike<string> {
    return elementWithActionResult.then(someValue => {
        return someValue;
    });
}

// elementWithActionResult is an ElementFinder whose parent ElementArrayFinder
// has action results and therefore has a then function
async function getActionResultAsync(elementWithActionResult: ElementFinder): Promise<string> {
    // ERROR: [ts] Operand for await does not have a valid callable 'then' member.
    return await elementWithActionResult as any;
}```
@juliemr
Copy link
Member

juliemr commented Dec 19, 2016

Thanks for investigating. Historically, we removed the then function on elements because it prevented bad use cases where there was nothing to resolve to. This might be a valid reason for returning it though - or at least messing with the typing.

@massimocode
Copy link
Contributor Author

Just to add some clarification for anyone reading this:

https://github.com/angular/protractor/blob/master/lib/element.ts#L808 shows that the then function is optional (may or may not be there, and hence should be declared as then?). However, https://github.com/angular/protractor/blob/master/lib/element.ts#L795 declares it as always being there.

Every async function undergoes a transpilation process (whether at transpilation time or at runtime on supported engines) so that it returns a promise. The return value of the function is what is used to resolve that promise.

If a "standard" ElementFinder (one that does not have a then function on it) is returned from an async function, then the promise returned by the async function will be resolved with a reference to that ElementFinder. See getElementWhenVisible() in the example above which currently fails to compile.

However, if the promise implicitly returned by the async function is resolved with a thenable (i.e. we returned an ElementFinder which had a then function) then the promise returned by the async function will be resolved with the value which would be given to a callback passed to the thenable (which in this case is ElementFinder.then, representing the action result of the parent ElementArrayFinder). An example of this is in getActionResultAsync() in the example above.

I've submitted a PR with what I believe is the correct typing for the then function:
#3835

I submitted this PR on the basis that it would be incorrect to leave the then function declared as always being there (i.e. not suffixing with ?) because this doesn't reflect the runtime reality.

I should note that it may be possible to have a subclass of ElementFinder which has the then function declared as always being there (possibly even a generic class which types the return value of then), and removing then altogether from the normal ElementFinder, but this would definitely involve changing a quite a bit more code.

@massimocode
Copy link
Contributor Author

@juliemr I think this issue can be closed as the PR was merged :)

@awaisraad
Copy link

awaisraad commented Aug 17, 2020

@juliemr @massimocode This PR #3835 did not fix the issue. Typescript still complains "An async function or method must have a valid awaitable return type."

@Fuun347
Copy link

Fuun347 commented Aug 17, 2020

@awaisraad the PR did fix it at the time making the the then function optional. Since then in typescript 3.9 they've added stricter checks to optional properties. This is the reason why you're getting this issue now.

@awaisraad
Copy link

@Fuun347 thank you for clarifying.

@blidblid
Copy link

Is there any known workaround for this issue in Typescript 3.9?

@awaisraad
Copy link

@blidblid yes the workaround is to update the types for ElementFinder in element.d.ts.
Remove
then?: (fn: (value: any) => any | wdpromise.IThenable<any>, errorFn?: (error: any) => any) => wdpromise.Promise<any>;
and it will be all okay.

@Roaders
Copy link

Roaders commented Oct 19, 2020

@blidblid yes the workaround is to update the types for ElementFinder in element.d.ts.
Remove
then?: (fn: (value: any) => any | wdpromise.IThenable<any>, errorFn?: (error: any) => any) => wdpromise.Promise<any>;
and it will be all okay.

sorry but that's not a workaround at all. How Am I supposed to remove that in a CI job where the dependencies are installed on a remote server then immediately built? Very dissapointed to see that this post is 2 years old and there is still no real fix.

This is causing real issues for us updating to angular 10.

@Fuun347
Copy link

Fuun347 commented Oct 20, 2020

@Roaders for now I cannot see a way to remove from protractor as I believe it would be very breaking. The only real workaround I see is not to return ElementFinder or ElementArrayFinder. Also just to clarify, this bug reported in 2016 is not related to the current issues. The current issue is caused by Typescript 3.9 (Changelog). In 3.9 they have added stricter checks to optional properties, which now catches the optional then.

I cannot really also classify this as a bug as it's intended behavior. You should not be able to return a thenable and ElementFinder and ElementArrayFinder where indeed thenables.

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

No branches or pull requests

7 participants