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

cloudformation-diff: Hidden dependency on aws-sdk #28679

Closed
frankpengau opened this issue Jan 12, 2024 · 2 comments · Fixed by #28680 or #28768 · May be fixed by stack-spot/app-handler-functions-template#2, stack-spot/eks-env-ts-template#2 or stack-spot/web-react-deploy#4
Labels
@aws-cdk/cloudformation-diff bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@frankpengau
Copy link
Contributor

frankpengau commented Jan 12, 2024

Describe the bug

We have a custom aws-cdk consruct library that exports cloudformation-diff.

Since @aws-cdk/cloudformation-diff for version 2.119.0, it is throwing the following error:

node_modules/@aws-cdk/cloudformation-diff/lib/diff-template.d.ts:1:32 - error TS2307: Cannot find module 'aws-sdk' or its corresponding type declarations.

1 import { CloudFormation } from 'aws-sdk';
                                 ~~~~~~~~~


Found 1 error in node_modules/@aws-cdk/cloudformation-diff/lib/diff-template.d.ts:1

We believe the error is coming from changes in this PR: #28336

Specifically in packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts, the following change:

// The SDK is only used to reference `DescribeChangeSetOutput`, so the SDK is added as a devDependency.
// The SDK should not make network calls here
// eslint-disable-next-line import/no-extraneous-dependencies
import { CloudFormation } from 'aws-sdk';

I have a sample repo to reproduce the exact issue: https://github.com/frankpengau/cdk-lib-cfn-diff-issue-20240112

Expected Behavior

It should not have a missing dependency.

Current Behavior

It currently has an indirect dependency on aws-sdk causing our construct library to have a transitive dependency on aws-sdk. If it is indeed a dependency, it should be added in as such, instead of being only a devdependency.

Reproduction Steps

  1. cdk init lib --language typescript
  2. Install @aws-cdk/cloudformation-diff for version 2.119.0
  3. In a new file in lib folder: export * as cfndiff from @aws-cdk/cloudformation-diff
  4. Compile typescript tsc

Possible Solution

Explicit dependency on aws-sdk, so that it shows up in the package-lock.json file and installs it as necessary for @aws-cdk/cloudformation-diff to work.

Additional Information/Context

No response

CDK CLI Version

2.114.1

Framework Version

No response

Node.js Version

18.13.0

OS

macOS

Language

TypeScript

Language Version

TypeScript (5.2.2)

Other information

No response

@frankpengau frankpengau added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 12, 2024
@pahud
Copy link
Contributor

pahud commented Jan 12, 2024

Thank you for the report. We will review the implementation in your PR.

@pahud pahud added p1 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jan 12, 2024
@mergify mergify bot closed this as completed in #28680 Jan 16, 2024
mergify bot pushed a commit that referenced this issue Jan 16, 2024
Missing dependency when exporting `@aws-cdk/cloudformation-diff` in custom AWS Construct Library. 

Closes #28679

----

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

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

mergify bot pushed a commit that referenced this issue Mar 16, 2024
…get CFN types resolved in exports (#28768)

Issue still persists after #28680. 
```
node_modules/@aws-cdk/cloudformation-diff/lib/diff-template.d.ts:1:37 - error TS2307: Cannot find module 'aws-sdk' or its corresponding type declarations.

1 import type { CloudFormation } from 'aws-sdk';
                                      ~~~~~~~~~


Found 1 error in node_modules/@aws-cdk/cloudformation-diff/lib/diff-template.d.ts:1
```

Types are still required as a direct dependency in package.json.

Closes #28679 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
ahammond pushed a commit to ahammond/aws-cdk that referenced this issue Mar 26, 2024
…get CFN types resolved in exports (aws#28768)

Issue still persists after aws#28680. 
```
node_modules/@aws-cdk/cloudformation-diff/lib/diff-template.d.ts:1:37 - error TS2307: Cannot find module 'aws-sdk' or its corresponding type declarations.

1 import type { CloudFormation } from 'aws-sdk';
                                      ~~~~~~~~~


Found 1 error in node_modules/@aws-cdk/cloudformation-diff/lib/diff-template.d.ts:1
```

Types are still required as a direct dependency in package.json.

Closes aws#28679 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment