-
Notifications
You must be signed in to change notification settings - Fork 252
feat(@jsii/spec): add a utility library to be consumed by jsii modules #3570
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RomainMuller my game plan is to iterate on this PR to your fancy, and then modify the previous PR to use this library (and address the rest of your comments) when its merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If they both exist... there might be an issue -- in particular, this will ALWAYS prefer loading the uncompressed assembly, when it should likely prefer loading the compressed one if it exists...
A possible solution would be to ALWAYS load
.jsii
, but if there is a compressed assembly, that file basically contains something along the lines of:{ "schema": "jsii/file-redirect", "compression": "gzip", "filename": ".jsii.gz" }The loader would detect this... and proceed to load (and uncompress)
.jsii.gz
as indicated.
@RomainMuller if you want this, i am happy to do things this way. but as it stands, i simply changed to preferring the compressed file if it exists, and i don't really see what always writing the .jsii file will achieve, and in fact, i think it might be worse.
we also talked offline about hacking the .jsii file to throw a descriptive error when an old jsii version is being used. i couldn't really get this to work, because the only error i can throw is Unexpected token E in JSON at position 0
, which gives us 1 character of description to work with 😂. if what we write in .jsii is actual JSON, then the loader will simply load the .jsii file normally. Since we don't really enforce validating the assembly, i think it's entirely possible that users get ugly non-descriptive errors when they try to access the nonexistent assembly['name']
.
my conclusion is that the best we can do is throw the error that the .jsii
file does not exist in these situations. At least that alerts the user that the issue is at the level of a missing .jsii
file, and not that their .jsii
file is malformed. let me know if that conclusion makes sense!
Using that "redirect" placeholder gives us a new point where we can detect future "you need to upgrade your toolchain" situations... We can actually insert an unconditional validation of the contents of the Some of the stuff we could end up doing in the future include (not that we are planning to do any of these at this time, but I could see those as possible future directions):
Basically it would help crystallize the contract that you can ALWAYS start by looking at We could also have a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this nears the finish line... how do we feel about the name @jsii/utils
? Currently utils
is too broad of a term, I think. I plan to add functions for writing/loading .jsii.tabl.json
files as well, maybe also writing/loading package.json
files.
Since these functions seem to govern input/output, is there a better term that we can name this module? @jsii/io
?
packages/jsii/package.json
Outdated
@@ -37,6 +37,7 @@ | |||
"dependencies": { | |||
"@jsii/check-node": "0.0.0", | |||
"@jsii/spec": "^0.0.0", | |||
"@jsii/utils": "^0.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be a related entry of references
in tsconfig.json
.
Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it! |
Merging (with squash)... |
This PR utilizes the functions added in #3570 everywhere in the jsii monorepo. The benefit of this is two-fold -- it refactors all places where we load/read assemblies and points them to the source of truth (introduced in #3570). This also means the logic for compressing assemblies will be available for all packages in the jsii monorepo if/when we flip that switch. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
Updates `cdk-generate-synthetic-examples` to use the functions introduced in aws/jsii#3570 where appropriate.
This is part of the Reduce Package Size [project](https://github.com/aws/aws-cdk/projects/15). Assembly compression is supported in jsii v1.64.0 (see this jsii [PR](aws/jsii#3570) for details + discussion about how compressed assemblies are handled). This PR includes the following: - `--compress-assembly` flag turned on for `aws-cdk-lib` only - `run-rosetta.sh` depends on `cdk-generate-synthetic-examples@^0.1.14` which is the minimum version with compressed assembly [support](cdklabs/cdk-generate-synthetic-examples#16). ---- This `--compress-assembly` flag was tested in the following way: - this branch was pushed to `test-main-pipeline` and successfully built in the test pipeline. - the build artifact was downloaded from s3, unzipped, and verified to have the correct `.jsii` and `.jsii.gz` structure in both TypeScript and Python. - the build artifact was used to synth and deploy basic CDK apps in TypeScript and Python. - the build artifact was privately published to a codeartifact repository. - the codeartifact repository was used as a source to a local instance of construct hub. - verified that the published library was ingested correctly by the ingestion lambda and shows up on construct hub. - verified (via eye test, nothing too rigorous) that documentation was generated correctly on construct hub, including generated examples. ---- On code artifact, the package size shows up as 54 MB, compared to 194 MB, which is the size of the current package in npm. ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This is part of the Reduce Package Size [project](https://github.com/aws/aws-cdk/projects/15). Assembly compression is supported in jsii v1.64.0 (see this jsii [PR](aws/jsii#3570) for details + discussion about how compressed assemblies are handled). This PR includes the following: - `--compress-assembly` flag turned on for `aws-cdk-lib` only - `run-rosetta.sh` depends on `cdk-generate-synthetic-examples@^0.1.14` which is the minimum version with compressed assembly [support](cdklabs/cdk-generate-synthetic-examples#16). ---- This `--compress-assembly` flag was tested in the following way: - this branch was pushed to `test-main-pipeline` and successfully built in the test pipeline. - the build artifact was downloaded from s3, unzipped, and verified to have the correct `.jsii` and `.jsii.gz` structure in both TypeScript and Python. - the build artifact was used to synth and deploy basic CDK apps in TypeScript and Python. - the build artifact was privately published to a codeartifact repository. - the codeartifact repository was used as a source to a local instance of construct hub. - verified that the published library was ingested correctly by the ingestion lambda and shows up on construct hub. - verified (via eye test, nothing too rigorous) that documentation was generated correctly on construct hub, including generated examples. ---- On code artifact, the package size shows up as 54 MB, compared to 194 MB, which is the size of the current package in npm. ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This is part of the Reduce Package Size [project](https://github.com/aws/aws-cdk/projects/15). Assembly compression is supported in jsii v1.64.0 (see this jsii [PR](aws/jsii#3570) for details + discussion about how compressed assemblies are handled). This PR includes the following: - `--compress-assembly` flag turned on for `aws-cdk-lib` only - `run-rosetta.sh` depends on `cdk-generate-synthetic-examples@^0.1.14` which is the minimum version with compressed assembly [support](cdklabs/cdk-generate-synthetic-examples#16). ---- This `--compress-assembly` flag was tested in the following way: - this branch was pushed to `test-main-pipeline` and successfully built in the test pipeline. - the build artifact was downloaded from s3, unzipped, and verified to have the correct `.jsii` and `.jsii.gz` structure in both TypeScript and Python. - the build artifact was used to synth and deploy basic CDK apps in TypeScript and Python. - the build artifact was privately published to a codeartifact repository. - the codeartifact repository was used as a source to a local instance of construct hub. - verified that the published library was ingested correctly by the ingestion lambda and shows up on construct hub. - verified (via eye test, nothing too rigorous) that documentation was generated correctly on construct hub, including generated examples. ---- On code artifact, the package size shows up as 54 MB, compared to 194 MB, which is the size of the current package in npm. ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This is part of the Reduce Package Size [project](https://github.com/aws/aws-cdk/projects/15). Assembly compression is supported in jsii v1.64.0 (see this jsii [PR](aws/jsii#3570) for details + discussion about how compressed assemblies are handled). This PR includes the following: - `--compress-assembly` flag turned on for `aws-cdk-lib` only - `run-rosetta.sh` depends on `cdk-generate-synthetic-examples@^0.1.14` which is the minimum version with compressed assembly [support](cdklabs/cdk-generate-synthetic-examples#16). ---- This `--compress-assembly` flag was tested in the following way: - this branch was pushed to `test-main-pipeline` and successfully built in the test pipeline. - the build artifact was downloaded from s3, unzipped, and verified to have the correct `.jsii` and `.jsii.gz` structure in both TypeScript and Python. - the build artifact was used to synth and deploy basic CDK apps in TypeScript and Python. - the build artifact was privately published to a codeartifact repository. - the codeartifact repository was used as a source to a local instance of construct hub. - verified that the published library was ingested correctly by the ingestion lambda and shows up on construct hub. - verified (via eye test, nothing too rigorous) that documentation was generated correctly on construct hub, including generated examples. ---- On code artifact, the package size shows up as 54 MB, compared to 194 MB, which is the size of the current package in npm. ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Adds a set of utility functions under
@jsii/spec
that can (and will) be leveraged by other jsii modules.Currently has the following functions exposed:
getAssemblyFile()
writeAssembly()
loadAssemblyFromPath()
loadAssemblyFromFile()
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.