feat(runtime): convert with attribute to use render location (#64)

* feat(runtime): convert with attribute to use render location

* test(with): add basic tests and refactor test assistance

* testI(runtime): reduce duplication in template controller tests

* test(runtime): make shared templatr controller tests more composable

* feat(runtime): improvements to attribute and element bindable control and common interfaces

* fix(runtime): correct observer current value update and callback ordering

* test(kernel): add Toggle tests

* fix(runtime): ensure all bindable callback slots are initialized

* fix(runtime-behavior): remove use of Toggle in favor of simple boolean

.vscode/settings.json

@@ -24,6 +24,7 @@
     "**/scripts": true
   },
   "cSpell.words": [
+    "Constructable",
     "Renderable",
     "attachables",
     "bindables"

packages/kernel/test/unit/reporter.spec.ts

@@ -0,0 +1,10 @@
+import { expect } from 'chai';
+import { Reporter } from '@aurelia/kernel';
+
+describe('The default Reporter', () => {
+  it('creates an Error with a minimal message upon request', () => {
+    const code = 42;
+    const error = Reporter.error(code);
+    expect(error.message).to.equal(`Code ${code}`);
+  });
+});

packages/runtime/src/binding/binding-context.ts

@@ -12,12 +12,24 @@ export const BindingContext = {
   createScopeFromOverride(overrideContext: IOverrideContext): IScope {
     return { bindingContext: overrideContext.bindingContext, overrideContext };
   },
+
+  createScopeFromParent(parentScope: IScope, bindingContext: any): IScope {
+    return {
+      bindingContext: bindingContext,
+      overrideContext: BindingContext.createOverride(
+        bindingContext,
+        parentScope.overrideContext
+      )
+    };
+  },
+
   createOverride(bindingContext?: any, parentOverrideContext?: IOverrideContext): IOverrideContext {
     return {
       bindingContext: bindingContext,
       parentOverrideContext: parentOverrideContext || null
     };
   },
+
   get(scope: IScope, name: string, ancestor: number): any {
     let overrideContext = scope.overrideContext;
 

packages/runtime/src/binding/binding.ts

@@ -31,12 +31,13 @@ const toViewOrOneTime = toView | oneTime;
 
 // tslint:disable:no-any
 export class Binding implements IBinding, IPropertySubscriber {
+  public $isBound: boolean = false;
+
   public targetObserver: AccessorOrObserver;
   /*@internal*/public __connectQueueId: number;
   private observerSlots: number;
   private version: number;
   private $scope: IScope;
-  private $isBound: boolean = false;
 
   constructor(
     public sourceExpression: IExpression,

packages/runtime/src/binding/call.ts

@@ -8,9 +8,9 @@ import { IAccessor } from './observation';
 import { IObserverLocator } from './observer-locator';
 
 export class Call implements IBinding {
+  public $isBound: boolean = false;
   public targetObserver: IAccessor;
   private $scope: IScope;
-  private $isBound = false;
 
   constructor(
     private sourceExpression: IExpression,

packages/runtime/src/binding/listener.ts

@@ -7,8 +7,8 @@ import { BindingFlags } from './binding-flags';
 import { DelegationStrategy, IEventManager } from './event-manager';
 
 export class Listener implements IBinding {
+  public $isBound: boolean = false;
   private source: IScope;
-  private $isBound = false;
   private handler: IDisposable;
 
   constructor(

packages/runtime/src/binding/observation.ts

@@ -4,6 +4,7 @@ import { BindingFlags } from './binding-flags';
 import { IChangeSet } from './change-set';
 
 export interface IBindScope {
+  readonly $isBound: boolean;
   $bind(flags: BindingFlags, scope: IScope): void;
   $unbind(flags: BindingFlags): void;
 }

packages/runtime/src/binding/property-observation.ts

@@ -151,19 +151,21 @@ export class Observer<T>  implements IPropertyObserver<IIndexable, string> {
   }
 
   public setValue(newValue: T, flags?: BindingFlags): void {
-    const currentValue = this.currentValue;
-    if (currentValue !== newValue) {
-      this.previousValue = currentValue;
-      this.notify(newValue, currentValue);
+    const previousValue = this.currentValue;
+
+    if (previousValue !== newValue) {
+      this.previousValue = previousValue;
+      this.currentValue = newValue;
 
       if (this.selfCallback !== undefined) {
-        const coercedValue = this.selfCallback(newValue, currentValue);
+        const coercedValue = this.selfCallback(newValue, previousValue);
 
         if (coercedValue !== undefined) {
-          newValue = <T>coercedValue;
+          this.currentValue = newValue = <T>coercedValue;
         }
       }
-      this.currentValue = newValue;
+
+      this.notify(newValue, previousValue);
     }
   }
 }

packages/runtime/src/binding/ref.ts

@@ -5,8 +5,8 @@ import { IScope } from './binding-context';
 import { BindingFlags } from './binding-flags';
 
 export class Ref implements IBinding {
+  public $isBound: boolean = false;
   private $scope: IScope;
-  private $isBound = false;
 
   constructor(
     private sourceExpression: IExpression,

packages/runtime/src/templating/custom-attribute.ts

@@ -28,15 +28,14 @@ export interface ICustomAttributeSource {
 export type ICustomAttributeType = IResourceType<ICustomAttributeSource, ICustomAttribute>;
 
 export interface ICustomAttribute extends IBindScope, IAttach {
-  readonly $isBound: boolean;
-  readonly $isAttached: boolean;
   readonly $scope: IScope;
   $hydrate(renderingEngine: IRenderingEngine): void;
 }
 
 /*@internal*/
 export interface IInternalCustomAttributeImplementation extends Writable<ICustomAttribute> {
-  $changeCallbacks: (() => void)[];
+  $bindableCallbacks: (() => void)[];
+  $bindableCallbacksEnabled: boolean;
   $behavior: IRuntimeBehavior;
   $child: IAttach;
 }
@@ -107,12 +106,12 @@ export const CustomAttributeResource: IResourceKind<ICustomAttributeSource, ICus
     };
 
     proto.$hydrate = function(this: IInternalCustomAttributeImplementation, renderingEngine: IRenderingEngine): void {
-      this.$changeCallbacks = [];
       this.$isAttached = false;
       this.$isBound = false;
       this.$scope = null;
       this.$child = this.$child || null;
-      this.$behavior = renderingEngine.applyRuntimeBehavior(Type, this, description.bindables);
+
+      renderingEngine.applyRuntimeBehavior(Type, this);
 
       if (this.$behavior.hasCreated) {
         (this as any).created();
@@ -128,18 +127,20 @@ export const CustomAttributeResource: IResourceKind<ICustomAttributeSource, ICus
         this.$unbind(flags);
       }
 
-      this.$scope = scope
-      this.$isBound = true;
+      this.$scope = scope;
+      this.$bindableCallbacksEnabled = true;
 
-      const changeCallbacks = this.$changeCallbacks;
+      const bindableCallbacks = this.$bindableCallbacks;
 
-      for (let i = 0, ii = changeCallbacks.length; i < ii; ++i) {
-        changeCallbacks[i]();
+      for (let i = 0, ii = bindableCallbacks.length; i < ii; ++i) {
+        bindableCallbacks[i]();
       }
 
       if (this.$behavior.hasBound) {
         (this as any).bound(scope);
       }
+
+      this.$isBound = true;
     };
 
     proto.$attach = function(this: IInternalCustomAttributeImplementation, encapsulationSource: INode, lifecycle: AttachLifecycle): void {
@@ -186,6 +187,7 @@ export const CustomAttributeResource: IResourceKind<ICustomAttributeSource, ICus
           (this as any).unbound();
         }
 
+        this.$bindableCallbacksEnabled = false;
         this.$isBound = false;
       }
     };

packages/runtime/src/templating/custom-element.ts

@@ -23,13 +23,13 @@ export type IElementHydrationOptions = Immutable<Pick<IHydrateElementInstruction
 
 export interface ICustomElement extends IBindSelf, IAttach, Readonly<IRenderable> {
   readonly $projector: IViewProjector;
-  readonly $isAttached: boolean;
   $hydrate(renderingEngine: IRenderingEngine, host: INode, options?: IElementHydrationOptions): void;
 }
 
 /*@internal*/
 export interface IInternalCustomElementImplementation extends Writable<ICustomElement> {
-  $changeCallbacks: (() => void)[];
+  $bindableCallbacks: (() => void)[];
+  $bindableCallbacksEnabled: boolean;
   $behavior: IRuntimeBehavior;
   $child: IAttach;
 }
@@ -110,9 +110,9 @@ export const CustomElementResource: ICustomElementResource = {
     proto.$hydrate = function(this: IInternalCustomElementImplementation, renderingEngine: IRenderingEngine, host: INode, options: IElementHydrationOptions = PLATFORM.emptyObject): void {
       const template = renderingEngine.getElementTemplate(description, Type);
 
+      this.$context = template.renderContext;
       this.$bindables = [];
       this.$attachables = [];
-      this.$changeCallbacks = [];
       this.$child = null;
       this.$isAttached = false;
       this.$isBound = false;
@@ -121,8 +121,8 @@ export const CustomElementResource: ICustomElementResource = {
         overrideContext: BindingContext.createOverride()
       };
 
-      this.$context = template.renderContext;
-      this.$behavior = renderingEngine.applyRuntimeBehavior(Type, this, description.bindables);
+      renderingEngine.applyRuntimeBehavior(Type, this);
+
       this.$projector = determineProjector(this, host, description, options);
       this.$nodes = this.$behavior.hasRender
         ? (this as any).render(host, options.parts, template)
@@ -141,21 +141,23 @@ export const CustomElementResource: ICustomElementResource = {
       const scope = this.$scope;
       const bindables = this.$bindables;
 
+      this.$bindableCallbacksEnabled = true;
+
       for (let i = 0, ii = bindables.length; i < ii; ++i) {
         bindables[i].$bind(flags, scope);
       }
 
-      this.$isBound = true;
-
-      const changeCallbacks = this.$changeCallbacks;
+      const bindableCallbacks = this.$bindableCallbacks;
 
-      for (let i = 0, ii = changeCallbacks.length; i < ii; ++i) {
-        changeCallbacks[i]();
+      for (let i = 0, ii = bindableCallbacks.length; i < ii; ++i) {
+        bindableCallbacks[i]();
       }
 
       if (this.$behavior.hasBound) {
         (this as any).bound();
       }
+
+      this.$isBound = true;
     };
 
     proto.$attach = function(this: IInternalCustomElementImplementation, encapsulationSource: INode, lifecycle?: AttachLifecycle): void {
@@ -233,6 +235,7 @@ export const CustomElementResource: ICustomElementResource = {
           (this as any).unbound();
         }
 
+        this.$bindableCallbacksEnabled = false;
         this.$isBound = false;
       }
     };

packages/runtime/src/templating/lifecycle.ts

@@ -95,11 +95,13 @@ export class DetachLifecycle {
 }
 
 export interface IAttach {
+  readonly $isAttached: boolean;
   $attach(encapsulationSource: INode, lifecycle?: AttachLifecycle): void;
   $detach(lifecycle?: DetachLifecycle): void;
 }
 
 export interface IBindSelf {
+  readonly $isBound: boolean;
   $bind(flags: BindingFlags): void;
   $unbind(flags: BindingFlags): void;
 }

packages/runtime/src/templating/renderable.ts

@@ -8,10 +8,10 @@ import { IRenderContext } from './render-context';
 export const IRenderable = DI.createInterface<IRenderable>().noDefault();
 
 export interface IRenderable {
-  $context: IRenderContext;
-  $nodes: INodeSequence;
-  $scope: IScope;
-  $isBound: boolean;
-  $bindables: IBindScope[];
-  $attachables: IAttach[];
+  readonly $context: IRenderContext;
+  readonly $nodes: INodeSequence;
+  readonly $scope: IScope;
+  readonly $isBound: boolean;
+  readonly $bindables: IBindScope[];
+  readonly $attachables: IAttach[];
 }

packages/runtime/src/templating/rendering-engine.ts

@@ -26,8 +26,8 @@ export interface IRenderingEngine {
   getElementTemplate(definition: TemplateDefinition, componentType: ICustomElementType): ITemplate;
   getViewFactory(context: IRenderContext, source: Immutable<ITemplateSource>): IViewFactory;
 
-  applyRuntimeBehavior(type: ICustomAttributeType, instance: ICustomAttribute, bindables: BindableDefinitions): IRuntimeBehavior;
-  applyRuntimeBehavior(type: ICustomElementType, instance: ICustomElement, bindables: BindableDefinitions): IRuntimeBehavior
+  applyRuntimeBehavior(type: ICustomAttributeType, instance: ICustomAttribute): void;
+  applyRuntimeBehavior(type: ICustomElementType, instance: ICustomElement): void
 
   createViewFromComponent(context: IRenderContext, componentOrType: any, instruction: Immutable<IHydrateElementInstruction>): ViewWithCentralComponent;
   createRenderer(context: IRenderContext): IRenderer;
@@ -96,29 +96,23 @@ export class RenderingEngine implements IRenderingEngine {
     let found = this.factoryLookup.get(definition);
 
     if (!found) {
-      let validSource = createDefinition(definition);
+      const validSource = createDefinition(definition);
       found = this.factoryFromSource(context, validSource);
       this.factoryLookup.set(definition, found);
     }
 
     return found;
   }
 
-  public applyRuntimeBehavior(type: ICustomAttributeType | ICustomElementType, instance: ICustomAttribute | ICustomElement, bindables: BindableDefinitions): IRuntimeBehavior {
+  public applyRuntimeBehavior(type: ICustomAttributeType | ICustomElementType, instance: ICustomAttribute | ICustomElement): void {
     let found = this.behaviorLookup.get(type);
 
     if (!found) {
-      found = RuntimeBehavior.create(instance, bindables, type);
+      found = RuntimeBehavior.create(type, instance);
       this.behaviorLookup.set(type, found);
     }
 
-    if ('$projector' in instance) {
-      found.applyToElement(this.changeSet, instance);
-    } else {
-      found.applyToAttribute(this.changeSet, instance);
-    }
-
-    return found;
+    found.applyTo(instance, this.changeSet);
   }
 
   public createViewFromComponent(context: IRenderContext, componentOrType: any, instruction: Immutable<IHydrateElementInstruction>): ViewWithCentralComponent {
@@ -314,7 +308,6 @@ export abstract class View implements IView {
     }
 
     this.$scope = scope;
-
     const bindables = this.$bindables;
 
     for (let i = 0, ii = bindables.length; i < ii; ++i) {