Skip to content
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

feat: add migration to convert from ngOnDestroy to DestroyRef #533

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

michaelbe812
Copy link
Contributor

@michaelbe812 michaelbe812 commented Dec 20, 2024

DRAFT

Draft for a migration schematic to migrate from ngOnDestroy lifecycle hook to DestroyRef.

Please bare with me - I have not written many schematics for now :)

todos

  • tests
  • docs

Feedback wanted

I would like to know if you generally like the idea.

Also I would like to know which style you would prefer:

// option 1 (currently implemented this way in the schematic)
export class SomeComponent {
  readonly #destroy = inject(DestroyRef).onDestroy(() => {
    // whatever was before in ngOnDestroy
  });
}

// option 2 
export class SomeComponent {
  readonly #destroyRef = inject(DestroyRef)
  
  constructor(){
   this.#destroyRef.onDestroy(() => {
    // whatever was before in ngOnDestroy
  });
}

}

Anything else I should consider?

@michaelbe812
Copy link
Contributor Author

@eneajaho
just asking for general feedback:
Would you like to see this kind of migration in ngxtension?
If yes, what general style should the migration follow (see pr description)

looking forward :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant