guillotinaweb / ngx-schema-form

HTML form generation based on JSON Schema
MIT License
485 stars 173 forks source link

Using providers instead of using a custom widget registry. #78

Open jdrew1303 opened 7 years ago

jdrew1303 commented 7 years ago

Hi,

Currently A2SF uses 'WidgetRegistry' to provide your own custom widgets. Its possible to use the provider in a much more rigorous way to allow for a more descriptive API and to allow for more flexibility in the widget types.

There's an interesting little quirk in Angular that actually allows you to register a component as a component and a provider. This allows you to inject in the component against a token like so:

import { DashboardWidget } from './dashboard-widget.interface';
import { DemoWidgetComponent } from './demo-widget.component';

@Component({
  selector: 'example-dashboard',
  template: `<div #target></div>`
})
export class DashboardComponent implements OnInit {

  // where we're going to store the rendered widgets.
  @ViewChild('target', {read: ViewContainerRef}) private container: ViewContainerRef;

  constructor(
    private resolver: ComponentFactoryResolver,
    // these are our widgets. we need to have them registered as providers and as components 
    // on the module. This allows us to gain a reference easily to them
    @Inject(DashboardWidget) private widgets: DashboardWidget[]
  ) {}

  public ngOnInit(): void {
    // this is the magic! boom! they see me creating widgets, they hating!!! 
    this.widgets.forEach((w) => {
      // this is the bit that ties it all together. usually people pass in a reference 
      // to the class. you can do the same thing by passing in the constructor. its all 
      // the same under the hood.
      const classType: Type<DashboardWidget> = <Type<DashboardWidget>> w.constructor;

      // then we inject it into our dom element.
      const componentFactory = this.resolver.resolveComponentFactory(classType);
      this.container.createComponent(componentFactory);
    });
  }

}

We create our token like so:

import { Component, Type } from '@angular/core';

export abstract class DashboardWidget {
    public readonly name: string;
}

and our component that extends that token:

import { Component } from '@angular/core';
import { DashboardWidget } from './dashboard-widget.interface';

@Component({
  template: '<div>hello is it me youre looking for?</div>'
})
export class DemoWidgetComponent extends DashboardWidget {
    public name: string = 'DummyWidget';
}

And then to wire it all up together:


import { DashboardWidget } from './dashboard-widget.interface';
import { DemoWidgetComponent } from './demo-widget.component';
import { DashboardComponent } from './dashboard.component';

@NgModule({
    declarations: [
      DashboardComponent,
      DemoWidgetComponent
    ],
    entryComponents: [
      DashboardComponent,
      DemoWidgetComponent
    ],
    providers : [
      {
          provide: DashboardWidget,
          useClass: DemoWidgetComponent,
          multi: true
      },
      {
          provide: DashboardWidget,
          useClass: DemoWidgetComponent,
          multi: true
      },
      {
          provide: DashboardWidget,
          useClass: DemoWidgetComponent,
          multi: true
      },
      {
          provide: DashboardWidget,
          useClass: DemoWidgetComponent,
          multi: true
      }
    ]
})
export class DashboardModule {}

In this case I use a multi binder because I don't distinguish between widget types. In the case of the forms you can expose an abstract class that each of the widget must adhere to. This allows a developer to see exactly what is available to them to implement and what default values will be available from the schema.

It also allows you to keep providing your default widgets but they can be selectively overridden by a developer as needed.

I use schema form quite a lot at the moment and wouldn't mind helping to make it a bit more flexible and modular. If you like the approach and would like me to work on a pull request for this then let me know.

ebrehault commented 7 years ago

Hi @jdrew1303, Your approach seems very interesting! The registry approach is ok but it implies to register widgets at runtime (in a ngOnInit() call). Your approach is fully declarative, everything can be done in a NgModule.

A PR would be wonderful! Do not hesitate to submit a work-in-progress PR, so we can discuss about the implementation and the features (@fbessou and @SBats might be interesting in following this discussion).

jdrew1303 commented 7 years ago

Hey @ebrehault,

Cool. :+1: It'll probably be early next week before I can get a preliminary pull request in for handling forms like above.

It also gives you something that I only realized today. I allows you to load different components per module due to the the hierarchical injector (if you're not using a multi-provider).

image

Ill give you a shout back when I get started on the core refactorings. It will probably involve modifying the core first and then recreating the original widgets. If youve any suggestions or ideas feel free to shout.

jdrew1303 commented 7 years ago

Docs [WIP]

Im going to use this comment to document some of the work as Im going through it (it'll be edited and updated). I haven't dug too far into the widget rendering core before (mostly just the jsonschema processing) so some of my assumptions could be wrong (feel free to hop in and let me know 👍 ).

Prelim work:

The Current Class and Inheritance Structure

Legend:

default_widgets

Default Widgets

image

The Current Dependencies Between Classes

default_widgets

The current dependencies aren't too bad. Theres no cycles which is a big bonus. (NOTE: the current TS to UML gen doesnt pick up all deps so take these with a pinch of salt, still give a reasonable overview.)

ebrehault commented 7 years ago

@jdrew1303 looks pretty good to me!

jdrew1303 commented 7 years ago

Ok, so, I've been digging around and playing with different options for the past few hours and there's some options, and some limitations.

If you use just the Widget abstract class as the provider token then a number of things have to happen:

  1. We need to register the providers as multibinders. This means that we lose the ability to have the providers overridden (angular will just add them to the list that is injected for the token). We register them like so:
    providers: [
      {provide: Widget, useClass: ArrayWidget, multi: true},
      {provide: Widget, useClass: ObjectWidget, multi: true},
      {provide: Widget, useClass: CheckboxWidget, multi: true},
      {provide: Widget, useClass: FileWidget, multi: true},
      {provide: Widget, useClass: IntegerWidget, multi: true},
      {provide: Widget, useClass: TextAreaWidget, multi: true},
      {provide: Widget, useClass: RadioWidget, multi: true},
      {provide: Widget, useClass: RangeWidget, multi: true},
      {provide: Widget, useClass: SelectWidget, multi: true},
      {provide: Widget, useClass: StringWidget, multi: true}
    ]
  2. We have to restrict the name of the widget to be what we would register it as in the WidgetRegistry (we will get back an array for the injector token, so we need to look it up by class name (e.g. using this.constructor.name)) or we need to add a public field to the widget that specifies the type. Neither one is great because there is always the possibility that a developer will make a mistake. We do however get the ability to have an arbitrary number of widget types (this is similar to the current solution where you can just add a widget property to a schema to use that instead of the widget associated with the type field).

If we were to create an interface per object type then:

  1. We would lose the ability to have arbitrarily defined widgets (all widgets would have extend a abstract class for their exact type).
  2. We gain the ability to define exactly the available values in the template for the developer to utilise.
  3. We can allow a developer to override in a submodule only specific widgets and provide an alternative implementation.

@ebrehault They're the tradeoffs I can see. I think it needs a bit of hammering out to see which one would be worth pursuing and if there are any other ways to approach this?

ebrehault commented 7 years ago

Solution 2 seems more solid to me and also more elegant.

jdrew1303 commented 7 years ago

@ebrehault Ok cool. Ill put a PR together over the next few days and we can take it from there :bowtie: In the meantime if you have any ideas feel free to shout 👍

jdrew1303 commented 7 years ago

Hey @ebrehault

Just wanted to keep you updated. I should have a pr in for you to take a look over this weekend. Sorry about the delay (work and all that jazz :bowtie:).

gpessa commented 6 years ago

no... this was interesting