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

Deep typescript client imports no longer import all clients #3398

Merged

Conversation

JakeGinnivan
Copy link
Contributor

@JakeGinnivan JakeGinnivan commented Aug 15, 2020

Separates global config from service config

ConfigurationServicePlaceholders references all clients, this means when a single client is deep imported it still transitively references all clients.

The impact of this is TypeScript needs to parse all type definitions in the SDK, and there is no way for a project to avoid this.

By separating client config from global config clients can be deep imported without pulling all type definitions in, greatly reducing build time

Fixes #3034

I created a simple repo demonstrating the issue at https://github.com/JakeGinnivan/aws-sdk-deep-imports. I then used patch-package to apply this change directly to node modules, giving me the before/after.

For ease of reviewing I have separated the regeneration of the client definitions from the main changes, if you check out the first commit it contains the actual changes.

Example program

import S3 from 'aws-sdk/clients/s3'

Before

Files:                         259
Lines:                      439618
Memory used:               375717K
Parse time:                  2.19s
Program time:                2.62s

After

Files:                         22
Lines:                      31187
Memory used:               54651K
Parse time:                 0.36s
Program time:               0.43s
Checklist
  • .d.ts file is updated
  • changelog is added, npm run add-change

ConfigurationServicePlaceholders references all clients, this means when a single client is deep imported it still transitively references all clients.

The impact of this is TypeScript needs to parse all type definitions in the SDK, and there is no way for a project to avoid this.

By separating client config from global config clients can be deep imported without pulling all type definitions in, greatly reducing build time
@aws-sdk-js-automation
Copy link

AWS CodeBuild CI Report

  • CodeBuild project: sdk-v2-github
  • Commit ID: c0b82a1
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@JakeGinnivan
Copy link
Contributor Author

@trivikr any chance you could take a look at this. It would make a huge difference to anyone using the aws-sdk in a TypeScript project

@JakeGinnivan
Copy link
Contributor Author

The second commit is just regenerating the clients, ea14d77 contains the functional changes which is pretty small.

@trivikr
Copy link
Member

trivikr commented Aug 25, 2020

@JakeGinnivan Thank you for the PR!

AWS JS SDK team will prioritize reviewing this PR today (8/25)

@ankon
Copy link

ankon commented Sep 16, 2020

FWIW: Before this change one could import ConfigurationOptions from aws-sdk/lib/config, now this is exported from config-base instead. Same for some other types.

This broke one of our users of the aws-sdk, and I had to explicitly bump the dependency in that user.

@JakeGinnivan
Copy link
Contributor Author

Ah apologies, did not think about that. I can put up another PR which re-exports the types from config-base in config fixing the breaking change

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.

Too much memory used for typescript compilation
5 participants