Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(aws-ecs): Add type annotation for the workaround to missing import in d.ts file #1117

Merged
merged 2 commits into from
Nov 8, 2018

Conversation

cockscomb
Copy link
Contributor

The @aws-cdk/aws-ecs/lib/fargate/fargate-task-definition.d.ts file was emitted wrong because TypeScript have an issue (microsoft/TypeScript#26969). So I think we have to workaround by adding a type annotation.

Before

(Comments removed)

import cdk = require('@aws-cdk/cdk');
import { CommonTaskDefinitionProps, TaskDefinition } from '../base/task-definition';

export interface FargateTaskDefinitionProps extends CommonTaskDefinitionProps {
    cpu?: string;
    memoryMiB?: string;
}

export declare class FargateTaskDefinition extends TaskDefinition {
    readonly networkMode: NetworkMode;
    constructor(parent: cdk.Construct, name: string, props?: FargateTaskDefinitionProps);
}

This emit an error like ../../node_modules/@aws-cdk/aws-ecs/lib/fargate/fargate-task-definition.d.ts(46,28): error TS2304: Cannot find name 'NetworkMode'.

After

import cdk = require('@aws-cdk/cdk');
import { CommonTaskDefinitionProps, NetworkMode, TaskDefinition } from '../base/task-definition';

export interface FargateTaskDefinitionProps extends CommonTaskDefinitionProps {
    cpu?: string;
    memoryMiB?: string;
}

export declare class FargateTaskDefinition extends TaskDefinition {
    readonly networkMode: NetworkMode;
    constructor(parent: cdk.Construct, name: string, props?: FargateTaskDefinitionProps);
}

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

@rix0rrr
Copy link
Contributor

rix0rrr commented Nov 8, 2018

Yup. Thanks.

I'm going to add more comments inline but this is the gist of it.

@rix0rrr rix0rrr merged commit ebfb522 into aws:master Nov 8, 2018
rix0rrr pushed a commit that referenced this pull request Nov 8, 2018
Bug Fixes
==========

* correctly emit quoted YAML for account numbers ([#1105](#1105)) ([b4d9155](b4d9155)), closes [#1100](#1100) [#1098](#1098)
* **aws-ecs:** fix use of published NPM package with TypeScript ([#1117](#1117)) ([ebfb522](ebfb522))

Features
==========

* **aws-ecs:** Add desired count to LoadBalanced[Fargate|EC2]Service ([#1111](#1111)) ([cafcc11](cafcc11))
@rix0rrr rix0rrr mentioned this pull request Nov 8, 2018
rix0rrr added a commit that referenced this pull request Nov 8, 2018
Bug Fixes
==========

* correctly emit quoted YAML for account numbers ([#1105](#1105)) ([b4d9155](b4d9155)), closes [#1100](#1100) [#1098](#1098)
* **aws-ecs:** fix use of published NPM package with TypeScript ([#1117](#1117)) ([ebfb522](ebfb522))

Features
==========

* **aws-ecs:** Add desired count to LoadBalanced[Fargate|EC2]Service ([#1111](#1111)) ([cafcc11](cafcc11))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants