Node-slack-sdk: Type integration tests failing due to updated generator definitions

[clavin]

Description

An upcoming change made for TypeScript 3.6 is the addition of stricter generator types. Since dtslint depends on typescript@next, that means this change has started applying to our integration tests--even though we don't build on or run TS 3.6 yet (it's not released!).

This means integration tests are currently broken.

What type of issue is this? (place an x in one of the [ ])

• [x] bug
• [ ] enhancement (feature request)
• [ ] question
• [ ] documentation related
• [ ] testing related
• [ ] discussion

Requirements (place an x in each of the [ ])

• [x] I've read and understood the Contributing guidelines and have done my best effort to follow them.
• [x] I've read and agree to the Code of Conduct.
• [x] I've searched for any related issues and avoided creating a duplicate issue.

[copperwall]

Hey @clavin 👋

I'm interested in taking this on if you or others haven't started working on it yet!

[clavin]

@copperwall 👋 I've actually looked into it in my own branch, and I'd love to share what I've found here:

• ~When testing locally, I couldn't get this error to come up unless I fiddled with what typescript package dtslint was using; that is, the error didn't come up using the same process as the build 🤷‍♀~ Figured it out: needed to do a fresh install using --installAll
• dtslint seems to test against multiple version (I guess?), so the only real solution I can think of would be to pin that test to a single version using a comment, e.g.:

// TypeScript Version: 3.3

// ...

// $ExpectType AsyncIterator<WebAPICallResult>
web.paginate('conversations.list');

I'd love to have a different set of eyes to see if I'm missing anything, though! 👀

[clavin]

This is actually a very puzzling error. So long as multiple versions of TypeScript are being tested, since the type changes between versions, the AsyncIterator type will always be wrong in one or the other 😕

So the only fix I can suggest for something like this is to pick one or the other & change the test command to target a specific TypeScript installation. 😖🤷‍♀

[copperwall]

I tried taking a look at dtslint and the differences in the 3.6 release but I'm pretty stumped with this one. It's kind of a bummer that dtslint always tracks typescript@next, but it looks like there's some issues on their repo asking them not to. 🤷‍♂

Anyways, I'm glad your PR resolves things for now. I'm pretty new to TypeScript, so it didn't occur to me that creating a new interface that extends AsyncIterator would make the type checker happy, but that's cool to know!

That doc comment is really helpful too.