ngxtension / ngxtension-platform

Utilities for Angular
https://ngxtension.netlify.app/
MIT License
576 stars 83 forks source link

feat: added inject migration #326

Closed eneajaho closed 4 months ago

eneajaho commented 5 months ago

It supports everything 🎉

Before: test (1)

After: test (2)

Also inspiration taken from: https://github.com/kreuzerk/inject-pilot/tree/main

cc @kreuzerk @ilirbeqirii

nx-cloud[bot] commented 5 months ago

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 24dffc3754ce3bc2e5383e3ecfc4b5087479154c. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 4 targets - [`nx affected --target=build --parallel=3 --exclude=docs`](https://cloud.nx.app/runs/Uh9fQA7FtP?utm_source=pull-request&utm_medium=comment) - [`nx affected --target=test --parallel=3`](https://cloud.nx.app/runs/eoiSVrKRhp?utm_source=pull-request&utm_medium=comment) - [`nx affected --target=lint --parallel=3`](https://cloud.nx.app/runs/srL0ZBQKls?utm_source=pull-request&utm_medium=comment) - [`nx-cloud record -- nx format:check`](https://cloud.nx.app/runs/gwOwKg1fux?utm_source=pull-request&utm_medium=comment)

Sent with 💌 from NxCloud.

k3nsei commented 4 months ago

Shouldn’t MyService2 be just const in constructor scope and not public property in class instance?

constructor() {
  const myService2 = inject(MyService2);
}
eneajaho commented 4 months ago

Shouldn’t MyService2 be just const in constructor scope and not public property in class instance?

constructor() {
  const myService2 = inject(MyService2);
}

Yeah, you're right about this one.

Now I'm not sure on what I should do with this. Should we unify the deps or keep it inside the constructor only for that one case?

eneajaho commented 4 months ago
export const appConfig: ApplicationConfig = {
  providers: [
    { provide: 'value', useValue: 'Hello, World!' }, // this works fine.
  ]
};

export class TestComponent {
  value = inject('value'); // this fails with error: No overload matches this call. Overload 1 of 7, '(token: ProviderToken<unknown>): unknown', gave the following error.

  // while this works
  value = inject('value' as any);  // we will include as this one and include a TODO comment for the user to let him know.
}
ilirbeqirii commented 4 months ago

Shouldn’t MyService2 be just const in constructor scope and not public property in class instance?

constructor() {
  const myService2 = inject(MyService2);
}

Yeah, you're right about this one.

Now I'm not sure on what I should do with this. Should we unify the deps or keep it inside the constructor only for that one case?

man, I think you should keep it inside the constructor as that's its scope of work.

eneajaho commented 4 months ago

Shouldn’t MyService2 be just const in constructor scope and not public property in class instance?

constructor() {
  const myService2 = inject(MyService2);
}

Yeah, you're right about this one. Now I'm not sure on what I should do with this. Should we unify the deps or keep it inside the constructor only for that one case?

man, I think you should keep it inside the constructor as that's its scope of work.

Most of the time I've seen this be used as:

class Cmp {
  myService2: MyService2;

  constructor(myService2: MyService2;) {
     // just re-assign to a class field
     this.myService2 = myService2;
  }

}

So, I'm keep the same behavior by creating it as a variable inside the constructor.