Updates based on code review.

Author: kevinpschaaf

lib/elements/dom-if.js

@@ -251,7 +251,9 @@ export class DomIf extends PolymerElement {
       if (c$ && c$.length) {
         // use first child parent, for case when dom-if may have been detached
         let parent = c$[0].parentNode;
-        if (parent) {
+          // Instance children may be disconnected from parents when dom-if
+          // detaches if a tree was innerHTML'ed
+          if (parent) {
           for (let i=0, n; (i<c$.length) && (n=c$[i]); i++) {
             parent.removeChild(n);
           }

lib/elements/dom-module.js

@@ -14,6 +14,12 @@ import { strictTemplatePolicy } from '../utils/settings.js';
 
 let modules = {};
 let lcModules = {};
+function setModule(id, module) {
+  // store id separate from lowercased id so that
+  // in all cases mixedCase id will stored distinctly
+  // and lowercase version is a fallback
+  modules[id] = lcModules[id.toLowerCase()] = module;
+}
 function findModule(id) {
   return modules[id] || lcModules[id.toLowerCase()];
 }
@@ -122,16 +128,14 @@ export class DomModule extends HTMLElement {
   register(id) {
     id = id || this.id;
     if (id) {
-      if (strictTemplatePolicy && findModule(id)) {
-        modules[id] = lcModules[id.toLowerCase()] = null;
-        throw new Error(`strictTemplatePolicy: dom-module ${id} registered twice`);
+      // Under strictTemplatePolicy, reject and null out any re-registered
+      // dom-module since it is ambiguous whether first-in or last-in is trusted 
+      if (strictTemplatePolicy && findModule(id) !== undefined) {
+        setModule(id, null);
+        throw new Error(`strictTemplatePolicy: dom-module ${id} re-registered`);
       }
       this.id = id;
-      // store id separate from lowercased id so that
-      // in all cases mixedCase id will stored distinctly
-      // and lowercase version is a fallback
-      modules[id] = this;
-      lcModules[id.toLowerCase()] = this;
+      setModule(id, this);
       styleOutsideTemplateCheck(this);
     }
   }

lib/legacy/class.js

@@ -150,22 +150,6 @@ function GenerateClassFromInfo(info, Base) {
       return info.observers;
     }
 
-    /**
-     * @return {HTMLTemplateElement} template for this class
-     */
-    static get template() {
-      // get template first from any imperative set in `info._template`
-      return info._template ||
-        // next look in dom-module associated with this element's is.
-        (!strictTemplatePolicy && (DomModule && DomModule.import(this.is, 'template'))) ||
-        // next look for superclass template (note: use superclass symbol
-        // to ensure correct `this.is`)
-        Base.template ||
-        // finally fall back to `_template` in element's prototype.
-        this.prototype._template ||
-        null;
-    }
-
     /**
      * @return {void}
      */
@@ -363,5 +347,10 @@ export const Class = function(info) {
     LegacyElementMixin(HTMLElement));
   // decorate klass with registration info
   klass.is = info.is;
-  return klass;
+  // if user provided template on info, make sure the static _template
+  // is set so the static template getter uses it
+  if (info._template !== undefined) {
+    klass._template = info._template;
+  }
+return klass;
 };

lib/mixins/element-mixin.js

@@ -16,7 +16,7 @@ import { pathFromUrl, resolveCss, resolveUrl as resolveUrl$0 } from '../utils/re
 import { DomModule } from '../elements/dom-module.js';
 import { PropertyEffects } from './property-effects.js';
 import { PropertiesMixin } from './properties-mixin.js';
-import { strictTemplatePolicy } from '../utils/settings.js';
+import { strictTemplatePolicy, allowTemplateFromDomModule } from '../utils/settings.js';
 
 /**
  * Element class mixin that provides the core API for Polymer's meta-programming
@@ -270,6 +270,27 @@ export const ElementMixin = dedupingMixin(base => {
     }
   }
 
+  /**
+   * Look up template from dom-module for element
+   * 
+   * @param {!string} is Element name to look up
+   * @return {!HTMLTemplateElement} Template found in dom module, or
+   *   undefined if not found
+   * @private
+   */
+  function getTemplateFromDomModule(is) {
+    let template = null;
+    if (is && allowTemplateFromDomModule) {
+      template = DomModule.import(is, 'template')
+      // Under strictTemplatePolicy, require any element with an `is`
+      // specified to have a dom-module
+      if (strictTemplatePolicy && !template) {
+        throw new Error(`strictTemplatePolicy: expecting dom-module or null template for ${is}`);
+      }
+    }
+    return template;
+  }
+
   /**
    * @polymer
    * @mixinClass
@@ -379,15 +400,18 @@ export const ElementMixin = dedupingMixin(base => {
      */
     static get template() {
       if (!this.hasOwnProperty(JSCompiler_renameProperty('_template', this))) {
-        this._template = (!strictTemplatePolicy && DomModule && DomModule.import(
-          /** @type {PolymerElementConstructor}*/ (this).is, 'template')) ||
-          // note: implemented so a subclass can retrieve the super
-          // template; call the super impl this way so that `this` points
-          // to the superclass.
-          Object.getPrototypeOf(/** @type {PolymerElementConstructor}*/ (this).prototype).constructor.template;
+        this._template =
+          // Look in dom-module associated with this element's is
+          getTemplateFromDomModule(/** @type {PolymerElementConstructor}*/ (this).is) ||
+          // Next look for superclass template (call the super impl this
+          // way so that `this` points to the superclass)
+          Object.getPrototypeOf(/** @type {PolymerElementConstructor}*/ (this).prototype).constructor.template ||
+          // Finally, fall back to any _template set on prototype, e.g.
+          // via registered callback
+          this.prototype._template;
       }
       return this._template;
-    }
+  }
 
     /**
      * Path matching the url from which the element was imported.
@@ -414,7 +438,7 @@ export const ElementMixin = dedupingMixin(base => {
         if (meta) {
           this._importPath = pathFromUrl(meta.url);
         } else {
-          const module = DomModule && DomModule.import(/** @type {PolymerElementConstructor} */ (this).is);
+          const module = DomModule.import(/** @type {PolymerElementConstructor} */ (this).is);
           this._importPath = (module && module.assetpath) ||
             Object.getPrototypeOf(/** @type {PolymerElementConstructor}*/ (this).prototype).constructor.importPath;
         }

lib/utils/settings.js

@@ -86,8 +86,8 @@ export const setPassiveTouchGestures = function(usePassive) {
 
 /**
  * Setting to ensure Polymer template evaluation only occurs based on tempates
- * defined in trusted script.  When true, `<dom-module>` based template lookup
- * is disabled, `<dom-bind>` is disabled, and `<dom-if>`/`<dom-repeat>`
+ * defined in trusted script.  When true, `<dom-module>` re-registration is
+ * disallowed, `<dom-bind>` is disabled, and `<dom-if>`/`<dom-repeat>`
  * templates will only evaluate in the context of a trusted element template.
  */
 export let strictTemplatePolicy = false;
@@ -102,3 +102,22 @@ export let strictTemplatePolicy = false;
 export const setStrictTemplatePolicy = function(useStrictPolicy) {
   strictTemplatePolicy = useStrictPolicy;
 };
+
+/**
+ * Setting to enable dom-module lookup from Polymer.Element.  By default,
+ * templates must be defined in script using the `static get template()`
+ * getter and the `html` tag function.  To enable legacy loading of templates
+ * via dom-module, set this flag to true.
+ */
+export let allowTemplateFromDomModule = false;
+
+/**
+ * Sets `lookupTemplateFromDomModule` globally for all elements
+ *
+ * @param {boolean} allowDomModule enable or disable template lookup 
+ *   globally
+ * @return {void}
+ */
+export const setAllowTemplateFromDomModule = function(allowDomModule) {
+  allowTemplateFromDomModule = allowDomModule;
+};

lib/utils/templatize.js

@@ -496,6 +496,9 @@ and this string can then be deleted`;
  * @suppress {invalidCasts}
  */
 export function templatize(template, owner, options) {
+  // Under strictTemplatePolicy, the templatized element must be owned
+  // by a (trusted) Polymer element, indicated by existence of _methodHost;
+  // e.g. for dom-if & dom-repeat in main document, _methodHost is null
   if (strictTemplatePolicy && !owner._methodHost) {
     throw new Error('strictTemplatePolicy: template owner not trusted');
   }

test/unit/strict-template-policy.html

@@ -37,34 +37,57 @@
 </head>
 <body>
 
-  <dom-module id="trusted-element">
+  <dom-module id="no-dm">
     <template>Should never be used</template>
     <script type="module">
       import {PolymerElement} from '../../polymer-element.js';
       class TrustedElement extends PolymerElement {
-        static get is() { return 'trusted-element'; }
+        static get is() { return 'no-dm'; }
       }
       customElements.define(TrustedElement.is, TrustedElement);
     </script>
   </dom-module>
 
-  <dom-module id="trusted-element-legacy">
+  <dom-module id="no-dm-legacy">
     <template>Should never be used</template>
     <script type="module">
       import {Polymer} from '../../polymer-legacy.js';
-      Polymer({is: 'trusted-element-legacy'});
+      Polymer({is: 'no-dm-legacy'});
     </script>
   </dom-module>
 
+  <dom-module id="trusted-element">
+    <template>Trusted</template>
+    <script type="module">
+      import {PolymerElement} from '../../polymer-element.js';
+      class TrustedElement extends PolymerElement {
+        static get is() { return 'trusted-element'; }
+      }
+      customElements.define(TrustedElement.is, TrustedElement);
+    </script>
+  </dom-module>
+    
+  <dom-module id="trusted-element-legacy">
+    <template>Trusted</template>
+    <script type="module">
+      import {Polymer} from '../../polymer-legacy.js';
+      Polymer({is: 'trusted-element-legacy'});
+    </script>
+  </dom-module>
+  
   <div id="target"></div>
-
+    
   <script type="module">
+    import {PolymerElement} from '../../polymer-element.js';
+    import {Polymer} from '../../polymer-legacy.js';
     import {flush} from '../../lib/utils/flush.js';
+    import {setAllowTemplateFromDomModule} from '../../lib/utils/settings.js';
 
     suite('strictTemplatePolicy', function() {
 
       teardown(function() {
         window.uncaughtErrorFilter = window.top.uncaughtErrorFilter = null;
+        document.getElementById('target').textContent = '';
       });
 
       // Errors thrown in custom element reactions are not thrown up
@@ -76,7 +99,9 @@
         // Safari errors are thrown on the top window, sometimes not, so
         // catch in both places
         window.uncaughtErrorFilter = window.top.uncaughtErrorFilter = function(err) {
-          uncaughtError = err;
+          if (!uncaughtError) {
+            uncaughtError = err;
+          }
           return true;
         };
         assert.throws(function() {
@@ -131,28 +156,104 @@
       });
 
       test('dom-module never used', function() {
-        var el = document.createElement('trusted-element');
+        var el = document.createElement('no-dm');
         document.getElementById('target').appendChild(el);
         assert.notOk(el.shadowRoot);
       });
 
       test('dom-module never used (legacy)', function() {
-        var el = document.createElement('trusted-element-legacy');
+        var el = document.createElement('no-dm-legacy');
         document.getElementById('target').appendChild(el);
         assert.notOk(el.shadowRoot);
       });
 
-      test('dom-module re-registration throws', function() {
+      // From this point down, dom-module lookup is globally enabled
+      test('setAllowTemplateFromDomModule', function() {
+        setAllowTemplateFromDomModule(true);
+      });
+
+      test('dom-module after registration', function() {
         assertThrows(function() {
           document.getElementById('target').innerHTML =
             '<dom-module id="trusted-element">' +
             '  <template>' +
             '    <div id="injected"></div>'+
             '  </template>`' +
-            '</dom-module>';
-        }, /trusted-element registered twice/);
+            '</dom-module>' +
+            '<trusted-element></trusted-element>';
+        }, /trusted-element re-registered/);
+        const el = document.querySelector('trusted-element');
+        assert.notOk(el && el.shadowRoot && el.shadowRoot.querySelector('#injected'));
+      });
+      
+      test('dom-module before registration', function() {
+        document.getElementById('target').innerHTML =
+          '<dom-module id="has-no-template">' +
+          '  <template>' +
+          '    <div id="injected"></div>'+
+          '  </template>`' +
+          '</dom-module>';
+        class HasNoTemplate extends PolymerElement {
+          static get is() { return 'has-no-template'; }
+          static get template() { return null; }
+        }
+        customElements.define(HasNoTemplate.is, HasNoTemplate);
+        let el = document.createElement('has-no-template');
+        document.getElementById('target').appendChild(el);
+        assert.notOk(el.shadowRoot);
+      });
+
+      test('dom-module after registration (legacy)', function() {
+        assertThrows(function() {
+          document.getElementById('target').innerHTML =
+            '<dom-module id="trusted-element-legacy">' +
+            '  <template>' +
+            '    <div id="injected"></div>'+
+            '  </template>`' +
+            '</dom-module>' +
+            '<trusted-element-legacy></trusted-element-legacy>';
+        }, /trusted-element-legacy re-registered/);
+        const el = document.querySelector('trusted-element-legacy');
+        assert.notOk(el && el.shadowRoot && el.shadowRoot.querySelector('#injected'));
+      });
+      
+      test('dom-module before registration (legacy)', function() {
+        document.getElementById('target').innerHTML =
+          '<dom-module id="has-no-template-legacy">' +
+          '  <template>' +
+          '    <div id="injected"></div>'+
+          '  </template>`' +
+          '</dom-module>';
+        Polymer({
+          is: 'has-no-template-legacy',
+          _template: null
+        });
+        let el = document.createElement('has-no-template-legacy');
+        document.getElementById('target').appendChild(el);
+        assert.notOk(document.querySelector('has-no-template-legacy').shadowRoot);
       });
 
+      test('element without explicit template throws', function() {
+        assertThrows(function() {
+          class HasNoTemplateThrows extends PolymerElement {
+            static get is() { return 'has-no-template-throws'; }
+          }
+          customElements.define(HasNoTemplateThrows.is, HasNoTemplateThrows); 
+          var el = document.createElement('has-no-template-throws');
+          document.getElementById('target').appendChild(el);
+        }, /expecting dom-module or null template/);
+      });
+
+      test('element without explicit template throws (legacy)', function() {
+        assertThrows(function() {
+          Polymer({
+            is: 'has-no-template-throws-legacy'
+          });
+          var el = document.createElement('has-no-template-throws-legacy');
+          document.getElementById('target').appendChild(el);
+        }, /expecting dom-module or null template/);
+      });
+      
     });
   </script>