-
Notifications
You must be signed in to change notification settings - Fork 4.3k
chore: make all L2 Constructs property injectable during release #34328
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
Changes from 2 commits
c9133dd
cfb09f8
33f4dc2
d53558f
273544b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import { ClassDeclaration, IndentationText, Project, PropertyDeclaration, QuoteKind, SourceFile, Symbol, SyntaxKind } from "ts-morph"; | ||
| import { ClassDeclaration, IndentationText, Project, PropertyDeclaration, QuoteKind, Scope, SourceFile, Symbol, SyntaxKind } from "ts-morph"; | ||
| import * as path from "path"; | ||
| import * as fs from "fs"; | ||
| // import { exec } from "child_process"; | ||
|
|
@@ -152,10 +152,134 @@ export class ConstructsUpdater extends MetadataUpdater { | |
| const classes = this.getCdkResourceClasses(sourceFile.getFilePath()); | ||
| for (const resource of classes) { | ||
| this.addImportAndMetadataStatement(resource.sourceFile, resource.filePath, resource.node); | ||
| this.makeConstructsPropInjectable(resource.sourceFile, resource.filePath, resource.node); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * This makes a Construct Property Injectable by doing 3 things: | ||
| * - add PROPERTY_INJECTION_ID property | ||
| * - import propertyInjectable from core/lib/prop-injectable | ||
| * - add class decorator @propertyInjectable | ||
| * | ||
| * If the Construct already has PROPERTY_INJECTION_ID, then skip it. | ||
| */ | ||
| private makeConstructsPropInjectable(sourceFile: SourceFile, filePath: string, node: ClassDeclaration) { | ||
| console.log(`path: ${filePath}, class: ${node.getName()}`); | ||
|
|
||
| if (this.isAlreadyInjectable(node)) { | ||
| return; // do nothing | ||
| } | ||
|
|
||
| // Add PROPERTY_INJECTION_ID | ||
| node.addProperty({ | ||
| scope: Scope.Public, | ||
| isStatic: true, | ||
| isReadonly: true, | ||
| name: 'PROPERTY_INJECTION_ID', | ||
| type: "string", | ||
| initializer: this.filePathToInjectionId(filePath, node.getName()), | ||
| }); | ||
| console.log(' Added PROPERTY_INJECTION_ID') | ||
|
|
||
| // Add Decorator | ||
| node.addDecorator({ | ||
| name: "propertyInjectable", | ||
| }); | ||
| console.log(' Added @propertyInjectable') | ||
|
|
||
| // import propertyInjectable | ||
| this.importPropertyInjectable(sourceFile, filePath); | ||
|
|
||
| // Write the updated file back to disk | ||
| sourceFile.saveSync(); | ||
| } | ||
|
|
||
| /** | ||
| * If the Construct already has PROPERTY_INJECTION_ID, then it is injectable already. | ||
| */ | ||
| private isAlreadyInjectable(classDeclaration: ClassDeclaration): boolean { | ||
| const properties: PropertyDeclaration[] = classDeclaration.getProperties(); | ||
| for (const prop of properties) { | ||
| if (prop.getName() === 'PROPERTY_INJECTION_ID') { | ||
| console.log(`Skipping ${classDeclaration.getName()}. It is already injectable`); | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * This converts the filePath | ||
| * '<HOME_DIR>/<CDK_HOME>/aws-cdk/packages/aws-cdk-lib/aws-apigateway/lib/api-key.ts' | ||
| * and className 'ApiKey' | ||
| * to 'aws-cdk-lib.aws-apigateway.ApiKey' | ||
| */ | ||
| private filePathToInjectionId(filePath: string, className: string | undefined): string { | ||
| if (!className) { | ||
| throw new Error('Could not build PROPERTY_INJECTION_ID if className is undefined'); | ||
| } | ||
|
|
||
| const start = 'packages/aws-cdk-lib/'; | ||
| const startIndex = filePath.indexOf(start); | ||
| const subPath = filePath.substring(startIndex + start.length); | ||
| const parts: string[] = subPath.split('\/'); | ||
| if (parts.length < 2) { | ||
| throw new Error(`Could not build PROPERTY_INJECTION_ID for ${filePath} ${className}`); | ||
| } | ||
| return `'aws-cdk-lib.${parts[0]}.${className}'`; | ||
| } | ||
|
|
||
| /** | ||
| * This returns the relative path of prop-injectable.ts. | ||
| */ | ||
| private getRelativePathForPropInjectionImport(filePath: string): string { | ||
| const absoluteFilePath = path.resolve(filePath); | ||
| const absoluteTargetPath = path.resolve(__dirname, '../../../../packages/aws-cdk-lib/core/lib/prop-injectable.ts'); | ||
| let relativePath = path.relative(path.dirname(absoluteFilePath), absoluteTargetPath).replace(/\\/g, "/").replace(/.ts/, ""); | ||
| if (absoluteFilePath.includes('@aws-cdk')) { | ||
| relativePath = 'aws-cdk-lib/core/lib/prop-injectable' | ||
| } | ||
| return relativePath; | ||
| } | ||
|
|
||
| /** | ||
| * This adds import of prop-injectable to the file. | ||
| */ | ||
| private importPropertyInjectable(sourceFile: SourceFile, filePath: string) { | ||
|
||
| const relativePath = this.getRelativePathForPropInjectionImport(filePath); | ||
|
|
||
| // Check if an import from 'prop-injectable' already exists | ||
| const existingImport = sourceFile.getImportDeclarations().find((stmt: any) => { | ||
| return stmt.getModuleSpecifier().getText().includes('/prop-injectable'); | ||
| }); | ||
| if (existingImport) { | ||
| return; | ||
| } | ||
|
|
||
| // Find the correct insertion point (after the last import before the new one) | ||
| const importDeclarations = sourceFile.getImportDeclarations(); | ||
| let insertIndex = importDeclarations.length; // Default to appending | ||
|
|
||
| for (let i = importDeclarations.length - 1; i >= 0; i--) { | ||
| const existingImport = importDeclarations[i].getModuleSpecifier().getLiteralText(); | ||
|
|
||
| // Insert the new import before the first one that is lexicographically greater | ||
| if (existingImport.localeCompare(relativePath) > 0) { | ||
| insertIndex = i; | ||
| } else { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // Insert the new import at the correct index | ||
| sourceFile.insertImportDeclaration(insertIndex, { | ||
| moduleSpecifier: relativePath, | ||
| namedImports: [{ name: "propertyInjectable" }], | ||
| }); | ||
| console.log(` Added import for propertyInjectable in file: ${filePath} with relative path: ${relativePath}`); | ||
| } | ||
|
|
||
| /** | ||
| * Add the import statement for MetadataType to the file. | ||
|
|
||
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.
What about alpha modules?
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.
Sorry, I am not familiar with alpha modules. Can you give me some examples and I can add them to the unit tests.
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.
Modules in
packages/@aws-cdk/are alpha modules (aka experimental modules). The constructs in these modules should also be property injectable.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.
I updated the logic to handle /packages/@awsk-cdk/...