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

[MatDialog]: Infer return-type R of MatDialogRef<T, R> based on component passed into MatDialog.open() #24538

Open
JWess opened this issue Mar 7, 2022 · 12 comments
Labels
area: material/dialog feature This issue represents a new feature or feature request rather than a bug or bug fix P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@JWess
Copy link

JWess commented Mar 7, 2022

Feature Description

If I call:

MatDialog
  .open(ADialogComponent)
  .afterClosed()
  .subscribe((result) => {
    // result should have a type rather than `any`
  });

It would be nice if result could be typed based on what ADialogComponent is known to return. Perhaps some work would need to be done for a component to be able to specify its return type. Maybe it would need to implement an interface MatDialogComponent<T, R> or something. I don't know the solution...I just know the current way leaves a lot of room for error.

As things are currently, the caller must specify the return type when calling MatDialog.open<T, R>() like this:

MatDialog
  .open<ADialogComponent, any, ReturnTypeHere>(ADialogComponent)
  .afterClosed()
  .subscribe((result: ReturnTypeHere) => {
    // result is typed as ReturnTypeHere because it was specified when calling MatDialog.open()
  });

The gives the caller the opportunity to incorrectly specify the return type...or to not specify one at all.

Use Case

No response

@JWess JWess added feature This issue represents a new feature or feature request rather than a bug or bug fix needs triage This issue needs to be triaged by the team labels Mar 7, 2022
@JWess JWess changed the title [MatDialog]: Infer return-type R of MatDialogRef<T, R> based on component passed into MatDialog.open() [MatDialog]: Infer return-type R of MatDialogRef<T, R> based on component passed into MatDialog.open() Mar 7, 2022
@angular-robot
Copy link
Contributor

angular-robot bot commented Mar 14, 2022

This feature request is now candidate for our backlog! In the next phase, the community has 60 days to upvote. If the request receives more than 20 upvotes, we'll move it to our consideration list.

You can find more details about the feature request process in our documentation.

@andrewseguin andrewseguin added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent and removed needs triage This issue needs to be triaged by the team labels Mar 16, 2022
@andrewseguin
Copy link
Contributor

Based on my investigation with this in another class, I don't think TypeScript supports that sort of inferred generic. However we've got some experts on the team who might be able to help

@vampyr09
Copy link

It should be possible using something like this:

export type Newable = new (...args: any[]) => any;

export type FindMatDialogOutput<TComponent extends Newable> =
  FindMatDialogOutputParameter<ConstructorParameters<TComponent>>[number];

export type FindMatDialogOutputParameter<T extends unknown[]> = {
  [Key in keyof T]: T[Key] extends MatDialogRef<infer _, infer TOutput>
    ? TOutput
    : never;
};

Then the open call type becomes:

open<TComponent extends Newable, TOutput extends FindMatDialogOutput<TComponent>>(
  component: TComponent,
  [...]
): MatDialogRef<TComponent, TOutput>;

@angular-robot
Copy link
Contributor

angular-robot bot commented Apr 23, 2022

Just a heads up that we kicked off a community voting process for your feature request. There are 20 days until the voting process ends.

Find more details about Angular's feature request process in our documentation.

@PooSham
Copy link
Contributor

PooSham commented Jan 12, 2023

MatDialog is really cumbersome. Not only is the return type unsafe, but it's also very annoying that as soon as you want to specify return type, you have to specify T too, which is usually exactly the same thing as the open function's first argument. So it's just repetitive information. It would also be nice if the default R was unknown instead of any, at least for us using no-unsafe-return and other anti-any measures in eslint.

Too bad it didn't get enough votes. I hope it's reconsidered. MatDialog/CdkDialog needs a typesafe option in some way.

@aceArt-GmbH
Copy link

ideally MAT_DIALOG_DATA could be inferred too.

Neither having input or output typesafe is unfortunate

@carlos-ds
Copy link

carlos-ds commented Sep 24, 2023

I also don't know how this should be resolved but would definitely be an enormous improvement regarding type safety. Maybe someday this could be reconsidered?

While I haven't tested it out yet, an interesting (yet, somewhat cumbersome) way to mitigate the missing link between the type of the result value and the parent component is detailed in this blog post by @robinpellegrims. It involves creating an abstract class that your child component can extend from and a custom dialog service as follows:

@Directive()
export abstract class StronglyTypedDialog<DialogData, DialogResult> {
  constructor(
    @Inject(MAT_DIALOG_DATA) public data: DialogData,
    public dialogRef: MatDialogRef<
      StronglyTypedDialog<DialogData, DialogResult>,
      DialogResult
    >
  ) {}
}

@Injectable({ providedIn: 'root' })
export class DialogService {
  constructor(public dialog: MatDialog) {}

  open = <DialogData, DialogResult>(
    component: ComponentType<StronglyTypedDialog<DialogData, DialogResult>>,
    config?: MatDialogConfig<DialogData>
  ): MatDialogRef<
    StronglyTypedDialog<DialogData, DialogResult>,
    DialogResult
  > => this.dialog.open(component, config);
}

@Frankitch
Copy link

@carlos-ds That's terrific, that's the workaround I was looking for for monthes! It's working very well, thanks so much!
And it's even easier if you use the inject function instead of constructor params.

@carlos-ds
Copy link

carlos-ds commented Sep 25, 2023

@Frankitch no worries but all credits go to the original author, I'm just the messenger.

Edit: in the meantime, I also had the chance to play around with the solution I referenced above and it's a really good solution indeed if you feel type safety is of utmost importance. The only necessary change was replacing ComponentType with Type.

@aceArt-GmbH
Copy link

aceArt-GmbH commented Oct 4, 2023

@carlos-ds thanks for sharing ❤️

And it's even easier if you use the inject function instead of constructor params.

@Frankitch how would that look in the code?

(cc #12898 (comment))

@Frankitch
Copy link

@aceArt-GmbH, my abstract class just looks like this:

import {inject} from '@angular/core'
import {MAT_DIALOG_DATA, MatDialogRef} from '@angular/material/dialog'

export abstract class TypedDialog<DialogData, DialogResult> {
  protected data: DialogData = inject(MAT_DIALOG_DATA)
  protected dialogRef: MatDialogRef<TypedDialog<DialogData, DialogResult>, DialogResult> =
    inject(MatDialogRef)
}

And now you do not have to pass arguments when you call super in your concrete dialog class constructor.

@aceArt-GmbH
Copy link

aceArt-GmbH commented Oct 5, 2023

Resolved: `No provider for InjectionToken` error

hm, with either solution, I seem to get

ERROR NullInjectorError: R3InjectorError(AppModule)[InjectionToken mat-datepicker-scroll-strategy -> InjectionToken mat-datepicker-scroll-strategy -> InjectionToken mat-datepicker-scroll-strategy]: 
  NullInjectorError: No provider for InjectionToken mat-datepicker-scroll-strategy!

and similar (Angular 15 using legacy material)

Works with providedIn: 'any' but that's deprecated.

@Frankitch @carlos-ds do you not have this problem with lazyLoaded modules?

works with @Injectable({ providedIn: MatDialogModule }) flawless

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: material/dialog feature This issue represents a new feature or feature request rather than a bug or bug fix P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

No branches or pull requests

8 participants