Remove collection libraries

[hotforfeature]

For Origami v2, I am considering removing the collection libraries and would like feedback from the community on it.

Current Implementation

The collection libraries are small directives that auto-target many common Polymer elements. This means developers do not have to use the [ironControl] and [emitChanges] directives directly.

With Collections

import { Component, NgModule } from '@angular/core';
import { PolymerModule } from '@codebakery/origami';
import { PaperElementsModule } from '@codebakery/origami/collections';

@NgModule({
  imports: [
    PolymerModule,
    PaperElementsModule
  ],
  declarations: [
    PolymerComponent
  ]
})
export class AppModule { }

@Component({
  selector: 'app-polymer',
  template: `
    <paper-input [(ngModel)]="value"></paper-input>
  `
})
export class PolymerComponent { }

Without Collections

import { Component, NgModule } from '@angular/core';
import { PolymerModule } from '@codebakery/origami';

@NgModule({
  imports: [
    PolymerModule
  ],
  declarations: [
    PolymerComponent
  ]
})
export class AppModule { }

@Component({
  selector: 'app-polymer',
  template: `
    <paper-input [(ngModel)]="value" ironControl></paper-input>
  `
})
export class PolymerComponent { }

Motivation

Size

The collections module adds 8kb of data for this convenience. Not a lot, but every bit counts when trying to target a lower payload size for mobile, and Angular apps are already extremely bloated with Angular core, Zone.js, and Rxjs.

This extra size is even more frustrating when you realize you're only using a small portion of paper/app elements and not all of iron/gold/molecule/platinum elements that are included.

Confusion

When non-Google custom elements are using (such as the ever-popular Vaadin elements), it gets confusing when some Polymer elements need the directives and some do not.

<vaadin-combo-box [(ngModel)]="value"></vaadin-combo-box> <!-- Does not work -->
<vaadin-combo-box [(ngModel)]="value" ironControl></vaadin-combo-box> <!-- Works -->
<paper-input [(ngModel)]="value"></paper-input <!-- Works -->
<paper-input [(ngModel)]="value" ironControl></paper-input> <!-- Works -->

Performance

Not all elements need the [ironControl] or [emitChanges] directives. Auto-adding them by default will not be favorable on performance, especially when using dozens or hundreds of Polymer elements at once.

<paper-item>{{name}}</paper-item> <!-- Does not use EmitChangesDirective -->
<paper-input [(ngModel)]="value"></paper-input> <!-- Does not use EmitChangesDirective -->
<paper-checkbox [(checked)]="isChecked"></paper-checkbox> <!-- Does not use IronControlDirective -->

Proposal

I would suggest we remove the collections library entirely from Origami v2 and change the documentation to encourage developers to use these helper directives sparingly and only when needed.

[alexeygt]

I'm perfectly fine with removing these directives. But I'd appreciate more detailed examples and explanations when you need them and when you don't.

Like can you please explain these three:

<paper-item>{{name}}</paper-item> <!-- Does not use EmitChangesDirective --> <paper-input [(ngModel)]="value"></paper-input> <!-- Does not use EmitChangesDirective --> <paper-checkbox [(checked)]="isChecked"></paper-checkbox> <!-- Does not use IronControlDirective -->

-- like paper-item doesn't use both directives, right? - then short explanation why
-- paper-input doesn't use emitChanges - why? but does it use IronControl?
-- same for third.

[hotforfeature]

Your question highlights a big motivation for their removal: how inherently confusing they are.

<paper-item>{{name}}</paper-item>

This element does not need EmitChangesDirective or IronControlDirective. There is no banana-binding syntax, so there's no need to re-emit change events in an Angular format. There's also no usage of Angular form directives.

<paper-input [(ngModel)]="value"></paper-input>
<paper-input formControlName="value"></paper-input>

Both of these elements require IronControlDirective because they use Angular's forms. The first uses Template forms with the ngModel directive, and the second uses Reactive forms with the formControlName directive.

Neither of them require EmitChangesDirective, since we aren't directly two-way banana binding to a direct property. Instead we're banana binding to the ngModel directive, which IronControlDirective handles.

<paper-checkbox [(checked)]="isChecked"></paper-checkbox>

This requires EmitChangesDirective, because of the banana binding syntax. This element will fire a checked-changed event, and in order for this syntax to work we need EmitChangesDirective to listen for this event and re-fire it as checkedChange, the naming convention Angular's banana binding syntax uses.

It doesn't use any form directives, so it doesn't need IronControlDirective in this situation.

<paper-checkbox [checked]="isChecked" (checked-changed)="isChecked = $event.detail.value"></paper-checkbox>

As a bonus, this element does not need either EmitChangesDirective or IronControlDirective. Its two-way binding is hooked up manually instead of using the banana binding shortcut.

[alexeygt]

@hotforfeature Thank a lot for detailed explanation! I actually wasn't asking these questions, I was just proposing how readme should be organized :) But this is really helpful - just copy-pasted this whole seciont to our confulence page, coz I'm already tired explaining these things to our developers over and over again :)

[jaichandra]

@hotforfeature I am in complete agreement on removing these. Ours is the same confusion. All our components are extensions of either paper-elements or other third-party elements. It always confuses me if I really need these collections or if I can remove them from the imports. I kept them as I never had enough time to figure out if that would break any of our extended elements. We in fact followed the same pattern as in paper-element-module.ts and created a module for our custom elements. I think this would clear a lot of confusion. same applies for #60

[hotforfeature]

Removal it is!