chore(core): embedded export for cfn-parse by kaizencc · Pull Request #19773 · aws/aws-cdk

kaizencc · 2022-04-05T21:55:14Z

This PR moves cfn-parse.ts under a folder within @aws-cdk/core/lib, where it is "secretly" exported. You can access the API of cfn-parse by importing import * as cfn_parse from '@aws-cdk/core/lib/cfn-parse';.

The motivation behind this change is to stabilize the code generated by cfn2ts. cfn2ts imports cfn_parse but until now, this was not possible in v2 since aws-cdk-lib is export-restricted. This change should allow cdk v2 users the ability to generate L1 code via cfn2ts.

Closes #18037.

All Submissions:

Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

This PR adds new unconventional dependencies following the process described here

New Features

Have you added the new feature to an integration test?
- Did you use cdk-integ to deploy the infrastructure and generate the snapshot (i.e. cdk-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

gitpod-io · 2022-04-05T21:55:20Z

rix0rrr

Excellent, thanks! Can you drop in a cfn-parse.ts at the old location as well which re-exports the new cfn-parse.ts ?

That's so that old imports at the old location still work.

rix0rrr

Also, maybe not name the internal helper folder cfn-parse, but something like internal-helpers or something. Then we have an obvious point if we ever need to add more stuff like this.

And doesn't the package.json need to be updated as well?

kaizencc

Currently this is targeting master. But I think this is a breaking change to core since we are restricting deep imports there with the package.json changes. So should we target v2-main instead? Or am I missing something.

kaizencc · 2022-04-06T16:57:47Z

tools/@aws-cdk/cfn2ts/lib/codegen.ts

-    // explicitly import the cfn-parse.ts file from @core, which is not part of the public API of the module
-    this.code.line(`import * as ${CFN_PARSE} from '${coreImport}/${coreImport === '.' ? '' : 'lib/'}cfn-parse';`);
+    // import cfn-parse from an embedded folder inside @core, since it is not part of the public API of the module
+    this.code.line(`import * as ${CFN_PARSE} from '${coreImport}/${coreImport === '.' ? '' : 'lib/'}helpers-internal';`);


Here's my understanding, please correct me if I'm off-base. The old import of @aws-cdk/core/lib/cfn-parse will work for internal cdk modules. They will not work for external users of cfn2ts because I am not exporting it out of package.json. So this does need to change to @aws-cdk/core/lib/helpers-internal.

kaizencc · 2022-04-06T16:58:04Z

packages/@aws-cdk/core/package.json

+    "./package.json": "./package.json",
+    "./.jsii": "./.jsii",
+    "./.warnings.jsii.js": "./.warnings.jsii.js",
+    "./lib/helpers-internal": "./lib/helpers-internal/index.js"


named helpers-internal because that is what it is in pipelines

mergify · 2022-04-07T08:08:51Z

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

aws-cdk-automation · 2022-04-07T08:52:45Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: d71a02b
Result: SUCCEEDED
Build Logs (available for 30 days)

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

mergify · 2022-04-07T08:53:03Z

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

This should have been a part of #19773. Ensures that old imports of `@aws-cdk/core/lib/cfn-parse` still work, so that it is not a breaking change. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/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/master/INTEGRATION_TESTS.md)? * [ ] Did you use `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-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 PR moves `cfn-parse.ts` under a folder within `@aws-cdk/core/lib`, where it is "secretly" exported. You can access the API of `cfn-parse` by importing `import * as cfn_parse from '@aws-cdk/core/lib/cfn-parse';`. The motivation behind this change is to stabilize the code generated by `cfn2ts`. `cfn2ts` imports `cfn_parse` but until now, this was not possible in v2 since `aws-cdk-lib` is export-restricted. This change should allow cdk v2 users the ability to generate L1 code via `cfn2ts`. Closes aws#18037. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/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/master/INTEGRATION_TESTS.md)? * [ ] Did you use `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-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 should have been a part of aws#19773. Ensures that old imports of `@aws-cdk/core/lib/cfn-parse` still work, so that it is not a breaking change. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/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/master/INTEGRATION_TESTS.md)? * [ ] Did you use `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-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*

Due to an oversight, embedded exports from core were not exported in cdk v2. This is necessary for #19773 to work. The export logic added in that PR would not get executed for `core`, but it is supposed to. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/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/master/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*

Due to an oversight, embedded exports from core were not exported in cdk v2. This is necessary for aws#19773 to work. The export logic added in that PR would not get executed for `core`, but it is supposed to. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/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/master/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*

kaizencc added 2 commits April 5, 2022 17:36

chore: embedded export for cfn-parse

4617c8e

change comment

705d2f9

github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1 labels Apr 5, 2022

kaizencc requested review from madeline-k and rix0rrr April 5, 2022 21:55

aws-cdk-automation requested a review from a team April 5, 2022 21:55

mergify bot added the contribution/core This is a PR that came from AWS. label Apr 5, 2022

rix0rrr requested changes Apr 6, 2022

View reviewed changes

pr comments

8adcc39

kaizencc commented Apr 6, 2022

View reviewed changes

kaizencc requested a review from rix0rrr April 6, 2022 17:00

kaizencc and others added 3 commits April 6, 2022 17:04

modules outside of core must import from helpers-internal

dc48372

remove cfn-parse

730f369

Make "exports" section for core only appear in v2

6f885fe

rix0rrr approved these changes Apr 7, 2022

View reviewed changes

Merge branch 'master' into conroy/cfn-parse

d71a02b

mergify bot merged commit f82d96b into master Apr 7, 2022

mergify bot deleted the conroy/cfn-parse branch April 7, 2022 08:53

kaizencc mentioned this pull request Apr 7, 2022

chore: add mistakenly dropped cfn-parse.ts to core #19804

Merged

4 tasks

kaizencc mentioned this pull request May 3, 2022

fix(ubergen): expose exports in core module for v2 #20176

Merged

4 tasks

This was referenced May 5, 2022

cli: Cloud assembly schema version mismatch with latest versions #20121

Closed

Support for schema version 18.0.0 on AWS CDK v2 #20135

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(core): embedded export for cfn-parse#19773

chore(core): embedded export for cfn-parse#19773
mergify[bot] merged 7 commits intomasterfrom
conroy/cfn-parse

kaizencc commented Apr 5, 2022

Uh oh!

gitpod-io bot commented Apr 5, 2022

Uh oh!

rix0rrr left a comment

Uh oh!

rix0rrr left a comment

Uh oh!

kaizencc left a comment

Uh oh!

kaizencc Apr 6, 2022

Uh oh!

kaizencc Apr 6, 2022

Uh oh!

mergify bot commented Apr 7, 2022

Uh oh!

aws-cdk-automation commented Apr 7, 2022

Uh oh!

mergify bot commented Apr 7, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

kaizencc commented Apr 5, 2022

All Submissions:

Adding new Unconventional Dependencies:

New Features

Uh oh!

gitpod-io bot commented Apr 5, 2022

Uh oh!

rix0rrr left a comment

Choose a reason for hiding this comment

Uh oh!

rix0rrr left a comment

Choose a reason for hiding this comment

Uh oh!

kaizencc left a comment

Choose a reason for hiding this comment

Uh oh!

kaizencc Apr 6, 2022

Choose a reason for hiding this comment

Uh oh!

kaizencc Apr 6, 2022

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Apr 7, 2022

Uh oh!

aws-cdk-automation commented Apr 7, 2022

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Apr 7, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants