Switch to stateless directives (#220)

# Pull Request

## 🤨 Rationale

This PR addresses [TASK 1741886](https://ni.visualstudio.com/DevCentral/_workitems/edit/1741886) and [TASK 1746789](https://ni.visualstudio.com/DevCentral/_workitems/edit/1746789).

The major changes include:
- Removing [barrel](https://basarat.gitbook.io/typescript/main-1/barrel) files
  - After removing barrel files from nimble-angular the library is able to be used in other ViewEngine libraries without resulting in a `Maximum call stack size exceeded` from the angular compiler. 
  - Some possible reasons removing the barrel files helped is eliminating [import cycles](angular/angular#14649) or preventing the same [symbol from being exposed from multiple paths](psykolm22/angular-google-place#29 (comment)). A couple attempts at bisecting did not reveal the exact culprit so barrel files were removed wholesale in this PR.
- Change to stateless directives
  - After switching to ViewEngine builds the `@HostBinding` directives binding to properties that were specific to the nimble components would fail (ie expended state of tree). It seems like that would be [expected generally](angular/angular#13776). Maybe a change in ivy allows for that kind of binding but it is not ViewEngine compatible. 
  - Regardless, this change makes directives stateless instead. This is preferred generally because our custom elements themselves should be managing their state and be the source of truth, not the Angular directive which is just acting as a proxy to the underlying web component.

## 👩‍💻 Implementation

- Barrel files were removed
  - A similar change (but unrelated to the top-level re-factor) was removing the idea of shared control value accessors and instead moving the existing control value accessors to be next to their component. There isn't a strong reason either way but I think it helps keep the components isolated and helps with repo organization even though it may result in some code duplication (very minimal with the current control value accessor patterns).
- Stateless directives
  - Removed existing `@HostBinding` calls and replaced with accessors setting values through the Renderer2 interface as a step to avoid breaking [Angular Universal](https://www.willtaylor.blog/angular-universal-gotchas/) support and 

## 🧪 Testing

- Barrel files were removed
  - Verified against a local build of systemlink-lib-angular with the [changes for nimble-tree-view](https://ni.visualstudio.com/DevCentral/_git/Skyline/pullrequest/228468) applied. Verified that the header was able to perform a production build but not beyond that (ie verifying systemlink-lib-angular using nimble-angular could be used in SystemLinkShared).
- Stateless directives
  - Since we are now managing the definition and state on the directives ourselves (proxying to the web component) it seems necessary to test those code paths instead of relying on the behavior of `@HostBinding`. I modified the nimble-angular boolean directive test to cover the cases I think we should cover and updated the testing docs.
  - One behavior I found writing the tests is that when binding to templates directly (ie as strings in html `<my-elem something="7">`) angular will [assign to the directive property](angular/angular#6919) a string value even if the TypeScript type is something else, like number. See the comments in `template-value-helpers.ts`. Making sure to test this behavior is captured in the test docs updates.

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or determined no changes are needed.

angular-workspace/.eslintrc.js

@@ -17,6 +17,17 @@ module.exports = {
                     group: ['@microsoft/fast-*'],
                     message: 'Do not directly use underlying libraries of nimble. Instead rely on or add to exports of nimble packages.'
                 }]
+            }],
+
+            // Recommended rules with strict null checks enabled: https://github.com/ni/javascript-styleguide/#strict-null-checks
+            '@typescript-eslint/no-non-null-assertion': 'off',
+            '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'error',
+            '@typescript-eslint/no-unnecessary-condition': 'error',
+            '@typescript-eslint/strict-boolean-expressions': ['error', {
+                allowNumber: false,
+                allowNullableBoolean: true,
+                allowNullableString: true,
+                allowNullableNumber: false
             }]
         }
     },

angular-workspace/package.json

@@ -14,7 +14,7 @@
     "watch": "ng build --watch --configuration development",
     "test": "ng test --watch=false",
     "lint": "ng lint",
-    "lint-fix": "ng lint --fix"
+    "format": "ng lint --fix"
   },
   "private": true,
   "dependencies": {

angular-workspace/projects/example-client-app/src/environments/.eslintrc.js

@@ -0,0 +1,16 @@
+module.exports = {
+    overrides: [
+      {
+        files: [
+          'environment.*.ts'
+        ],
+        rules: {
+            // The environment.*.ts files are used in fileReplacements and are not part of the normal tsconfig.
+            // Because they do not participate in tsconfig they don't get the strictNullChecks compiler setting
+            // so we disable rules requiring stictNullChecks for these files.
+            '@typescript-eslint/no-unnecessary-condition': 'off',
+            '@typescript-eslint/strict-boolean-expressions': 'off'
+        }
+      }
+    ]
+  };

angular-workspace/projects/ni/nimble-angular/CONTRIBUTING.md

@@ -2,15 +2,15 @@
 
 ## Creating an Angular directive for a Nimble element
 
-In order to provide first-class Angular integration for our Nimble elements, we create Angular directives. Each directive targets one or more Nimble elements by their tag names. The directives allow us to specify properties and events for binding, create good Angular form integration behavior, and extend the behavior of the Nimble elements via lifecycle hooks. 
+In order to provide first-class Angular integration for our Nimble elements, we create Angular directives. Each directive targets one or more Nimble elements by their tag names. The directives allow us to specify accessors and events for binding, create good Angular form integration behavior, and extend the behavior of the Nimble elements via lifecycle hooks. 
 
-Each Nimble element should have at least one directive which references its tag name (in the `selector` section of the `@Directive` decorator). This registers the element with Angular, so that client applications don't need to use the `CUSTOM_ELEMENTS_SCHEMA`. The main directive for an element should specify inputs (attributes and properties) and outputs (events). Inputs can use the `HostBinding` decorator to apply the property value to the Nimble element via a property or attribute. Outputs can use the `HostListener` decorator to forward the event through the directive to the consumer of the element.
+Each Nimble element should have at least one directive which references its tag name (in the `selector` section of the `@Directive` decorator). This registers the element with Angular, so that client applications don't need to use the `CUSTOM_ELEMENTS_SCHEMA`. The main directive for an element should specify `@Input`s (accessors) and `@Output`s (events). `@Input`s are bound to the setter of a property accessor and use `Renderer2` to apply property updates to the underlying `nativeElement`. `@Outputs` can use `@HostListener` to forward DOM events from the custom element as Angular events for two-way binding.
 
-We can potentially create shared directives which target multiple Nimble elements in order to share logic between them.
+Directives for the underlying Nimble web components should generally be stateless. While you would normally expect directives to own their own property state which are reflected outside using `@HostBinding`s, we instead are treating directives as proxies for the underlying web component which will actually own the directive state. Directive properties are accessors that write updates directly to elements via `Renderer2` and read values from elements via `nativeElement` references.
 
 ### Angular forms integration
 
-We can make Nimble components integrate well with Angular forms and `ngModel` binding by implementing the `ControlValueAccessor` interface on directives. Unless custom behavior is needed, we extend Angular's built-in `ControlValueAccessor`s to target our controls.
+We can make Nimble components integrate well with Angular forms and `ngModel` binding by implementing the `ControlValueAccessor` interface on directives. Unless custom behavior is needed, we extend Angular's built-in `ControlValueAccessor` implementations to target our controls.
 
 ## Testing
 
@@ -21,4 +21,8 @@ The Angular directives need code coverage via unit tests.
   * For example, `expect(customElements.get('nimble-foo')).not.toBeUndefined();`
   * Tests should not import the Nimble element directly. Use `import type` if the element's type is needed.
 * Any other custom behavior implemented in the directives (lifecycle hooks, new properties) should be tested.
-  *  However, it shouldn't be necessary to test properties which only use a simple `HostBinding` decorator.
+* Tests should be written to verify property values with the different template binding modes. For example, the directive `disabled` property for an element should verify the property value is correctly set under the following conditions:
+  * Templates relying on default values, for example: `<my-element></my-element`.
+  * Templates setting values via template strings, for example: `<my-element disabled></my-element`.
+  * Templates setting values via property bindings, for example: `<my-element [disabled]="isDisabled"></my-element`.
+  * Templates setting values via attribute bindings, for example: `<my-element [attr.disabled]="isDisabled"></my-element`.

angular-workspace/projects/ni/nimble-angular/src/directives/button/nimble-button.directive.ts

@@ -1,5 +1,7 @@
-import { Directive, HostBinding, Input } from '@angular/core';
+import { Directive, ElementRef, Input, Renderer2 } from '@angular/core';
+import { Button } from '@ni/nimble-components/dist/esm/button';
 import { ButtonAppearance } from '@ni/nimble-components/dist/esm/button/types';
+import { toBooleanProperty } from '../utilities/template-value-helpers';
 
 /**
  * Directive to provide Angular integration for the button.
@@ -8,6 +10,21 @@ import { ButtonAppearance } from '@ni/nimble-components/dist/esm/button/types';
     selector: 'nimble-button'
 })
 export class NimbleButtonDirective {
-    @HostBinding('disabled') @Input() public disabled: boolean;
-    @HostBinding('appearance') @Input() public appearance: ButtonAppearance;
+    public get appearance(): ButtonAppearance {
+        return this.elementRef.nativeElement.appearance;
+    }
+
+    @Input() public set appearance(value: ButtonAppearance) {
+        this.renderer.setProperty(this.elementRef.nativeElement, 'appearance', value);
+    }
+
+    public get disabled(): boolean {
+        return this.elementRef.nativeElement.disabled;
+    }
+
+    @Input() public set disabled(value: boolean) {
+        this.renderer.setProperty(this.elementRef.nativeElement, 'disabled', toBooleanProperty(value));
+    }
+
+    public constructor(private readonly renderer: Renderer2, private readonly elementRef: ElementRef<Button>) {}
 }

angular-workspace/projects/ni/nimble-angular/src/directives/button/tests/nimble-button.directive.spec.ts

@@ -1,14 +1,205 @@
-import { TestBed } from '@angular/core/testing';
+import { Component, ElementRef, ViewChild } from '@angular/core';
+import { ComponentFixture, TestBed } from '@angular/core/testing';
+import { Button } from '@ni/nimble-components/dist/esm/button';
+import { ButtonAppearance } from '@ni/nimble-components/dist/esm/button/types';
+import { BooleanAttribute } from '../../utilities/template-value-helpers';
+import { NimbleButtonDirective } from '../nimble-button.directive';
 import { NimbleButtonModule } from '../nimble-button.module';
 
 describe('Nimble button', () => {
-    beforeEach(() => {
-        TestBed.configureTestingModule({
-            imports: [NimbleButtonModule]
+    describe('module', () => {
+        beforeEach(() => {
+            TestBed.configureTestingModule({
+                imports: [NimbleButtonModule]
+            });
+        });
+
+        it('defines custom element', () => {
+            expect(customElements.get('nimble-button')).not.toBeUndefined();
+        });
+    });
+
+    describe('with no values in template', () => {
+        @Component({
+            template: `
+                <nimble-button #button></nimble-button>
+            `
+        })
+        class TestHostComponent {
+            @ViewChild('button', { read: NimbleButtonDirective }) public directive: NimbleButtonDirective;
+            @ViewChild('button', { read: ElementRef }) public elementRef: ElementRef<Button>;
+        }
+
+        let fixture: ComponentFixture<TestHostComponent>;
+        let directive: NimbleButtonDirective;
+        let nativeElement: Button;
+
+        beforeEach(async () => {
+            await TestBed.configureTestingModule({
+                declarations: [TestHostComponent],
+                imports: [NimbleButtonModule]
+            }).compileComponents();
+            fixture = TestBed.createComponent(TestHostComponent);
+            fixture.detectChanges();
+            directive = fixture.componentInstance.directive;
+            nativeElement = fixture.componentInstance.elementRef.nativeElement;
+        });
+
+        it('has expected defaults for disabled', () => {
+            expect(directive.disabled).toBeFalse();
+            expect(nativeElement.disabled).toBeFalse();
+        });
+
+        it('has expected defaults for appearance', () => {
+            expect(directive.appearance).toBe(ButtonAppearance.Outline);
+            expect(nativeElement.appearance).toBe(ButtonAppearance.Outline);
+        });
+    });
+
+    describe('with template string values', () => {
+        @Component({
+            template: `
+                <nimble-button #button
+                    disabled
+                    appearance="${ButtonAppearance.Ghost}">
+                </nimble-button>`
+        })
+        class TestHostComponent {
+            @ViewChild('button', { read: NimbleButtonDirective }) public directive: NimbleButtonDirective;
+            @ViewChild('button', { read: ElementRef }) public elementRef: ElementRef<Button>;
+        }
+
+        let fixture: ComponentFixture<TestHostComponent>;
+        let directive: NimbleButtonDirective;
+        let nativeElement: Button;
+
+        beforeEach(async () => {
+            await TestBed.configureTestingModule({
+                declarations: [TestHostComponent],
+                imports: [NimbleButtonModule]
+            }).compileComponents();
+            fixture = TestBed.createComponent(TestHostComponent);
+            fixture.detectChanges();
+            directive = fixture.componentInstance.directive;
+            nativeElement = fixture.componentInstance.elementRef.nativeElement;
+        });
+
+        it('will use template string values for disabled', () => {
+            expect(directive.disabled).toBeTrue();
+            expect(nativeElement.disabled).toBeTrue();
+        });
+
+        it('will use template string values for appearance', () => {
+            expect(directive.appearance).toBe(ButtonAppearance.Ghost);
+            expect(nativeElement.appearance).toBe(ButtonAppearance.Ghost);
+        });
+    });
+
+    describe('with property bound values', () => {
+        @Component({
+            template: `
+                <nimble-button #button
+                    [disabled]="disabled"
+                    [appearance]="appearance">
+                </nimble-button>
+            `
+        })
+        class TestHostComponent {
+            @ViewChild('button', { read: NimbleButtonDirective }) public directive: NimbleButtonDirective;
+            @ViewChild('button', { read: ElementRef }) public elementRef: ElementRef<Button>;
+            public disabled = false;
+            public appearance = ButtonAppearance.Outline;
+        }
+
+        let fixture: ComponentFixture<TestHostComponent>;
+        let directive: NimbleButtonDirective;
+        let nativeElement: Button;
+
+        beforeEach(async () => {
+            await TestBed.configureTestingModule({
+                declarations: [TestHostComponent],
+                imports: [NimbleButtonModule]
+            }).compileComponents();
+            fixture = TestBed.createComponent(TestHostComponent);
+            fixture.detectChanges();
+            directive = fixture.componentInstance.directive;
+            nativeElement = fixture.componentInstance.elementRef.nativeElement;
+        });
+
+        it('can be configured with property binding for disabled', async () => {
+            expect(directive.disabled).toBeFalse();
+            expect(nativeElement.disabled).toBeFalse();
+
+            fixture.componentInstance.disabled = true;
+            fixture.detectChanges();
+
+            expect(directive.disabled).toBeTrue();
+            expect(nativeElement.disabled).toBeTrue();
+        });
+
+        it('can be configured with property binding for appearance', async () => {
+            expect(directive.appearance).toBe(ButtonAppearance.Outline);
+            expect(nativeElement.appearance).toBe(ButtonAppearance.Outline);
+
+            fixture.componentInstance.appearance = ButtonAppearance.Ghost;
+            fixture.detectChanges();
+
+            expect(directive.appearance).toBe(ButtonAppearance.Ghost);
+            expect(nativeElement.appearance).toBe(ButtonAppearance.Ghost);
         });
     });
 
-    it('custom element is defined', () => {
-        expect(customElements.get('nimble-button')).not.toBeUndefined();
+    describe('with attribute bound values', () => {
+        @Component({
+            template: `
+                <nimble-button #button
+                    [attr.disabled]="disabled"
+                    [attr.appearance]="appearance">
+                </nimble-button>
+            `
+        })
+        class TestHostComponent {
+            @ViewChild('button', { read: NimbleButtonDirective }) public directive: NimbleButtonDirective;
+            @ViewChild('button', { read: ElementRef }) public elementRef: ElementRef<Button>;
+            public disabled: BooleanAttribute = null;
+            public appearance: ButtonAppearance = ButtonAppearance.Outline;
+        }
+
+        let fixture: ComponentFixture<TestHostComponent>;
+        let directive: NimbleButtonDirective;
+        let nativeElement: Button;
+
+        beforeEach(async () => {
+            await TestBed.configureTestingModule({
+                declarations: [TestHostComponent],
+                imports: [NimbleButtonModule]
+            }).compileComponents();
+            fixture = TestBed.createComponent(TestHostComponent);
+            fixture.detectChanges();
+            directive = fixture.componentInstance.directive;
+            nativeElement = fixture.componentInstance.elementRef.nativeElement;
+        });
+
+        it('can be configured with attribute binding for disabled', () => {
+            expect(directive.disabled).toBeFalse();
+            expect(nativeElement.disabled).toBeFalse();
+
+            fixture.componentInstance.disabled = '';
+            fixture.detectChanges();
+
+            expect(directive.disabled).toBeTrue();
+            expect(nativeElement.disabled).toBeTrue();
+        });
+
+        it('can be configured with attribute binding for appearance', () => {
+            expect(directive.appearance).toBe(ButtonAppearance.Outline);
+            expect(nativeElement.appearance).toBe(ButtonAppearance.Outline);
+
+            fixture.componentInstance.appearance = ButtonAppearance.Ghost;
+            fixture.detectChanges();
+
+            expect(directive.appearance).toBe(ButtonAppearance.Ghost);
+            expect(nativeElement.appearance).toBe(ButtonAppearance.Ghost);
+        });
     });
 });