feat(inject-migration): add option for ES private field support (#382)

* feat(inject-migration): add option for ES private field support * fix(inject-migration): remove the 'private' keyword from source code * fix(inject-migration): update references outside constructor too * feat(inject-migration): handle public and protected fields from being converted * feat(inject-migration): add the missing test for --includeReadonlyByDefault option * fix(inject-migration): fix typo at schema json property * fix(inject-migration): resolve some feedback nits
ngxtension · May 29, 2024 · f27cabd · f27cabd
1 parent 9bef713
commit f27cabd
Show file tree

Hide file tree

Showing 6 changed files with 301 additions and 8 deletions.
diff --git a/docs/src/content/docs/utilities/Migrations/inject-migration.md b/docs/src/content/docs/utilities/Migrations/inject-migration.md
@@ -139,6 +139,7 @@ export class AppComponent {
 - `--project`: Specifies the name of the project.
 - `--path`: Specifies the path to the file to be migrated.
 - `--includeReadonlyByDefault`: Specifies whether to include the readonly keyword by default for the injections. Default is `false`.
+- `--useESPrivateFieldNotation`: Specifies whether to replace TS `private` modifier with ES `#private` field notation. Default is `false`.
 
 #### Include readonly by default
 
@@ -165,6 +166,31 @@ export class AppComponent {
 }
 ```
 
+#### Use ES private field notation
+
+By default, the migration will keep the `private` keyword to the injected dependencies. If you want to add replace TS `private` modifier with ES `#private` field notation to the injected dependencies you can set the `--useESPrivateFieldNotation` option to `true`.
+
+```typescript
+import { Component } from '@angular/core';
+import { MyService } from './my-service';
+
+@Component()
+export class AppComponent {
+	constructor(private myService: MyService) {}
+}
+```
+
+```typescript
+import { Component } from '@angular/core';
+import { MyService } from './my-service';
+
+@Component()
+export class AppComponent {
+	// will replace 'private' modifier with ES '#private' field notation if the option is set to true
+	readonly #myService = inject(MyService);
+}
+```
+
 ### Usage
 
 In order to run the schematics for all the project in the app you have to run the following script:

diff --git a/libs/plugin/src/generators/convert-di-to-inject/__snapshots__/generator.spec.ts.snap b/libs/plugin/src/generators/convert-di-to-inject/__snapshots__/generator.spec.ts.snap
@@ -1,5 +1,113 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`convertDiToInjectGenerator should add the readonly keyword to converted fields 1`] = `
+"
+    import { Component, inject, HostAttributeToken } from '@angular/core';
+    @Component({
+      template: ''
+    })
+    export class MyComponent {
+      private readonly type = inject<string>(new HostAttributeToken('type'));
+      private readonly service = inject<MyService>('my-service' as any /* TODO(inject-migration): Please check if the type is correct */);
+      private readonly service4 = inject(MyService4);
+      private readonly service5 = inject<MyService5>('my-service2' as any /* TODO(inject-migration): Please check if the type is correct */, { optional: true });
+      private readonly someToken = inject(SOME_TOKEN);
+      private readonly service2 = inject(MyService2);
+      service3: MyService3;
+
+      constructor(
+) {
+            const service3 = inject(MyService3);
+
+        this.service3 = service3;
+      }
+    }
+"
+`;
+
+exports[`convertDiToInjectGenerator should add the readonly keyword to converted native private fields 1`] = `
+"
+    import { Component, inject, HostAttributeToken } from '@angular/core';
+    @Component({
+      template: ''
+    })
+    export class MyComponent {
+      readonly #type = inject<string>(new HostAttributeToken('type'));
+      readonly #service = inject<MyService>('my-service' as any /* TODO(inject-migration): Please check if the type is correct */);
+      readonly #service4 = inject(MyService4);
+      readonly #service5 = inject<MyService5>('my-service2' as any /* TODO(inject-migration): Please check if the type is correct */, { optional: true });
+      readonly #someToken = inject(SOME_TOKEN);
+      readonly #service2 = inject(MyService2);
+      service3: MyService3;
+
+      constructor(
+) {
+            const service3 = inject(MyService3);
+
+        this.service3 = service3;
+      }
+    }
+"
+`;
+
+exports[`convertDiToInjectGenerator should convert private modifier fields to native private fields 1`] = `
+"
+    import { Component, inject } from '@angular/core';
+    @Component({
+      template: ''
+    })
+    export class MyComponent {
+      #myService = inject(MyService);
+      #elRef = inject<ElementRef<HtmlImageElement>>(ElementRef<HtmlImageElement>);
+      #tplRef = inject<TemplateRef<any>>(TemplateRef<any>);
+      #viewContainerRef = inject(ViewContainerRef);
+
+      constructor(
+) {
+            const service2 = inject(MyService2);
+
+        this.#myService.doSomething();
+
+        service2.doSomethingElse();
+
+        service2.doSomething();
+
+        someList.forEach(() => {
+          // nested scope
+          this.#myService.doSomething();
+        });
+
+        // use service in a function call
+        someFunction(service2).test(this.#myService);
+
+        this.service7.getTask();
+        this.service8.getTick();
+      }
+
+      #service = inject<MyService>('my-service' as any /* TODO(inject-migration): Please check if the type is correct */);
+      #service4 = inject(MyService4);
+      #service5 = inject<MyService5>('my-service2' as any /* TODO(inject-migration): Please check if the type is correct */, { optional: true });
+      #service6 = inject(MyService6, { self: true, optional: true });
+      protected service7 = inject(MyService7);
+      public service8 = inject(MyService8);
+
+      myMethod() {
+        this.#myService.doCompleteTask();
+        this.service7.getTask();
+        this.service8.getTick();
+      }
+
+      get value() {
+        return this.#myService.getValue();
+      }
+    
+      set value(theAge: number) {
+        this.#myService.setValue();
+      }
+    }
+"
+`;
+
 exports[`convertDiToInjectGenerator should convert properly 1`] = `
 "
     import { Component, ElementRef, inject, HostAttributeToken } from '@angular/core';
@@ -71,7 +179,7 @@ exports[`convertDiToInjectGenerator should convert properly with @Inject decorat
         });
 
         // use service in a function call
-        someFunction(service2).test(myService);
+        someFunction(service2).test(this.myService);
       }
 
       private service = inject<MyService>('my-service' as any /* TODO(inject-migration): Please check if the type is correct */);

diff --git a/libs/plugin/src/generators/convert-di-to-inject/generator.spec.ts b/libs/plugin/src/generators/convert-di-to-inject/generator.spec.ts
@@ -137,6 +137,79 @@ const filesMap = {
         someFunction(service2).test(myService);
       }
     }
+`,
+	componentWithDepAndInjectAndOptionsAndPrivateFields: `
+    import { Component, Inject, Optional, Self } from '@angular/core';
+    @Component({
+      template: ''
+    })
+    export class MyComponent {
+      constructor(
+        private myService: MyService,
+        private elRef: ElementRef<HtmlImageElement>,
+        private tplRef: TemplateRef<any>,
+        private viewContainerRef: ViewContainerRef,
+        service2: MyService2,
+        @Inject('my-service') private service: MyService,
+        @Inject(MyService4) private service4: MyService4,
+        @Optional() @Inject('my-service2') private service5: MyService5,
+        @Self() @Optional() private service6: MyService6,
+        protected service7: MyService7,
+        public service8: MyService8
+      ) {
+        myService.doSomething();
+
+        this.service2.doSomethingElse();
+
+        service2.doSomething();
+
+        someList.forEach(() => {
+          // nested scope
+          myService.doSomething();
+        });
+
+        // use service in a function call
+        someFunction(service2).test(myService);
+
+        service7.getTask();
+        service8.getTick();
+      }
+
+      myMethod() {
+        this.myService.doCompleteTask();
+        this.service7.getTask();
+        this.service8.getTick();
+      }
+
+      get value() {
+        return this.myService.getValue();
+      }
+    
+      set value(theAge: number) {
+        this.myService.setValue();
+      }
+    }
+`,
+	componentWithDepAndInjectAndReadonly: `
+    import { Component, Inject, Attribute, Optional } from '@angular/core';
+    @Component({
+      template: ''
+    })
+    export class MyComponent {
+      service3: MyService3;
+
+      constructor(
+        @Attribute('type') private type: string,
+        @Inject('my-service') private service: MyService,
+        @Inject(MyService4) private service4: MyService4,
+        @Optional() @Inject('my-service2') private service5: MyService5,
+        @Inject(SOME_TOKEN) private someToken,
+        private service2: MyService2,
+        service3: MyService3
+      ) {
+        this.service3 = service3;
+      }
+    }
 `,
 } as const;
 
@@ -150,6 +223,8 @@ const filesMap = {
 // file with component and constructor with dependencies that don't have type
 
 // remove empty constructor if it's empty and has empty body
+// replace the TS 'private' access modifier with ES '#private' field notation
+// add the 'readonly' keyword
 
 describe('convertDiToInjectGenerator', () => {
 	let tree: Tree;
@@ -236,4 +311,37 @@ describe('convertDiToInjectGenerator', () => {
 		const [updated] = readContent();
 		expect(updated).toMatchSnapshot();
 	});
+
+	it('should convert private modifier fields to native private fields', async () => {
+		const readContent = setup(
+			'componentWithDepAndInjectAndOptionsAndPrivateFields',
+		);
+		await convertDiToInjectGenerator(tree, {
+			...options,
+			useESPrivateFieldNotation: true,
+		});
+		const [updated] = readContent();
+		expect(updated).toMatchSnapshot();
+	});
+
+	it('should add the readonly keyword to converted fields', async () => {
+		const readContent = setup('componentWithDepAndInjectAndReadonly');
+		await convertDiToInjectGenerator(tree, {
+			...options,
+			includeReadonlyByDefault: true,
+		});
+		const [updated] = readContent();
+		expect(updated).toMatchSnapshot();
+	});
+
+	it('should add the readonly keyword to converted native private fields', async () => {
+		const readContent = setup('componentWithDepAndInjectAndReadonly');
+		await convertDiToInjectGenerator(tree, {
+			...options,
+			includeReadonlyByDefault: true,
+			useESPrivateFieldNotation: true,
+		});
+		const [updated] = readContent();
+		expect(updated).toMatchSnapshot();
+	});
 });
diff --git a/libs/plugin/src/generators/convert-di-to-inject/generator.ts b/libs/plugin/src/generators/convert-di-to-inject/generator.ts
@@ -9,7 +9,7 @@ import {
 } from '@nx/devkit';
 import { readFileSync } from 'node:fs';
 import { exit } from 'node:process';
-import { Node, VariableDeclarationKind } from 'ts-morph';
+import { Node, Scope, VariableDeclarationKind } from 'ts-morph';
 import { ContentsStore } from '../shared-utils/contents-store';
 import { ConvertDiToInjectGeneratorSchema } from './schema';
 
@@ -118,6 +118,8 @@ export async function convertDiToInjectGenerator(
 			if (!applicableDecorator) continue;
 
 			const convertedDeps = new Set<string>();
+			const convertedPrivateDeps = new Set<string>();
+
 			let includeHostAttributeToken = false;
 
 			targetClass.getConstructors().forEach((constructor) => {
@@ -221,10 +223,23 @@ export async function convertDiToInjectGenerator(
 							}
 						});
 					} else {
+						const hasPrivateScope = scope === Scope.Private;
+						const propertyName =
+							hasPrivateScope && options.useESPrivateFieldNotation
+								? `#${name}`
+								: name;
+
+						if (hasPrivateScope) {
+							convertedPrivateDeps.add(name);
+						}
+
 						targetClass.insertProperty(index, {
-							name,
+							name: propertyName,
 							initializer,
-							scope,
+							scope:
+								hasPrivateScope && options.useESPrivateFieldNotation
+									? null
+									: scope,
 							isReadonly:
 								isReadonly || options.includeReadonlyByDefault || false,
 							leadingTrivia: '  ',
@@ -237,15 +252,13 @@ export async function convertDiToInjectGenerator(
 							.forEach((ref) => {
 								const node = ref.getNode();
 								const parent = node.getParent();
-								if (!parent || !Node.isMemberExpression(parent)) {
-									return;
-								}
+
 								const text = parent.getText();
 								if (text.includes(`this.${name}`)) {
 									return;
 								}
 								parent.replaceWithText(
-									text.replace(name.toString(), `this.${name}`),
+									text.replace(name.toString(), `this.${propertyName}`),
 								);
 							});
 					}
@@ -319,6 +332,39 @@ export async function convertDiToInjectGenerator(
 					}
 				}
 			});
+
+			if (options.useESPrivateFieldNotation) {
+				Array.from(convertedPrivateDeps).forEach((convertedDepsName) => {
+					const startIndex = targetClass.getProperties().length;
+					const tempAddedProperty = targetClass.insertProperty(startIndex, {
+						name: convertedDepsName,
+						leadingTrivia: '  ',
+					});
+
+					tempAddedProperty
+						.findReferences()
+						.flatMap((ref) => ref.getReferences())
+						.filter((ref) => !ref.isDefinition())
+						.forEach((ref) => {
+							const node = ref.getNode();
+							const parent = node.getParent();
+
+							const text = parent.getText();
+							if (
+								text.includes(`this.#${convertedDepsName}`) ||
+								text.includes(`#${convertedDepsName}`)
+							) {
+								return;
+							}
+
+							parent.replaceWithText(
+								text.replace(convertedDepsName, `#${convertedDepsName}`),
+							);
+						});
+
+					tempAddedProperty.remove();
+				});
+			}
 		}
 
 		tree.write(sourcePath, sourceFile.getFullText());

diff --git a/libs/plugin/src/generators/convert-di-to-inject/schema.d.ts b/libs/plugin/src/generators/convert-di-to-inject/schema.d.ts
@@ -2,4 +2,5 @@ export interface ConvertDiToInjectGeneratorSchema {
 	project?: string;
 	path?: string;
 	includeReadonlyByDefault?: boolean;
+	useESPrivateFieldNotation?: boolean;
 }