aurelia / dialog

A dialog plugin for Aurelia.
MIT License
106 stars 115 forks source link

Add class reference support for .viewModel #353

Closed StrahilKazlachev closed 5 years ago

StrahilKazlachev commented 6 years ago

I'm submitting a feature request

Please tell us about your environment:

Expected/desired behavior:

Adopt the new features from aurelia-templating@1.8.0 in regard to the usage of class references. This will make the class ref for .viewModel much simpler to use and reason about,support code splitting, ...

I already did an initial attempt, but @bigopon pinged me earlier today that this is not how it is done in the router - () => import(/*...*/)/() => class(it is lazy). In the discussion we ended up with the following:

  1. The router needs it lazy since it stores the VM factories for later optional/uncertain usage.
  2. In the dialog and <compose> the VM is consumed immediately.

So what we did not clear is do we want:

  1. An unified API when it comes to using a class ref, factories () => Promise<{new(...parms: any[]): object}> | () => {new(...parms: any[]): object}
  2. An APIs according to the use case
    • where lazy evaluation is needed we use factories - the router
    • where the usage of the class ref is not optional/uncertain we just use the class ref - dialog, <compose>, ...
      • this does not mean that import can not be used, it just it will not be out-of-the-box import().then(m => this.dialogService.open({viewModel: m.MyDialogVM}))

@EisenbergEffect @bigopon @jods4 @fkleuver so what are your thoughts?

bigopon commented 6 years ago

The router needs it lazy since it stores the VM factories for later optional/uncertain usage.

For this, I think we can also say they are both for later optional / uncertain usage. Typically the code responsible for opening dialog will be inside a class method, which makes it

StrahilKazlachev commented 6 years ago

@bigopon what you are saying is that the execution of the code(user code) that is using those APIs is uncertain and for that was my example import().then(m => this.dialogService.open({viewModel: m.MyDialogVM})). This does not change the fact that when you call the dialog API the class will be used.

EisenbergEffect commented 5 years ago

I think it's fine to have the dialog api know nothing about promises or factories. So, if lazy loading is needed, the developer would use import('some/path').then(m => this.dialogService.open({viewModel: m.MyDialogVM})). I think router is a bit different in that it does need to have registrations up front, but evaluation later. Modals are usually invoked directly and at the exact time they are needed.

StrahilKazlachev commented 5 years ago

Keeping the current API. Change already merged.