AWS definitions broke all builds

[blakeembrey]

The AWS definitions are incorrect in a number of ways and has broken all builds depending on your TypeScript configuration. Issues I run into:

• Relies on implicit any
• Relies on XMLHttpRequest (which is not in the standard definition, fails when you don't include the browser-side definitions with TypeScript)
• Cannot find type definition file for 'node'. - relies on @types which means I can't use the better definitions from Typings or DefinitelyTyped

[marcote]

@blakeembrey the bug about implicit any it's already fixed in #1221

[chrisradek]

@blakeembrey
What were you using previously to define types for the SDK?

#1221 has just been merged in, I'll go through and see if there are any other methods returning implicit any.

Regarding XMLHttpRequest, I'll look into that. I wonder if that can be resolved by including a jsconfig.json with this package that includes dom under compilerOptions.lib.

For your third point, on typescriptlang.org it mentions that references to paths should not be used in definition files, and using <reference types="..." /> is preferred. If I removed that reference, wouldn't I also need to remove any imports from that reference as well, such as where I'm importing http/https/stream/etc? Is the best practice to not import those in a library, or am I misunderstanding something?

[blakeembrey]

@chrisradek I had my own local definitions that were incomplete (mostly just polyfilled for what I was using).

For your third point

It's not entirely true. The types reference only causes a error to be emitted by TypeScript, but all the definitions still exist within TypeScript would resolve. Basically, it emits Cannot find type definition file for 'node' instead of Cannot find module 'http'. It's up to you whether you remove it, though me and a number of others won't be able to use it unless you do and the alternative errors may be preferred - especially if you're typing a definition that shouldn't require node anyway (like the browser version of this library, I shouldn't really need to install node for that).

Edit: Sorry, I just realised it might still be unclear. You can absolutely use any of the imports, it's just the type reference breaking people not using @types right now. Instead, you can rely on TypeScript encountering the import and not finding it (it will find it anyway if someone is using @types/node, regardless of the reference).

[midknight41]

Firstly, thank you for creating these definitions! I've been involved in maintaining these definitions at DefinitelyTyped for years and I can't keep up with you folks. 😦

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/aws-sdk/aws-sdk.d.ts

That said, this broke my builds too. I've not using @types yet either.

Are you planning to contribute the definitions to DefinitelyTyped? Your definitions should replace mine ideally!

[chrisradek]

@blakeembry
Ah, thanks for the clarification, I didn't realize the TypeScript compiler would still be able to recognize those imports.

It seems like the best thing to do is remove the types reference then. I'll fix that and the issue related to XMLHttpRequest and include them with the next release.

Really appreciate the quick feedback and the info you've provided!

[blakeembrey]

@chrisradek No problem, happy to help. I usually help people migrate to supporting TypeScript definitions, so if you ever need a second pair of eyes you can ping me. See typings/typings#322 (little outdated, but I still help with other repos). If those two things can be fixed, it'd be amazing 😄

@midknight41 Once the definitions here work and are stable, no one should need to rely on DefinitelyTyped anymore. I wouldn't want the developers here getting stuck trying to maintain two repo locations and having to refactor these external module definitions into global definitions for DefinitelyTyped.

[midknight41]

The main thing is making sure the community ends up with the correct definitions (yours) and avoids any kind of collision regardless of how they install it.

If I can help, let me know. :-)

[chrisradek]

I've created #1228 to fix the issues encountered here. If anyone has time to test it out, I'd love to hear if there are outstanding issues before these can start being used.

[chrisradek]

Since #1228 has been merged, I'm closing this issue. Feel free to comment or open up a new issue if you encounter problems with the typescript definitions!

[blakeembrey]

It seems like it's still an issue, just updated to 2.7.9 and got:

node_modules/aws-sdk/lib/config.d.ts(1,1): error TS2688: Cannot find type definition file for 'node'.
node_modules/aws-sdk/lib/request.d.ts(1,1): error TS2688: Cannot find type definition file for 'node'.
node_modules/aws-sdk/lib/services/glacier.d.ts(1,1): error TS2688: Cannot find type definition file for 'node'.
src/support/upload.ts(115,14): error TS2339: Property 'on' does not exist on type 'ManagedUpload'.

The last one is because I'm listening to the httpUploadProgress on http://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/S3/ManagedUpload.html.

[MarcusNoble]

I'm also seeing similar issues in latest version:

app/services/AwsQueueClient.ts(32,32): error TS2305: Module 'SQS' has no exported member 'SendMessageResult'.
app/services/AwsQueueClient.ts(34,30): error TS2305: Module 'SQS' has no exported member 'SendMessageParams'.
app/services/AwsQueueClient.ts(38,69): error TS2305: Module 'SQS' has no exported member 'SendMessageResult'.
app/services/AwsQueueClient.ts(63,32): error TS2305: Module 'SQS' has no exported member 'Message'.
app/services/AwsQueueClient.ts(66,30): error TS2305: Module 'SQS' has no exported member 'ReceiveMessageParams'.
app/services/AwsQueueClient.ts(72,74): error TS2305: Module 'SQS' has no exported member 'ReceiveMessageResult'.
app/services/AwsQueueClient.ts(104,30): error TS2305: Module 'SQS' has no exported member 'DeleteMessageParams'.
app/services/QueueWorker.ts(81,52): error TS2305: Module 'SQS' has no exported member 'Message'.
app/types/queues.d.ts(6,41): error TS2305: Module 'SQS' has no exported member 'SendMessageResult'.
app/types/queues.d.ts(7,43): error TS2305: Module 'SQS' has no exported member 'Message'.

[chrisradek]

@blakeembrey
You're right, I must have missed some files, updated with #1248.

I'll add another PR shortly for adding on to ManagedUpload.

@MarcusNoble
Can you share a snippet of what your code that's failing looks like?

[MarcusNoble]

I've just taken another look (in another project that i'm working on) and it seems that the Message has moved from aws.SQS.Message to aws.SQS.Types.Message. Was this intentional?

[chrisradek]

@MarcusNoble
Were you previously pulling in 3rd party typings for the AWS SDK? Since we started including typings in v2.7.0, we have always hung the service client operation interfaces on the Types namespace.

[MarcusNoble]

Yes. I previously used typings, then used the @types package on npm.

Is it best to stop using those and update code to use the .Types. namespace?

[chrisradek]

The main benefit to using the typings shipped with the SDK are that the service definitions are updated with each release, and will reflect the version of the SDK you're using.

I'll take a look to see how feasible it is to reference those interfaces directly on the service client, without removing them from Types since that might ease transitioning from 3rd party typings.

[midknight41]

@MarcusNoble
Once the official definitions are complete the ones you've been using (from DefinitelyTyped I believe) will be deprecated. Despite best efforts they are missing many services and haven't kept up with all the changes over the years.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.