Merge pull request #611 from bigopon/templating-composition-engine

feat(CompositionEngine): allow constructors

src/composition-engine.js

@@ -128,22 +128,35 @@ export class CompositionEngine {
     let childContainer;
     let viewModel;
     let viewModelResource;
+    /**@type {HtmlBehaviorResource} */
     let m;
 
-    return this.ensureViewModel(context).then(tryActivateViewModel).then(() => {
-      childContainer = context.childContainer;
-      viewModel = context.viewModel;
-      viewModelResource = context.viewModelResource;
-      m = viewModelResource.metadata;
+    return this
+      .ensureViewModel(context)
+      .then(tryActivateViewModel)
+      .then(() => {
+        childContainer = context.childContainer;
+        viewModel = context.viewModel;
+        viewModelResource = context.viewModelResource;
+        m = viewModelResource.metadata;
 
-      let viewStrategy = this.viewLocator.getViewStrategy(context.view || viewModel);
+        let viewStrategy = this.viewLocator.getViewStrategy(context.view || viewModel);
 
-      if (context.viewResources) {
-        viewStrategy.makeRelativeTo(context.viewResources.viewUrl);
-      }
+        if (context.viewResources) {
+          viewStrategy.makeRelativeTo(context.viewResources.viewUrl);
+        }
 
-      return m.load(childContainer, viewModelResource.value, null, viewStrategy, true);
-    }).then(viewFactory => m.create(childContainer, BehaviorInstruction.dynamic(context.host, viewModel, viewFactory)));
+        return m.load(
+          childContainer,
+          viewModelResource.value,
+          null,
+          viewStrategy,
+          true
+        );
+      }).then(viewFactory => m.create(
+        childContainer,
+        BehaviorInstruction.dynamic(context.host, viewModel, viewFactory)
+      ));
   }
 
   /**
@@ -171,12 +184,26 @@ export class CompositionEngine {
         return context;
       });
     }
-
-    let m = metadata.getOrCreateOwn(metadata.resource, HtmlBehaviorResource, context.viewModel.constructor);
+    // When viewModel in context is not a module path
+    // only prepare the metadata and ensure the view model instance is ready
+    // if viewModel is a class, instantiate it
+    let isClass = typeof context.viewModel === 'function';
+    let ctor = isClass ? context.viewModel : context.viewModel.constructor;
+    let m = metadata.getOrCreateOwn(metadata.resource, HtmlBehaviorResource, ctor);
+    // We don't call ViewResources.prototype.convention here as it should be called later
+    // Just need to prepare the metadata for later usage
     m.elementName = m.elementName || 'dynamic-element';
-    m.initialize(context.container || childContainer, context.viewModel.constructor);
-    context.viewModelResource = { metadata: m, value: context.viewModel.constructor };
-    childContainer.viewModel = context.viewModel;
+    // HtmlBehaviorResource has its own guard to prevent unnecessary subsequent initialization calls
+    // so it's safe to call initialize this way
+    m.initialize(isClass ? childContainer : (context.container || childContainer), ctor);
+    // simulate the metadata of view model, like it was analyzed by module analyzer
+    // Cannot create a ResourceDescription instance here as it does too much
+    context.viewModelResource = { metadata: m, value: ctor };
+    // register the host element in case custom element view model declares it
+    if (context.host) {
+      childContainer.registerInstance(DOM.Element, context.host);
+    }
+    childContainer.viewModel = context.viewModel = isClass ? childContainer.get(ctor) : context.viewModel;
     return Promise.resolve(context);
   }
 

src/view-resources.js

@@ -158,6 +158,8 @@ export function validateBehaviorName(name: string, type: string) {
   return name;
 }
 
+const conventionMark = '__au_resource__';
+
 /**
  * Represents a collection of resources used during the compilation of a view.
  * Will optinally add information to an existing HtmlBehaviorResource if given
@@ -171,13 +173,20 @@ export class ViewResources {
    */
   static convention(target: Function, existing?: HtmlBehaviorResource): HtmlBehaviorResource | ValueConverterResource | BindingBehaviorResource | ViewEngineHooksResource {
     let resource;
+    // Use a simple string to mark that an HtmlBehaviorResource instance
+    // has been applied all resource information from its target view model class
+    // to prevent subsequence call re initialization all info again
+    if (existing && conventionMark in existing) {
+      return existing;
+    }
     if ('$resource' in target) {
       let config = target.$resource;
       // 1. check if resource config is a string
       if (typeof config === 'string') {
         // it's a custom element, with name is the resource variable
         // static $resource = 'my-element'
         resource = existing || new HtmlBehaviorResource();
+        resource[conventionMark] = true;
         if (!resource.elementName) {
           // if element name was not specified before
           resource.elementName = validateBehaviorName(config, 'custom element');
@@ -206,6 +215,7 @@ export class ViewResources {
         case 'element': case 'attribute':
           // if a metadata is supplied, use it
           resource = existing || new HtmlBehaviorResource();
+          resource[conventionMark] = true;
           if (resourceType === 'element') {
             // if element name was defined before applying convention here
             // it's a result from `@customElement` call (or manual modification)
@@ -262,13 +272,28 @@ export class ViewResources {
         // though it will finally resolves to only 1 `name` attribute
         // Will not break if it's done in that way but probably only happenned in inheritance scenarios.
         let bindables = typeof config === 'string' ? undefined : config.bindables;
+        let currentProps = resource.properties;
         if (Array.isArray(bindables)) {
           for (let i = 0, ii = bindables.length; ii > i; ++i) {
             let prop = bindables[i];
             if (!prop || (typeof prop !== 'string' && !prop.name)) {
               throw new Error(`Invalid bindable property at "${i}" for class "${target.name}". Expected either a string or an object with "name" property.`);
             }
-            new BindableProperty(prop).registerWith(target, resource);
+            let newProp = new BindableProperty(prop);
+            // Bindable properties defined in $resource convention
+            // shouldn't override existing prop with the same name
+            // as they could be explicitly defined via decorator, thus more trust worthy ?
+            let existed = false;
+            for (let j = 0, jj = currentProps.length; jj > j; ++j) {
+              if (currentProps[j].name === newProp.name) {
+                existed = true;
+                break;
+              }
+            }
+            if (existed) {
+              continue;
+            }
+            newProp.registerWith(target, resource);
           }
         }
       }

test/composition-engine.spec.js

@@ -0,0 +1,182 @@
+import './setup';
+import {Container} from 'aurelia-dependency-injection';
+import {createOverrideContext, OverrideContext} from 'aurelia-binding';
+import {HtmlBehaviorResource} from '../src/html-behavior';
+import {CompositionEngine} from '../src/composition-engine';
+import {TemplatingEngine} from '../src/templating-engine';
+import {inlineView, noView} from '../src/decorators';
+import {ViewResources} from '../src/view-resources';
+import {DOM} from 'aurelia-pal';
+import { ViewSlot } from '../src/view-slot';
+
+describe('enhance', () => {
+  /**@type {Container} */
+  let container;
+  let element;
+  let mockModule;
+  /**@type {CompositionEngine} */
+  let compositionEngine;
+  /**@type {CompositionContext} */
+  let compositionContext;
+
+  beforeEach(() => {
+    container = new Container();
+    element = DOM.createElement('div');
+    compositionEngine = container.get(CompositionEngine);
+    compositionContext = new CompositionContext({});
+  });
+
+  describe('ensureViewModel()', () => {
+
+    it('ensures view model when view model\'s a string', done => {
+      class MyClass {
+        message = 'My class';
+      }
+
+      mockModule = {
+        MyClass: MyClass
+      };
+      compositionEngine.viewEngine.loader.loadModule = () => new Promise(resolve => setTimeout(() => resolve(mockModule), 50));
+
+      compositionContext.host = document.createElement('div');
+      compositionContext.viewSlot = new ViewSlot(compositionContext.host, true);
+      compositionContext.container = container;
+      compositionContext.viewModel = '';
+      container.registerInstance(DOM.Element, compositionContext.host);
+
+      compositionEngine.ensureViewModel(compositionContext).then((context) => {
+        expect(context).toBe(compositionContext);
+        expect(context.viewModel instanceof MyClass).toBe(true);
+        done();
+      });
+    });
+
+    it('ensures view model when view model is a class', done => {
+      class MyClass {
+        message = 'My class';
+      }
+
+      compositionContext.host = document.createElement('div');
+      compositionContext.viewSlot = new ViewSlot(compositionContext.host, true);
+      compositionContext.container = container;
+      compositionContext.viewModel = MyClass;
+      container.registerInstance(DOM.Element, compositionContext.host);
+
+      compositionEngine.ensureViewModel(compositionContext).then((context) => {
+        expect(context).toBe(compositionContext);
+        expect(container.hasResolver(MyClass)).toBe(true);
+        expect(context.viewModel instanceof MyClass).toBe(true);
+        done();
+      });
+    });
+
+    it('ensures view model when view model is an object', done => {
+      class MyClass {
+        message = 'My class';
+      }
+
+      compositionContext.host = document.createElement('div');
+      compositionContext.viewSlot = new ViewSlot(compositionContext.host, true);
+      compositionContext.container = container;
+      compositionContext.viewModel = new MyClass();
+      container.registerInstance(DOM.Element, compositionContext.host);
+
+      compositionEngine.ensureViewModel(compositionContext).then((context) => {
+        expect(context).toBe(compositionContext);
+        expect(context.viewModel).toBe(compositionContext.viewModel);
+        expect(container.hasResolver(MyClass)).toBe(false);
+        expect(context.viewModel instanceof MyClass).toBe(true);
+        done();
+      });
+    });
+  });
+
+
+  /**
+  * Instructs the composition engine how to dynamically compose a component.
+  */
+  class CompositionContext {
+    /**
+     * @param {Partial<CompositionContext>} context
+     */
+    constructor(context) {
+
+      /**
+        * The parent Container for the component creation.
+        * @type {Container}
+        */
+      this.container = undefined;
+
+      /**
+        * The child Container for the component creation. One will be created from the parent if not provided.
+        * @type {Container}
+        */
+      this.childContainer = undefined;
+
+      /**
+        * The context in which the view model is executed in.
+        */
+      this.bindingContext = undefined;
+
+      /**
+        * A secondary binding context that can override the standard context.
+        * @type {OverrideContext}
+        */
+      this.overrideContext = undefined;
+
+      /**
+       * The view model url or instance for the component.
+       * @type {string | object}
+       */
+      this.viewModel = undefined;
+
+      /**
+       * Data to be passed to the "activate" hook on the view model.
+       */
+      this.model = undefined;
+
+      /**
+        * The HtmlBehaviorResource for the component.
+        * @type {HtmlBehaviorResource}
+        */
+      this.viewModelResource = undefined;
+
+      /**
+        * The view resources for the view in which the component should be created.
+        * @type {ViewResources}
+        */
+      this.viewResources = undefined;
+
+      /**
+        * The view inside which this composition is happening.
+        * @type {View}
+        */
+      this.owningView = undefined;
+
+      /**
+        * The view url or view strategy to override the default view location convention.
+        * @type {string | ViewStrategy}
+        */
+      this.view = undefined;
+
+      /**
+        * The slot to push the dynamically composed component into.
+        * @type {ViewSlot}
+        */
+      this.viewSlot = undefined;
+
+      /**
+        * Should the composition system skip calling the "activate" hook on the view model.
+        */
+      this.skipActivation = false;
+
+      /**
+        * The element that will parent the dynamic component.
+        * It will be registered in the child container of this composition.
+        * @type {Element}
+        */
+      this.host = null;
+      Object.assign(this, context);
+    }
+  }
+});

test/view-resources.spec.js

@@ -105,6 +105,23 @@ describe('ViewResources', () => {
       meta = ViewResources.convention(El1);
       expect(meta.elementName).toBe('el');
     });
+
+    it('does not reapply convention', () => {
+      class El  {
+        static $resource = {
+          bindables: ['value']
+        }
+      }
+      let meta = metadata.getOrCreateOwn(metadata.resource, HtmlBehaviorResource, El);
+      ViewResources.convention(El, meta);
+      expect(meta.elementName).toBe('el');
+      expect(meta.properties.length).toBe(1);
+      expect(meta.__au_resource__).toBe(true);
+
+      El.$resource.bindables.push('name', 'label', 'type');
+      ViewResources.convention(El, meta);
+      expect(meta.properties.length).toBe(1);
+    });
   });
 
   it('auto register', () => {
@@ -220,20 +237,20 @@ describe('ViewResources', () => {
       expect(resources.getAttribute('b').target).toBe(At)
     });
 
-    it('adds bindables', () => {
-
-      class El {
-        static $resource() {
-          return {
-            bindables: ['name', 'value']
-          }
-        }
-        @bindable() name
-        @bindable() value
-      }
-      resources.autoRegister(container, El);
-      expect(resources.getElement('el').properties.length).toBe(4);
-    });
+    // it('adds bindables', () => {
+
+    //   class El {
+    //     static $resource() {
+    //       return {
+    //         bindables: ['name', 'value']
+    //       }
+    //     }
+    //     @bindable() name
+    //     @bindable() value
+    //   }
+    //   resources.autoRegister(container, El);
+    //   expect(resources.getElement('el').properties.length).toBe(4);
+    // });
 
     describe('with inheritance', () => {