Skip to content

feat(cdk): update tsconfig to more modern settings in typescript init templates #31927

Merged
mergify[bot] merged 9 commits intoaws:mainfrom
ren-yamanashi:update-typescript-init-template
Oct 30, 2024
Merged

feat(cdk): update tsconfig to more modern settings in typescript init templates #31927
mergify[bot] merged 9 commits intoaws:mainfrom
ren-yamanashi:update-typescript-init-template

Conversation

@ren-yamanashi
Copy link
Copy Markdown
Contributor

@ren-yamanashi ren-yamanashi commented Oct 29, 2024 •
edited
Loading

Issue # (if applicable)

n/A

Reason for this change

I think contents of tsconfig.json are a little outdated and seemed to need update.

Description of changes

The following two changes I made:

  1. Removed unnecessary options
  2. Updated target and lib options at compilerOptions

1. Removed unnecessary options

Unnecessary options included in the compilerOptions of tsconfig.json.
The unnecessary options are as follows

  • noImplicitAny
  • strictNullChecks
  • noImplicitThis
  • alwaysStrict

If strict: true is specified in compilerOptions, these options will also be true, so there is no need to specify them.

see: https://www.typescriptlang.org/tsconfig/#strict

2. Updated target and lib options at compilerOptions

  • target
    • I think 'ES2023' is more appropriate for the target option at this point.
  • lib

Description of how you validated changes

Checklist

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

@github-actions github-actions bot added p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Oct 29, 2024
@aws-cdk-automation aws-cdk-automation requested a review from a team October 29, 2024 13:33
@ren-yamanashi ren-yamanashi changed the title chore(init-templates): update typescript/tsconfig.json chore(init-templates): update tsconfig.json Oct 29, 2024
@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 29, 2024
mrgrain
mrgrain previously requested changes Oct 29, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@mrgrain mrgrain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Going by this https://www.npmjs.com/package/@tsconfig/node-lts we should set the target to ES2022 and lib to include ES2023.

strictPropertyInitialization can also be removed.

@aws-cdk-automation aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 29, 2024
@mergify mergify bot dismissed mrgrain’s stale review October 30, 2024 00:19

Pull request has been modified.

@ren-yamanashi
Copy link
Copy Markdown
Contributor Author

ren-yamanashi commented Oct 30, 2024

@mrgrain

Nice! Going by this https://www.npmjs.com/package/@tsconfig/node-lts we should set the target to ES2022 and lib to include ES2023.

Thank you review! I fixed in the following commit.

strictPropertyInitialization can also be removed.

strictPropertyInitialization is currently set to false, but is it okay to remove it?
Note: If strict: true is set, strictPropertyInitialization will implicitly become true.

If it is better not to remove, I will reset this commit. 6a66600

@mrgrain
Copy link
Copy Markdown
Contributor

mrgrain commented Oct 30, 2024

strictPropertyInitialization can also be removed.

strictPropertyInitialization is currently set to false, but is it okay to remove it?
Note: If strict: true is set, strictPropertyInitialization will implicitly become true.

If it is better not to remove, I will reset this commit. 6a66600

Ha nice catch. Let's keep it set to false then for now.

@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 30, 2024
@mrgrain mrgrain changed the title chore(init-templates): update tsconfig.json feat(cdk): update tsconfig to more modern settings in typescript init templates Oct 30, 2024
aws-cdk-automation
aws-cdk-automation previously requested changes Oct 30, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

@aws-cdk-automation aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 30, 2024
@ren-yamanashi ren-yamanashi force-pushed the update-typescript-init-template branch from 6a66600 to 8c3d56b Compare October 30, 2024 10:07
@ren-yamanashi
Copy link
Copy Markdown
Contributor Author

ren-yamanashi commented Oct 30, 2024 •
edited
Loading

Ha nice catch. Let's keep it set to false then for now.

@mrgrain

I have set strictPropertyInitialization back to false, so please check.

Also, there is no README and test, and CI seems to be failing.
Should I address these issues?

The PR that made a similar change to this PR did not seem to have updated the README or test...
#27422

@mrgrain mrgrain added pr-linter/exempt-readme The PR linter will not require README changes pr-linter/exempt-test The PR linter will not require test changes pr-linter/exempt-integ-test The PR linter will not require integ test changes labels Oct 30, 2024
@mrgrain
Copy link
Copy Markdown
Contributor

mrgrain commented Oct 30, 2024

Ha nice catch. Let's keep it set to false then for now.

@mrgrain

I have set strictPropertyInitialization back to false, so please check.

Also, there is no README and test, and CI seems to be failing. Should I address these issues?

The PR that made a similar change to this PR did not seem to have updated the README or test... #27422

Great! I can take care of the PR "requirements" ;)

Disabled readme and test reqs. This is a change to code that has existing tests. Will run through the cli test pipeline.

@aws-cdk-automation aws-cdk-automation dismissed their stale review October 30, 2024 10:21

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

aws-cdk-automation commented Oct 30, 2024

➡️ PR build request submitted to test-main-pipeline ⬅️

A maintainer must now check the pipeline and add the pr-linter/cli-integ-tested label once the pipeline succeeds.

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Oct 30, 2024

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

aws-cdk-automation commented Oct 30, 2024

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: f2a66d4
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit 8e4c247 into aws:main Oct 30, 2024
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Oct 30, 2024

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 30, 2024

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 30, 2024
@ren-yamanashi ren-yamanashi deleted the update-typescript-init-template branch November 10, 2025 02:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@mrgrain mrgrain mrgrain approved these changes

@aws-cdk-automation aws-cdk-automation aws-cdk-automation left review comments

Assignees

No one assigned

Labels

beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK p2 pr-linter/exempt-integ-test The PR linter will not require integ test changes pr-linter/exempt-readme The PR linter will not require README changes pr-linter/exempt-test The PR linter will not require test changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ren-yamanashi @mrgrain @aws-cdk-automation