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

Stream has no exports #4352

Closed
xanderle opened this issue Oct 3, 2019 · 13 comments · Fixed by #4362
Closed

Stream has no exports #4352

xanderle opened this issue Oct 3, 2019 · 13 comments · Fixed by #4362
Assignees
Labels
@aws-cdk/aws-lambda Related to AWS Lambda bug This issue is a bug. management/devenv Related to CDK development/build environment

Comments

@xanderle
Copy link

xanderle commented Oct 3, 2019

tsc fails to compile, StreamEventSource, StreamEventSourceProps missing from aws-lambda-event-sources streams file.

Reproduction Steps

npm install @aws-cdk/core
npm install @aws-cdk/aws-lambda-event-sources
tsc

Error Log

node_modules/@aws-cdk/aws-lambda-event-sources/lib/dynamodb.d.ts:3:10 - error TS2305: Module '"./stream"' has no exported member 'StreamEventSource'.

3 import { StreamEventSource, StreamEventSourceProps } from './stream';
           ~~~~~~~~~~~~~~~~~

node_modules/@aws-cdk/aws-lambda-event-sources/lib/dynamodb.d.ts:3:29 - error TS2305: Module '"./stream"' has no exported member 'StreamEventSourceProps'.

3 import { StreamEventSource, StreamEventSourceProps } from './stream';
                              ~~~~~~~~~~~~~~~~~~~~~~

node_modules/@aws-cdk/aws-lambda-event-sources/lib/kinesis.d.ts:3:10 - error TS2305: Module '"./stream"' has no exported member 'StreamEventSource'.

3 import { StreamEventSource, StreamEventSourceProps } from './stream';
           ~~~~~~~~~~~~~~~~~

node_modules/@aws-cdk/aws-lambda-event-sources/lib/kinesis.d.ts:3:29 - error TS2305: Module '"./stream"' has no exported member 'StreamEventSourceProps'.

3 import { StreamEventSource, StreamEventSourceProps } from './stream';
                              ~~~~~~~~~~~~~~~~~~~~~~

Environment

  • **CLI Version :10.
  • **Framework Version: 10.1.
  • **OS :MacOS
  • **Language : TypeScript

Other

Digging around the node_modules/@aws-cdk/aws-lambda-event-sources/lib/stream.d.ts file shows no exports


This is 🐛 Bug Report

@xanderle xanderle added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 3, 2019
@ashwgupt
Copy link
Contributor

ashwgupt commented Oct 3, 2019

We also faced the same issue even when not using either of the Kinesis and Dynamo streams but only SqsEventSource.

Looking at the change log of 1.11.0, the merged work of #4260 has removed redundant code in KinesisEventSource and DynamoEventSource into common StreamEventSource, and that looks like causing it.

@nmussy
Copy link
Contributor

nmussy commented Oct 3, 2019

Sorry about that guys. stream.ts is supposed to used internally only, I'm not sure why this error is showing up. The immediate solution would be to export the file like I originally was

@nija-at What do you think?

@nmussy
Copy link
Contributor

nmussy commented Oct 3, 2019

Here is an immediate fix in the meantime, using patch-package:

npm i patch-package

tee node_modules/@aws-cdk/aws-lambda-event-sources/lib/stream.d.ts << TS
import lambda = require('@aws-cdk/aws-lambda');
import cdk = require('@aws-cdk/core');
export interface StreamEventSourceProps {
  readonly batchSize?: number;
  readonly startingPosition: lambda.StartingPosition;
  readonly maxBatchingWindow?: cdk.Duration;
}

export declare abstract class StreamEventSource implements lambda.IEventSource {
  readonly props: StreamEventSourceProps;

  protected constructor(props: StreamEventSourceProps);
  public bind(_target: lambda.IFunction): void;
  protected enrichMappingOptions(options: lambda.EventSourceMappingOptions): lambda.EventSourceMappingOptions;
}
TS

npx patch-package @aws-cdk/aws-lambda-event-sources
git add patches/@aws-cdk+aws-lambda-event-sources+1.11.0.patch && git commit

You'll need to add "postinstall": "patch-package" to your package.json scripts field to ensure that the patch will run after each installation.

Hope that helps!

@nija-at
Copy link
Contributor

nija-at commented Oct 3, 2019

Unfortunately, we've introduced a bug here. My fault since I suggested this in the PR - #4260 (comment).

Given that StreamEventSource is extended by KinesisEventSource and DynamoEventSource, it will need to be exported so that the typescript compiler can infer the full type definition of the latter two. Same goes for StreamEventSourceProps.

Equally important, we should check why our current tests don't cover this; and how we should cover this gap.

@RomainMuller
Copy link
Contributor

I'm actually curious why the jsii compiler didn't fail on this as I reckon it should have.

@nmussy
Copy link
Contributor

nmussy commented Oct 3, 2019

IIRC, JSII was complaining until I added the @internal JSDoc tags

@RomainMuller
Copy link
Contributor

RomainMuller commented Oct 3, 2019

Oh. Adding @internal would have "hidden" this definition from the jsii model indeed, which could make this code compile, but then it may not be valid from a TypeScript perspective anymore (it's fairly difficult to assess this from the compiler, which is why we "have to" let that go here)...

Additionally, we currently have a pre-release test that ensures all libraries can be required, but this happens only on the Javascript layer... This test does not cover the type definitions, which I guess is a gap (but I'm not too sure how easy it will be to actually close that gap).

@nmussy
Copy link
Contributor

nmussy commented Oct 3, 2019

I see. My bad then, I thought this was a way to tell JSII not to worry about it not being exposed to the user, but I didn't think this would affect the internal dependencies.

As for testing the definitions, running tsc on a file importing all packages would catch this kind of error.

import '@aws-cdk/alexa-ask';
import '@aws-cdk/app-delivery';
// ...

In v1.11.0:

> npm i @aws-cdk/aws-lambda-event-sources
> echo "import '@aws-cdk/aws-lambda-event-sources';" > test.ts
> tsc test.ts
../node_modules/@aws-cdk/aws-lambda-event-sources/lib/dynamodb.d.ts:3:10 - error TS2305: Module '"./stream"' has no exported member 'StreamEventSource'.

3 import { StreamEventSource, StreamEventSourceProps } from './stream';
           ~~~~~~~~~~~~~~~~~

../node_modules/@aws-cdk/aws-lambda-event-sources/lib/dynamodb.d.ts:3:29 - error TS2305: Module '"./stream"' has no exported member 'StreamEventSourceProps'.

3 import { StreamEventSource, StreamEventSourceProps } from './stream';
                              ~~~~~~~~~~~~~~~~~~~~~~

../node_modules/@aws-cdk/aws-lambda-event-sources/lib/kinesis.d.ts:3:10 - error TS2305: Module '"./stream"' has no exported member 'StreamEventSource'.

3 import { StreamEventSource, StreamEventSourceProps } from './stream';
           ~~~~~~~~~~~~~~~~~

../node_modules/@aws-cdk/aws-lambda-event-sources/lib/kinesis.d.ts:3:29 - error TS2305: Module '"./stream"' has no exported member 'StreamEventSourceProps'.

3 import { StreamEventSource, StreamEventSourceProps } from './stream';
                              ~~~~~~~~~~~~~~~~~~~~~~


Found 4 errors.


> echo $?
2

@nija-at
Copy link
Contributor

nija-at commented Oct 3, 2019

Agreed. We definitely need a test like that.

#4363

@NGL321 NGL321 added management/devenv Related to CDK development/build environment @aws-cdk/aws-lambda Related to AWS Lambda and removed needs-triage This issue or PR still needs to be triaged. labels Oct 3, 2019
@aukaurea
Copy link

aukaurea commented Oct 4, 2019

@nija-at when it will be fixed?

We are a bit blocked.

Thank you.

@nmussy
Copy link
Contributor

nmussy commented Oct 4, 2019

Hey @aukaurea, if you need a fix ASAP, you can look into my previous comment, or downgrade back to the previous release.

Sorry about the inconvenience!

EDIT: fixed the comment link

nija-at pushed a commit that referenced this issue Oct 4, 2019
* fix(lambda-event-sources): add missing export of streams.ts

Fixes #4352
@aukaurea
Copy link

aukaurea commented Oct 4, 2019

HI @nmussy

Having the same issues with the previous version https://www.npmjs.com/package/@aws-cdk/aws-lambda-event-sources/v/1.10.1

node_modules/@aws-cdk/aws-lambda-event-sources/lib/dynamodb.d.ts:3:10 - error TS2305: Module '"./stream"' has no exported member 'StreamEventSource'.

3 import { StreamEventSource, StreamEventSourceProps } from './stream';                                                            ~~~~~~~~~~~~~~~~~                                                                                            
node_modules/@aws-cdk/aws-lambda-event-sources/lib/dynamodb.d.ts:3:29 - error TS2305: Module '"./stream"' has no exported member 'StreamEventSourceProps'.

3 import { StreamEventSource, StreamEventSourceProps } from './stream';                                                                               ~~~~~~~~~~~~~~~~~~~~~~                                                                    
node_modules/@aws-cdk/aws-lambda-event-sources/lib/kinesis.d.ts:3:10 - error TS2305: Module '"./stream"' has no exported member 'StreamEventSource'.

3 import { StreamEventSource, StreamEventSourceProps } from './stream';                                                            ~~~~~~~~~~~~~~~~~                                                                                            
node_modules/@aws-cdk/aws-lambda-event-sources/lib/kinesis.d.ts:3:29 - error TS2305: Module '"./stream"' has no exported member 'StreamEventSourceProps'.

3 import { StreamEventSource, StreamEventSourceProps } from './stream';  

Packages are used:

    "@aws-cdk/aws-appsync": "1.10.1",
    "@aws-cdk/aws-cloudfront": "1.10.1",
    "@aws-cdk/aws-cognito": "1.10.1",
    "@aws-cdk/aws-dynamodb": "1.10.1",
    "@aws-cdk/aws-events-targets": "1.10.1",
    "@aws-cdk/aws-lambda": "1.10.1",
    "@aws-cdk/aws-lambda-event-sources": "1.10.1",
    "@aws-cdk/aws-route53": "1.10.1",
    "@aws-cdk/aws-s3": "1.10.1",
    "@aws-cdk/aws-s3-deployment": "1.10.1",
    "@aws-cdk/aws-sam": "1.10.1",
    "@aws-cdk/core": "1.10.1",
    "aws-cdk": "1.10.1",

@nija-at
Copy link
Contributor

nija-at commented Oct 7, 2019

@aukaurea - We're working on releasing a fix within the next 2 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda Related to AWS Lambda bug This issue is a bug. management/devenv Related to CDK development/build environment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants