I am running into an issue now with imports like this:

https://github.com/aws/aws-cdk/blob/master/packages/@aws-cdk/aws-redshift/test/cluster.test.ts#L6

import { CfnCluster, Cluster, ClusterParameterGroup, ClusterSubnetGroup, ClusterType } from '../lib';

This one is currently failing the PR Build, and there are probably more.

I see two solutions here:

1. enforce in v1 that Cfn imports are separate, and actually directly reference the generated file, not just lib. That would rewrite the above to the following on master, and have a linter rule that requires Cfn imports to be separate. With this option, the logic I've written here won't have to change.

import { Cluster, ClusterParameterGroup, ClusterSubnetGroup, ClusterType } from '../lib';
import { CfnCluster } from '../lib/redshift.generated';

2. In rewrite.ts and copy-files-removing-deps.ts, write logic to directly transform to the below when running transform.sh.

import { Cluster, ClusterParameterGroup, ClusterSubnetGroup, ClusterType } from '../lib';
import { CfnCluster } from 'aws-cdk-lib/aws-redshift';

I think option 2 would be ideal, but I am concerned that changing one line into two in the v2 rewrite tools will get messy.

1. enforce in v1 that Cfn imports are separate, and actually directly reference the generated file, not just lib. That would rewrite the above to the following on master, and have a linter rule that requires Cfn imports to be separate. With this option, the logic I've written here won't have to change.

It appears there are 11 usages of loading Cfn* types from something that's not @aws-cdk/core or *.generated:

% git grep "import .*\bCfn.*" | grep -v generated | grep -v '@aws-cdk/core'
packages/@aws-cdk/aws-appmesh/lib/header-match.ts:import { CfnRoute } from './index';
packages/@aws-cdk/aws-athena/test/athena.test.ts:import { CfnWorkGroup } from '../lib';
packages/@aws-cdk/aws-athena/test/integ.workgroup.ts:import { CfnWorkGroup } from '../lib';
packages/@aws-cdk/aws-cloudfront/test/test-origin.ts:import { CfnDistribution, IOrigin, OriginBase, OriginBindConfig, OriginBindOptions, OriginProps, OriginProtocolPolicy } from '../lib';
packages/@aws-cdk/aws-iam/test/policy.test.ts:import { AnyPrincipal, CfnPolicy, Group, Policy, PolicyDocument, PolicyStatement, Role, ServicePrincipal, User } from '../lib';
packages/@aws-cdk/aws-msk/lib/cluster.ts:import { CfnCluster, KafkaVersion } from './';
packages/@aws-cdk/aws-redshift/test/cluster.test.ts:import { CfnCluster, Cluster, ClusterParameterGroup, ClusterSubnetGroup, ClusterType } from '../lib';
packages/@aws-cdk/aws-sam/test/application.test.ts:import { CfnApplication } from '../lib';
packages/@aws-cdk/cfnspec/lib/index.ts:import { CfnLintFileSchema } from './_private_schema/cfn-lint';
packages/awslint/lib/rules/module.ts:import { CfnResourceReflection } from './cfn-resource';
packages/awslint/lib/rules/resource.ts:import { CfnResourceReflection } from './cfn-resource';

Of those, 7 are only importing the L1s, so there's only 4 statements that would conflict with the proposed rule (as long as we exclude imports from @aws-cdk/core).

However, I worry a bit about us dodging this problem for named imports and missing the complexity of module imports:

import * as foo from '@aws-cdk/aws-foo';
/// ...
new foo.Flubber(...);
new foo.CfnFizzBuzz(...);
///

The above case will also require us to create the new import statements. If we're going to have to do it for that case, we might just want to handle it for both cases.

So Option 2 is probably preferable in the long run.

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
• Commit ID: 1c03976
• Result: FAILED
• Build Logs (available for 30 days)

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

After a lot of investigation into the options here, I think the best thing is to require that the L1 imports that we support rewriting in this tool are in a "normal" format. The three options are:

1. enforce in v1 that Cfn imports are separate, and actually directly reference the generated file, not just lib. That would rewrite the above to the following on master, and have a linter rule that requires Cfn imports to be separate. With this option, the logic I've written here won't have to change. Implemented in this PR: chore(individual-pkg-gen): remove L1s from alpha modules #16481

2. In rewrite.ts and copy-files-removing-deps.ts, write logic to directly transform Cfn imports using text manipulation. I implemented this one in another PR: chore(individual-pkg-gen): remove L1s from alpha modules, and support… #16482. It is super hacky, and I don't really trust it to not cause headaches for us in the future. Open to suggestions on how to make this one better.

3. In rewrite.ts and copy-files-removing-deps.ts, write logic to directly transform the Cfn imports (and all imports that need updating) using ts.updateXX() apis to manipulate the AST nodes. I explored this option in this PR: chore(individual-pkg-gen): remove L1s from alpha modules, and rewrite… #16483 It is not really working fully yet. But I have hit a roadblock, more description in the PR.

I think we should go with option 1. @njlynch, @rix0rrr, @nija-at, thoughts?

Closing this PR in favor of one of the above 3 :)

Option 1 seems the best to me. Trying to manipulate typescript text or nodes is just going to make this harder. Option 3 if it is simple and clean. Option 2 just sounds bad.