Adds typescript definitions to SDK #1189

chrisradek · 2016-10-14T21:10:19Z

This PR adds a new script that generates TypeScript Definition files for each service.

It also includes a number of d.ts files for classes that aren't dynamically generated at runtime.

If it helps with reviewing, I can upload a new PR that doesn't include all the d.ts files that are in the clients folder. These are the files generated by the script that will be automatically updated with each service update.

These typings were tested using TypeScript 2.0.x, which was recently released.

Developer tools, like vscode, will pick up the correct typings whether you require the whole sdk using require('aws-sdk') or if you require individual services using require('aws-sdk/clients/s3').

I will also work on updating the README to describe how to use these typings.

/cc @LiuJoyceC

…lobal AWS namespace

chrisradek · 2016-10-20T15:56:41Z

Updated the global.d.ts and index.d.ts files.
Using Object.assign was causing the typings to only work when targeting ES6, now these will also work when targeting ES5.

The index.d.ts file also exports the typings as a global namespace, AWS, so that browser SDK users can get typing support as well.

LiuJoyceC · 2016-10-24T23:28:51Z

clients/acm.d.ts

+   */
+  resendValidationEmail(callback?: (err: AWSError, data: {}) => void): Request<{}, AWSError>;
+}
+declare namespace ACM {


Is there a reason the service model's shapes need to be on this namespace? When I type AWS.ACM., I don't think my code completion tool should try to complete AddTagsToCertificateRequest or CertificateBody, since these aren't actually valid properties on AWS.ACM. Instead, this namespace should contain apiVersions, serviceIdentifier, and services (unless these are private, in which case just don't have an ACM namespace). The shapes can live in a separate namespace that isn't exported, and you just have to change the references above. I've tested this by changing this namespace name to be Shapes (and changing references above), and I still get the correct code completion suggestions when I'm creating my params in a request, and I no longer get shape name suggestions when I type AWS.ACM..

I wanted to export all the interfaces so that users could cast a result if they needed to, but I'm not sure if that's necessary. I'll have to take a look at some other TypeScript libraries to see if that's common practice.

You should only see these interfaces when looking at the service client constructors, but not on the service client instances.

Right, it would only be on the constructor, but there are valid reasons a customer may want to look at the properties of the constructor such as looking up what apiVersions are available for a service, or for the case of S3 and DynamoDB, they want to access the ManagedUpload or DocumentClient. When they try to access these, the constructors will autocomplete with a large number of properties that aren't real and make it difficult to find the actual property they are trying to access.

So, I tried a few different things. Ultimately, I went with putting the exported types in a sub-namespace:
declare namespace SERVICE.Types {
When I put them on their own namespace, I had to explicitly import them into my app, otherwise the typescript compiler would complain:
microsoft/TypeScript#9944

I also wanted them to be exported so user's can specify a type for cases when the typescript compiler can't quite infer what a type should be. That might not be necessary once microsoft/TypeScript#6606 is addressed.

LiuJoyceC · 2016-10-25T00:15:01Z

clients/acm.d.ts

+import {AWSError} from '../lib/error';
+import {Service} from '../lib/service';
+declare class ACM extends Service {
+  /**


This might not be a must-have, but it would be nice to have code completion work when I'm binding params to a service client on construction.
One way you could do this is have a constructor such as:
constructor(options?: ServiceConfigurationOptions & ServiceParams)
And then near the bottom of the file you can define:

interface ServiceParams { params?: ACM.AddTagsToCertificateRequest & ACM.DeleteCertificateRequest & ... }

Since the TSGenerator has to iterate through every operation input shape anyway, it could just keep track of a list of all of the input shape names.

That's an interesting idea. I don't think this will work for S3, or services that have custom config options (anything with dualstack, just S3 for now) if we stop exporting the service interfaces though. I'll take a look.

Well, for S3, since you currently have
constructor(options?: ServiceConfigurationOptions & UseDualstackConfigOptions),
couldn't that just be changed to
constructor(options?: ServiceConfigurationOptions & UseDualstackConfigOptions & ServiceParams)?

Yeah, but I'd have to import ServiceParams from somewhere, presumably the base S3 class.

I think the ServiceParams should be defined in the same file as where the service model shapes are defined (in the clients/#{service}.d.ts file), since they are specific to each service and they will reuse the model shapes definitions already in the file. And that would be the same file that the constructor definition for that service class is defined, too. For S3, it would be the same (the constructor definition is already in the same file as the model shapes definitions). The only exceptions would be any custom params (for example, partSize in the upload operation), which could be imported from the service customizations file (the S3Customizations class is already being imported from there anyway).

Also, as a modification to what I said above, instead of merging together all of the input shapes, it would be better if each potential param is explicitly listed, since they are all optional here, and we don't want to keep the requirements of the input params. Most services reuse most of their input params, so for many services the list of possible params may actually be shorter than the list of input shapes. So instead of what I have above, it could be more like:

interface ServiceParams { params?: ACM.AllInputParams }

and inside the ACM namespace have:

interface AllInputParams { CertificateArn?: Arn; Tags?: TagList; CertificateStatuses?: CertificateStatuses; ... }

It looks like I'll have to create a custom shape, since only top-level params are actually allowed:
https://github.com/aws/aws-sdk-js/blob/master/lib/service.js#L163

LiuJoyceC · 2016-10-25T21:22:52Z

clients/acm.d.ts

+import {AWSError} from '../lib/error';
+import {Service} from '../lib/service';
+declare class ACM extends Service {
+  /**


Another reason to have a constructor definition for each service class is to allow code completion for API version dates (by defining the optional apiVersion as an enum in ServiceParams), especially if we want to encourage users to specify API versions when constructing the service clients.

LiuJoyceC · 2016-10-25T21:49:20Z

scripts/lib/ts-generator.js

+ * Determines if a member is required by checking for it in a list.
+ */
+TSGenerator.prototype.checkRequired = function checkRequired(list, member) {
+    if (list.indexOf(member)) {


if (list.indexOf(member) !== -1)

Otherwise it's always returning true if the member isn't actually required, and it's returning false if the member happens to be the first one in the list

Thanks, good catch!

LiuJoyceC · 2016-10-25T22:00:14Z

scripts/lib/ts-generator.js

+            if (member.documentation) {
+                code += self.generateDocString(member.documentation, tabCount + 1);
+            }
+            var required = self.checkRequired(shape.required || [], memberKey) ? '?' : '';


If checkRequired is supposed to return true when the member is required, then this should be switched.
self.checkRequired(shape.required || [], memberKey) ? '' : '?'

Thanks, didn't catch this due to the mistake above.

LiuJoyceC · 2016-10-26T18:08:19Z

scripts/lib/ts-generator.js

+    } else if (['double', 'long', 'short', 'biginteger', 'bigdecimal', 'integer', 'float'].indexOf(type) >= 0) {
+        code += tabs(tabCount) + 'export type ' + shapeKey + ' = number;\n';
+    } else if (type === 'timestamp') {
+        code += tabs(tabCount) + 'export type ' + shapeKey + ' = Date;\n';


Perhaps timestamp should be Date|string|number? When the SDK validates input params, these 3 types are valid for timestamps, so we don't want to raise a type error if the user is using a valid type. For timestamps in the output, it often is returned as a string, and an error should be raised if someone tries to invoke a Date method on it.

What's an example of a timestamp in the output that is returned as a string, instead of a Date?
If the shape is modeled as a timestamp, and the value is a string or number, we call util.date.parseTimestamp, and parseTimestamp always returns a Date object, or throws an error.

Fair point about the above for inputs though.

If an output field modeled as a timestamp is sourced from the response header rather than the body, then it remains as a string in the output data. For example, in S3, the LastModified shape is modeled as a timestamp. In the output of getObject and headObject, the LastModified field is sourced from the header rather than the body, so the value is a string. In s3.d.ts, LastModified is typed as a Date, which would be incorrect for the output of these two operations. In some other operations such as listObjects or copyObjects, the LastModified field (modeled by the same LastModified shape) is sourced from the response body and the value is converted to a Date object.

LiuJoyceC · 2016-10-26T18:17:43Z

scripts/lib/ts-generator.js

+TSGenerator.prototype.generateTypingsFromShape = function generateTypingsFromShape(shapeKey, shape, tabCount) {
+    // some shapes shouldn't be generated if they are javascript primitives
+    if (['string', 'boolean', 'number', 'Date', 'Blob'].indexOf(shapeKey) >= 0) {
+        return '';


Instead of skipping shapes that happen to be named these, we could just add an underscore in front to differentiate the name of the shape from the javascript primitives.
It could still be useful to generate a typing for these shapes. For example, some services (like CodeCommit) have a shape named "Date" that is modeled as a string rather than a timestamp. If we rename the shape rather than skip it and have it be typed as a JavaScript Date object, then it could prevent errors before runtime, such as calling a Date method like getMonth() on the string.

So for example, the typing could look like:

export type _Date = string;

And the references to this shape would have to have an underscore added in front as well.

Fair point, thanks for finding that.

LiuJoyceC · 2016-10-26T19:04:33Z

scripts/lib/ts-generator.js

+        code += tabs(tabCount) + 'export type ' + shapeKey + ' = ' + shape.member.shape + '[];\n';
+    } else if (type === 'map') {
+        code += tabs(tabCount) + 'export type ' + shapeKey + ' = {[key: string]: ' + shape.value.shape + '};\n';
+    } else if (type === 'string' || type === 'character') {


It would be helpful to support enums.
(after checking for complex types and before starting to check primitive types, so between map and string):

else if (Array.isArray(shape.enum)) { code += tabs(tabCount) + 'export type ' + shapeKey + ' = ' + shape.enum.map(JSON.stringify).join('|') + ';\n'; }

So for example, Lambda's Runtime declaration would look like:

export type Runtime = "nodejs"|"nodejs4.3"|"java8"|"python2.7";

instead of just a generic string

export type Runtime = string;

Agreed, I had meant to get to this but hadn't yet. I think we should also still add the primitive type (string) as well, so that users can specify new enums as they become available without having to update the SDK.

So do you mean like this?

export type Runtime = "nodejs"|"nodejs4.3"|"java8"|"python2.7"|string;

If our main goal is for this to be used for code completion, then this is fine. However, if a user wants to catch compile-time errors, having the enum without the catch-all string type could be really useful in catching typos.

Yes, that's exactly what it'd look like.

I don't want the user to have to update the SDK to use a new enum. If Lambda added support for nodejs5, and we didn't specify string as an allowable type, they would have to update their SDK to the latest version or else get compilation errors. Today with JavaScript, they could just continue using their current version of the SDK.

LiuJoyceC · 2016-10-27T00:54:03Z

scripts/lib/ts-generator.js

+    code += this.generateDocString(operation.documentation, tabCount);
+    code += tabs(tabCount) + operationName + '(params: ' + inputShape + ', callback?: (err: AWSError, data: ' + outputShape + ') => void): Request<' + outputShape + ', AWSError>;\n';
+    code += this.generateDocString(operation.documentation, tabCount);
+    code += tabs(tabCount) + operationName + '(callback?: (err: AWSError, data: ' + outputShape + ') => void): Request<' + outputShape + ', AWSError>;\n';


Perhaps we could check whether the input has any required params, and if it has at least one, then don't generate this second second declaration where params is optional.

If the required params were specified in the service-level config though, then they wouldn't be required here, so the 2nd option could still be valid.

Well, we have that problem anyway if the user defines required params in the service-level config but has additional parameters. Perhaps we should just make all input params be optional?

LiuJoyceC · 2016-10-27T00:56:22Z

scripts/lib/ts-generator.js

+    tabCount = tabCount || 0;
+    var operationName = waiter.operation.charAt(0).toLowerCase() + waiter.operation.substring(1);
+
+    var docString = 'Waits for the ' + waiterState + ' state by periodically calling the underlying ' + className + '.' + operationName + 'operation every ' + waiter.delay + ' seconds (at most ' + waiter.maxAttempts + ' times).';


Add a space before the word 'operation'.

LiuJoyceC · 2016-10-27T01:01:32Z

scripts/lib/ts-generator.js

+/**
+ * Generates class method types for a waiter.
+ */
+TSGenerator.prototype.generateTypingsFromWaiters = function generateTypingsFromWaiters(className, waiterState, waiter, underlyingOperation, tabCount) {


waiterState needs to have its first letter lowercased. For example, using the state "BucketExists" will result in an error, but "bucketExists" is correct.

Good catch!

LiuJoyceC · 2016-10-27T01:03:39Z

scripts/lib/ts-generator.js

+    code += this.generateDocString(docString, tabCount);
+    code += this.tabs(tabCount) + 'waitFor(state: "' + waiterState + '", params: ' + inputShape + ', callback?: (err: AWSError, data: ' + outputShape + ') => void): Request<' + outputShape + ', AWSError>;\n';
+    code += this.generateDocString(docString, tabCount);
+    code += this.tabs(tabCount) + 'waitFor(state: "' + waiterState + '", callback?: (err: AWSError, data: ' + outputShape + ') => void): Request<' + outputShape + ', AWSError>;\n';


Same comment as above about required params. If the underlying operation has any required params, that perhaps we shouldn't generate this second typing where params is optional.

chrisradek · 2016-10-27T20:45:44Z

Posting my sample tsconfig.json file here:

{
    "compilerOptions": {
        "target": "es5",
        "module": "commonjs",
        "outDir": "dist",
        "declarationDir": "dist",
        "declaration": true,
        "lib": [
            "es5",
            "es2015.promise",
            "dom"
        ]
    },
    "include": [
        "src/**/*"
    ]
}

Assumes that .ts files are located under a src directory.

LiuJoyceC · 2016-10-27T21:18:33Z

lib/cloudfront/signer.d.ts

+     * @param {string} keyPairId - The ID of the CloudFront key pair being used.
+     * @param {string} privateKey - A private key in RSA format.
+     */
+    constructor(keyPaidId: string, privateKey: string);


typo: keyPairId instead of keyPaidId

Nice catch!

LiuJoyceC · 2016-10-27T22:14:58Z

lib/config.d.ts

@@ -0,0 +1,268 @@
+/// <reference types="node" />
+
+import * as http from 'http';


Would doing import {Agent as httpAgent} from 'http'; and import {Agent as httpsAgent} from 'https'; make a difference in how much typings data is loaded into memory by Intellisense or by the typescript compiler? I've seen complaints about Intellisense slowing down VS Code or the typescript compiler being too slow when declaration files are large. Since the only typings we need are for Agent from both modules, it seems unnecessary to load all of the typings from those modules into memory.

LiuJoyceC · 2016-10-29T00:12:56Z

lib/config.d.ts

+import * as https from 'https';
+import {AWSError} from './error';
+import {Credentials} from './credentials';
+export class Config {


The Config class also needs to have each service identifier as an optional instance variable. Otherwise the compiler will complain if a user tries to define service-specific parameters and options:

AWS.config.s3 = {params: {Bucket: 'myBucket'}, useDualstack: true};

LiuJoyceC · 2016-10-29T01:03:41Z

lib/config.d.ts

+     */
+    customBackoff?: (retryCount: number) => number
+}
+interface ConfigurationOptions {


Instead of maintaining two separate but identical lists of these config options plus comments (here and above in the Config class), perhaps we could define this as an abstract class rather than an interface, and then the Config class could extend this abstract class.
Another solution could be to generate this file (which may be a good idea anyway since we need to include all of the service identifiers in the Config class as mentioned above). However, we would then need to maintain the comments for each of these configuration options in another file, since I noticed these comments aren't exactly identical to the documentation in the config.js file.

…rs. Fixes required inputs checks. Adds support for string enum types. Fixes issue where some service operations included the apiversion in the name.

…ves service interfaces to [ClassName].Types namespace.

chrisradek · 2016-11-10T20:37:01Z

Closed by #1218

lock · 2019-09-28T11:57:37Z

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.

chrisradek added 4 commits October 14, 2016 13:53

Adds typescript definitions to SDK

3b97d35

removes client typings for easier review

d12e9e5

Adds a few more typings for review

ba43778

Add typing support for ES5 projects, and in browser environment via g…

f15c5d2

…lobal AWS namespace

LiuJoyceC reviewed Oct 24, 2016

View reviewed changes

LiuJoyceC reviewed Oct 25, 2016

View reviewed changes

LiuJoyceC reviewed Oct 26, 2016

View reviewed changes

LiuJoyceC reviewed Oct 27, 2016

View reviewed changes

chrisradek mentioned this pull request Oct 27, 2016

Typescript (and Angular2 support). #994

Closed

LiuJoyceC reviewed Oct 27, 2016

View reviewed changes

tomdavidson mentioned this pull request Oct 27, 2016

refactor aws-sdk for promise support and with overloading DefinitelyTyped/DefinitelyTyped#12205

Closed

1 task

LiuJoyceC reviewed Oct 27, 2016

View reviewed changes

LiuJoyceC reviewed Oct 29, 2016

View reviewed changes

chrisradek added 3 commits October 31, 2016 16:47

Moves service interfaces to the Types namespace on Service constructo…

e6deb18

…rs. Fixes required inputs checks. Adds support for string enum types. Fixes issue where some service operations included the apiversion in the name.

Fixes issues with service and config typings

a9dcd1f

Enhances Configuration typings to support service-specific config. Mo…

c9d2d43

…ves service interfaces to [ClassName].Types namespace.

chrisradek mentioned this pull request Nov 10, 2016

Adds TypeScript Definitions #1218

Merged

chrisradek closed this Nov 10, 2016

chrisradek deleted the typings branch November 10, 2016 20:37

lock bot locked as resolved and limited conversation to collaborators Sep 28, 2019

		@@ -0,0 +1,268 @@
		/// <reference types="node" />

		import * as http from 'http';

Adds typescript definitions to SDK #1189

Adds typescript definitions to SDK #1189

Conversation

chrisradek commented Oct 14, 2016

chrisradek commented Oct 20, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LiuJoyceC Oct 25, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LiuJoyceC Oct 25, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LiuJoyceC Oct 26, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LiuJoyceC Oct 26, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisradek commented Oct 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisradek commented Nov 10, 2016

lock bot commented Sep 28, 2019

LiuJoyceC Oct 25, 2016 •

edited

Loading

LiuJoyceC Oct 25, 2016 •

edited

Loading

LiuJoyceC Oct 26, 2016 •

edited

Loading

LiuJoyceC Oct 26, 2016 •

edited

Loading