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

Promise type does not flow generic type to then'able callback functions #3636

Closed
christianacca opened this issue Oct 12, 2016 · 7 comments
Closed

Comments

@christianacca
Copy link

Bug report

  • Node Version: 6.2.2
  • Protractor Version: 4.0.9
  • Angular Version: 1.5.8

Best to explain with some code...

Here's the code I have to write:

const rows = element.all(by.repeater('user in $data'));
rows.count().then((val: number) => {
  // val is going to be a number, but only because we've had to be explicit by adding a type
  // annotation 
});

Here's the code I would like to write:

const rows = element.all(by.repeater('user in $data'));
rows.count().then(val => {
  // val is inferred/understood to be `number` because the promise returned by `then`
  // has a generic type of `number`
});

The type declaration for Promise / IThenable should "flow" its generic type parameter to the signature of it's thenable callback function parameter.

Instead, as you can see from the first example, I'm having to explicitly add a type annotation for the val parameter.

@heathkit
Copy link
Contributor

I cloned the repo you linked in #3637, and I'm not sure what's going on. You might have an issue with your tsconfig.json, or that there's something else going on. I'm marking this for further investigation because it's possible there's an issue with the selenium-webdriver types - row.count() returns a WebDriver Promise. However, you'll probably have better luck following this up on StackOverflow or Gitter.

@christianacca
Copy link
Author

I have found the problem.

It's a regression with the selenium-webdriver type declarations. Specifically the Promise<T> class.

The problem was introduced by the commit "types(selenium-webdriver): revert 2.44 as default".

Maybe @cnishina should be looking into this as he was the committer?

Before (working)

Before the commit you can see the signature of the callback functions flowing the generic type from the containing Promise:

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/f46e80640829b7dc43a4030868c1e82889acdabd/selenium-webdriver/selenium-webdriver-2.44.0.d.ts#L1471

After (not working)

This is the function signature after the offending commit:

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/f46e80640829b7dc43a4030868c1e82889acdabd/selenium-webdriver/selenium-webdriver.d.ts#L2527

@heathkit
Copy link
Contributor

That's awesome, thanks for digging into this! I may have been a bit hasty to close #3637, but I suspected the issue was the signature of then() on WebDriver's Promise not satisfying the interface for async/await. Based on those commits, it looks like that is what's happening.

For anyone reading this in the future, the discussion in TypeScript #5911 is a good introduction to issues around async/await and type signatures.

Correct me if I'm wrong, but it looks like the new webdriver promise typings don't match the Promise interface in typescript. The fact that they don't respect generics would also explain why TypeScript doesn't consider them to have the right "then()" for async/await.

I'll get with @cnishina about fixing this. It seems like we're one of the first major projects to use selenium webdriver with TypeScript, so we've been doing a lot of unexpected maintenance on their typings.

@christianacca
Copy link
Author

Correct me if I'm wrong, but it looks like the new webdriver promise typings don't match the Promise interface in typescript.

Agreed it doesn't match. Of course it does satisfy the interface otherwise it wouldn't compile...

So hopefully looking into might resolve issue #3637

@megaboich
Copy link

Hi everyone,
I also have some problems with typescript definitions, it is not exactly as described, but at some point its connected.

I have problems with map function definition and how it behaves with incoming promises.
Sample code that illustrates a problem:

    public allColumnHeadersPromise() {
        return element
            .all(by.css('[id$="_table-header"]'))
            .getText();
    }

Here I clearly see returning type as Promise<string[]> which is promise to array of strings, and that is correct according to documentation.
If I change this code to achieve same functionality but with map function:

    public allColumnHeadersPromise() {
        return element
            .all(by.css('[id$="_table-header"]'))
            .map(e => e.getText())
    }

Now I got returning type as Promise<Promise<string>[]>. Promise to array of promises to string. But I expect to get the same result as previous code sample. And in reality it returns the same value, but type definitions is wrong.
So for now I need to manually cast returning type via all to Promise<string[]> which i consider an ugly hack.

I don't know should I create separate issue for this case.
Packages with versions:

"@types/protractor": "^1.5.20",
"@types/selenium-webdriver": "^2.44.29",
"protractor": "4.0.7",
"typescript": "^2.0.3",

@ccrowhurstram
Copy link

So the problem was in the @types/selenium-webdriver. It was fixed in version @types/[email protected] by this commit: DefinitelyTyped/DefinitelyTyped@4b4beff

@heathkit heathkit added this to the Upcoming milestone Oct 20, 2016
@heathkit
Copy link
Contributor

Thanks for the update! For anyone following along, if you're having this problem just do npm install to pick up the latest selenium-webdriver types, and make sure you're on @types/[email protected].

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

5 participants