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

feat: remove support for down-leveling of TypeScript declarations files to ts3.9 to improve performance #1474

Merged
merged 10 commits into from
Nov 13, 2024

Conversation

HBobertz
Copy link
Contributor

@HBobertz HBobertz commented Nov 12, 2024

BREAKING CHANGE: This PR removes the TypeScript declaration file down-leveling feature from the JSII compiler to address significant performance issues. Our benchmarks show that >50% of the JSII compile time is spent in the downlevel-dts dependency, causing JSII to be 2.7x slower than standard TypeScript compilation. The down-leveling feature was originally added to support TypeScript versions down to 3.9, but analysis shows minimal usage - none of our AWS CDK codebase uses this feature, and only about 12 non-fork repositories on GitHub would be affected by its removal, additionally typescript 3.9 and earlier have been EOL for quite a while on definitively typed.

If you found this pr because removing this feature impacted your builds, it is likely because you have this pattern somewhere in your codebase. Simply refactor that pattern and rebuild. Ensure you also are not seeing the below typesVersion in your app's package.json

  "typesVersions": {
    "<=3.9": {
      "*": [
        ".types-compat/ts3.9/*",
        ".types-compat/ts3.9/*/index.d.ts"
      ]
    }
  },

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

@HBobertz
Copy link
Contributor Author

Getting test failures I'm not getting locally. I'll figure that out

@mrgrain
Copy link
Contributor

mrgrain commented Nov 12, 2024

Let's probably not make this a chore but a fix.

@HBobertz HBobertz changed the title chore: remove downlevel-dts for performance improvement purposes fix: remove downlevel-dts for performance improvement purposes Nov 12, 2024
@HBobertz HBobertz marked this pull request as ready for review November 12, 2024 19:13
@mrgrain mrgrain changed the title fix: remove downlevel-dts for performance improvement purposes fix!: remove down-leveling of TypeScript declarations files to TS3.9 to improve performance Nov 12, 2024
src/compiler.ts Outdated Show resolved Hide resolved
test/compiler.test.ts Outdated Show resolved Hide resolved
Copy link
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.

small changes left

Signed-off-by: github-actions <[email protected]>
@aws-cdk-automation aws-cdk-automation added this pull request to the merge queue Nov 13, 2024
@mrgrain mrgrain removed this pull request from the merge queue due to a manual request Nov 13, 2024
@mrgrain mrgrain added this pull request to the merge queue Nov 13, 2024
@mrgrain mrgrain removed this pull request from the merge queue due to a manual request Nov 13, 2024
@mrgrain mrgrain changed the title fix!: remove down-leveling of TypeScript declarations files to TS3.9 to improve performance feat!: remove support for down-leveling of TypeScript declarations files to ts3.9 to improve performance Nov 13, 2024
@mrgrain mrgrain changed the title feat!: remove support for down-leveling of TypeScript declarations files to ts3.9 to improve performance feat: remove support for down-leveling of TypeScript declarations files to ts3.9 to improve performance Nov 13, 2024
@mrgrain mrgrain added this pull request to the merge queue Nov 13, 2024
Merged via the queue into main with commit 7be6ace Nov 13, 2024
73 checks passed
@mrgrain mrgrain deleted the bobertzh/deprecate-downlevel-dts branch November 13, 2024 15:29
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