Skip to content
New issue

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

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

Already on GitHub? # to your account

Improvements for cdk init typescript #18743

Closed
rantoniuk opened this issue Jan 31, 2022 · 8 comments
Closed

Improvements for cdk init typescript #18743

rantoniuk opened this issue Jan 31, 2022 · 8 comments
Labels
effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 package/tools Related to AWS CDK Tools or CLI

Comments

@rantoniuk
Copy link

General Issue

Improvements for cdk init typescript

The Question

After running cdk init app --language=typescript and checking how the project is set up, I can see the following that could be improved:

  • @aws-cdk dependencies are pinned, while they could (should?) use ^1.141.0
  • typescript major version is 3.x, could be bumped to ~4.4.4
  • import 'source-map-support/register'; line doesn't seem to be needed

tsconfig.json

  • add "outDir": "dist" and dist to exclude section to keep the workspace clean

Happy to provide PR if the above make sense, please let me know where to look though.

CDK CLI Version

1.141.0

Framework Version

No response

Node.js Version

14

OS

MacOS

Language

Typescript

Language Version

No response

Other information

No response

@rantoniuk rantoniuk added guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. labels Jan 31, 2022
@peterwoodworth peterwoodworth added effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 package/tools Related to AWS CDK Tools or CLI and removed guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. labels Feb 3, 2022
@peterwoodworth
Copy link
Contributor

Hey @rantoniuk,

Our init templates can be found here

I don't think we'll want to pin dependencies or remove source map support, but I think we'd like to have the TS version upgraded 🙂

I'm unsure if we'll want to make the tsconfig change either

@rantoniuk
Copy link
Author

Hi @peterwoodworth

  • TS version is pinned and needs to be pinned because of incompatibility warning.
  • I can see %cdk-version% but I can't find where it is defined/how it is computed. The resulting version should contain ~ or ^ instead of pinning specific cdk version.

I cannot react to the other two responses, since they do not contain any reason/explanation.
I will just wait then for a specific answer before creating any PRs in order not to waste time on both sides :)

@hoegertn
Copy link
Contributor

hoegertn commented Feb 4, 2022

For CDK v1 the version needs to be pinned as it will explode with version incompatibilities otherwise

@rantoniuk
Copy link
Author

It will not explode if package-lock is used and all packages are upgraded when new packages are added.
We are using it this way and it works fine (indeed, we had the exploding problem indeed if not using npm upgrade after adding new packages).

@skinny85
Copy link
Contributor

skinny85 commented Feb 4, 2022

A package-lock.json will not help you when adding new versions though.

We've actually had ^ versions there before, and we decided to change them on purpose.

The TypeScript version is dictated by the JSII version we use.

@rantoniuk
Copy link
Author

A package-lock.json will not help you when adding new versions though.

package-lock.json together with npm upgrade and ^ modifier works perfectly fine.
Indeed, if you don't want to promote the usage of the lockfile (which is a bad practice not only for CI), then sure, that will cause problems.

The TypeScript version is dictated by the JSII version we use.

Surely it should not be dictated up to a pinned version, but to a compatible version - i.e. it should use at least ~ to be able to get security fixes.

@TheRealAmazonKendra
Copy link
Contributor

Thank you for your suggestions here. Given that we are unable to upgrade TypeScript at this time, need the source-map-support/register, and no longer use individual imports on v2, I think this issue should be closed at this time. If there are further improvements you'd like to propose on our v2 templates and/or believe this issue was closed in error, please feel free to open a new issue.

@github-actions
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.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

No branches or pull requests

7 participants