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

RFC 460: Reduce aws-cdk-lib Package Size #430

Merged
merged 54 commits into from
Oct 14, 2022

Conversation

madeline-k
Copy link
Contributor

@madeline-k madeline-k commented Apr 26, 2022

Rendered Version

This is a request for comments about Reduce aws-cdk-lib Package Size. See #460 for
additional details.

APIs are signed off by @otaviomacedo.


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

@madeline-k madeline-k changed the title Madeline k/reduce module size RFC 39: Reduce aws-cdk-lib Package Size Apr 26, 2022
text/0039-reduce-module-size.md Outdated Show resolved Hide resolved
text/0039-reduce-module-size.md Outdated Show resolved Hide resolved
text/0039-reduce-module-size.md Outdated Show resolved Hide resolved
text/0039-reduce-module-size.md Outdated Show resolved Hide resolved
text/0039-reduce-module-size.md Outdated Show resolved Hide resolved
text/0039-reduce-module-size.md Outdated Show resolved Hide resolved
text/0039-reduce-module-size.md Outdated Show resolved Hide resolved
Copy link
Contributor

@skinny85 skinny85 left a comment

Choose a reason for hiding this comment

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

Awesome doc!

text/0039-reduce-module-size.md Outdated Show resolved Hide resolved
text/0039-reduce-module-size.md Outdated Show resolved Hide resolved
text/0039-reduce-module-size.md Outdated Show resolved Hide resolved
text/0039-reduce-module-size.md Outdated Show resolved Hide resolved
text/0039-reduce-module-size.md Outdated Show resolved Hide resolved
text/0039-reduce-module-size.md Outdated Show resolved Hide resolved
text/0039-reduce-module-size.md Outdated Show resolved Hide resolved
class from the packages in step 1. We will need to do something clever here
to make sure customers do not have IDE or compilation errors before the CDK
CLI has an opportunity to download and install the lambda layer packages.
3. Modify the CDK CLI to verify that the correct packages are available in the
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have any idea how will we implement that? Can we use optional dependencies?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I still need to work on a POC to see how this will actually be implemented. Thanks for the optional dependencies suggestion, that looks like it would work. I will dive deeper and then update this section!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've tested this with the POC jsii library, and if we add a package as an optional dependency, then the following happens:

  1. The optional dependency is not included in the jsii packaged bundle for npm or PyPI
  2. When you run npm install in a node project that lists the jsii library as a dependency, then the optional dependency is automatically installed as well.
  3. You can run npm install --no-optional to avoid installing the optional dependencies
  4. This has no impact on the experience for target languages (python).

So, this would only impact TS/JS users. The argument for using optional dependencies is that this could make it easier for TS/JS users, because the optional dependencies would be installed by default. The argument against is that TS/JS users will need to change their current workflow to avoid installing things they don't need. At first, I don't think we should include this. Let's wait to see if there are real customer issues that this would alleviate.

text/0039-reduce-module-size.md Show resolved Hide resolved
text/0039-reduce-module-size.md Show resolved Hide resolved
text/0039-reduce-module-size.md Outdated Show resolved Hide resolved
@aws aws deleted a comment from otaviomacedo Apr 27, 2022
@madeline-k
Copy link
Contributor Author

This RFC is now in the final comments period. It will be merged by next week.

@madeline-k
Copy link
Contributor Author

PR Lint is failing because of a different RFC document

kaizencc
kaizencc previously approved these changes Oct 14, 2022
@mergify mergify bot dismissed kaizencc’s stale review October 14, 2022 21:16

Pull request has been modified.

@kaizencc kaizencc merged commit 093a960 into master Oct 14, 2022
@kaizencc kaizencc deleted the madeline-k/reduce-module-size branch October 14, 2022 21:21
@madeline-k madeline-k changed the title RFC 39: Reduce aws-cdk-lib Package Size RFC 460: Reduce aws-cdk-lib Package Size Oct 14, 2022
@madeline-k madeline-k mentioned this pull request Oct 14, 2022
11 tasks
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.

10 participants