fix(engine): render() can only return qualifying templates (#764)

* fix(engine): render() can only return qualifying templates * chore: remove unneeded return statement
salesforce · Nov 5, 2018 · c6556de · c6556de
1 parent d2942fc
commit c6556de
Show file tree

Hide file tree

Showing 10 changed files with 51 additions and 14 deletions.
diff --git a/packages/lwc-engine/src/framework/__tests__/class-list.spec.ts b/packages/lwc-engine/src/framework/__tests__/class-list.spec.ts
@@ -1,6 +1,8 @@
 import { compileTemplate } from 'test-utils';
 import { createElement, LightningElement } from '../main';
 
+const emptyTemplate = compileTemplate(`<template></template>`);
+
 describe('class-list', () => {
     describe('integration', () => {
         it('should support static class attribute', () => {
@@ -237,6 +239,7 @@ describe('class-list', () => {
                 }
                 render() {
                     this.state.x;
+                    return emptyTemplate;
                 }
             }
             MyComponent.publicMethods = ['initClassNames', 'updateTracked', 'addAnotherClass', 'addOtherClass'];

diff --git a/packages/lwc-engine/src/framework/__tests__/invoker.spec.ts b/packages/lwc-engine/src/framework/__tests__/invoker.spec.ts
@@ -1,6 +1,8 @@
 import { compileTemplate } from 'test-utils';
 import { createElement, LightningElement } from '../main';
 
+const emptyTemplate = compileTemplate(`<template></template>`);
+
 describe('invoker', () => {
 
     describe('integration', () => {
@@ -9,7 +11,7 @@ describe('invoker', () => {
             document.body.innerHTML = '';
         });
 
-        it('should support undefined result from render()', () => {
+        it('should throw if render() returns undefined', () => {
             let counter = 0;
             class MyComponent extends LightningElement {
                 render() {
@@ -18,11 +20,13 @@ describe('invoker', () => {
                 }
             }
             const elm = createElement('x-foo', { is: MyComponent });
-            document.body.appendChild(elm);
+            expect(() => {
+                document.body.appendChild(elm);
+            }).toThrow();
             expect(counter).toBe(1);
         });
 
-        it('should throw if render() returns something that is not a function or a promise or undefined', () => {
+        it('should throw if render() returns something that is not a template function', () => {
             class MyComponent extends LightningElement {
                 render() {
                     return 1;
@@ -246,6 +250,7 @@ describe('invoker', () => {
                 }
                 render() {
                     lifecycle.push('render');
+                    return emptyTemplate;
                 }
             }
 

diff --git a/packages/lwc-engine/src/framework/__tests__/patch.spec.ts b/packages/lwc-engine/src/framework/__tests__/patch.spec.ts
@@ -1,6 +1,8 @@
 import { compileTemplate } from 'test-utils';
 import { createElement, LightningElement } from '../main';
 
+const emptyTemplate = compileTemplate(`<template></template>`);
+
 describe('patch', () => {
 
     describe('#patch()', () => {
@@ -55,6 +57,7 @@ describe('patch', () => {
                 }
                 render() {
                     calls.push('child:render');
+                    return emptyTemplate;
                 }
                 renderedCallback() {
                     calls.push('child:renderedCallback');
@@ -113,6 +116,7 @@ describe('patch', () => {
                 }
                 render() {
                     calls.push('child:render');
+                    return emptyTemplate;
                 }
                 renderedCallback() {
                     calls.push('child:renderedCallback');
@@ -190,6 +194,7 @@ describe('patch', () => {
                 }
                 render() {
                     calls.push('child:render');
+                    return emptyTemplate;
                 }
                 renderedCallback() {
                     calls.push('child:renderedCallback');

diff --git a/packages/lwc-engine/src/framework/__tests__/vm.spec.ts b/packages/lwc-engine/src/framework/__tests__/vm.spec.ts
@@ -3,6 +3,8 @@ import { createElement, LightningElement } from '../main';
 import { ViewModelReflection } from "../utils";
 import { getErrorComponentStack, isNodeFromTemplate } from "../vm";
 
+const emptyTemplate = compileTemplate(`<template></template>`);
+
 describe('vm', () => {
     describe('#isNodeFromTemplate', () => {
         it('should return false when element is created externally', () => {
@@ -104,6 +106,7 @@ describe('vm', () => {
                 }
                 render() {
                     counter++;
+                    return emptyTemplate;
                 }
             }
 

diff --git a/packages/lwc-engine/src/framework/__tests__/watcher.spec.ts b/packages/lwc-engine/src/framework/__tests__/watcher.spec.ts
@@ -1,6 +1,8 @@
 import { compileTemplate } from 'test-utils';
 import { createElement, LightningElement } from '../main';
 
+const emptyTemplate = compileTemplate(`<template></template>`);
+
 describe('watcher', () => {
 
     describe('integration', () => {
@@ -10,6 +12,7 @@ describe('watcher', () => {
             class MyComponent1 extends LightningElement {
                 render() {
                     counter++;
+                    return emptyTemplate;
                 }
             }
             const elm = createElement('x-foo', { is: MyComponent1 });
@@ -47,6 +50,7 @@ describe('watcher', () => {
             class MyComponent3 extends LightningElement {
                 render() {
                     counter++;
+                    return emptyTemplate;
                 }
             }
             MyComponent3.publicProps = { x: 1 };
@@ -147,6 +151,7 @@ describe('watcher', () => {
                 }
                 render() {
                     counter++;
+                    return emptyTemplate;
                 }
             }
 
@@ -265,6 +270,7 @@ describe('watcher', () => {
                 render() {
                     counter++;
                     this.state.list.map((v) => v + 1);
+                    return emptyTemplate;
                 }
             }
             MyComponent1.track = { state: 1 };
@@ -291,6 +297,7 @@ describe('watcher', () => {
                 render() {
                     counter++;
                     this.state.list.map((v) => v + 1);
+                    return emptyTemplate;
                 }
             }
             MyComponent1.publicMethods = ['popFromList'];
@@ -315,6 +322,7 @@ describe('watcher', () => {
                 render() {
                     counter++;
                     this.state.list.map((v) => v + 1);
+                    return emptyTemplate;
                 }
             }
             MyComponent1.publicMethods = ['unshiftFromList'];

diff --git a/packages/lwc-engine/src/framework/decorators/__tests__/api.spec.ts b/packages/lwc-engine/src/framework/decorators/__tests__/api.spec.ts
@@ -1,6 +1,8 @@
 import { compileTemplate } from 'test-utils';
 import { createElement, LightningElement } from '../../main';
 
+const emptyTemplate = compileTemplate(`<template></template>`);
+
 describe('decorators/api.ts', () => {
     describe('@api x', () => {
         it('should allow inheriting public props', function() {
@@ -50,6 +52,7 @@ describe('decorators/api.ts', () => {
             class MyComponent extends LightningElement  {
                 render() {
                     counter++;
+                    return emptyTemplate;
                 }
             }
             MyComponent.publicProps = {
@@ -74,6 +77,7 @@ describe('decorators/api.ts', () => {
                 render() {
                     this.x;
                     counter++;
+                    return emptyTemplate;
                 }
             }
             MyComponent.publicProps = {
@@ -178,6 +182,7 @@ describe('decorators/api.ts', () => {
                 render() {
                     this.x;
                     counter++;
+                    return emptyTemplate;
                 }
             }
             MyComponent.publicProps = {
@@ -209,6 +214,7 @@ describe('decorators/api.ts', () => {
                 render() {
                     this.x;
                     counter++;
+                    return emptyTemplate;
                 }
             }
             MyComponent.publicProps = {

diff --git a/packages/lwc-engine/src/framework/decorators/__tests__/track.spec.ts b/packages/lwc-engine/src/framework/decorators/__tests__/track.spec.ts
@@ -1,7 +1,10 @@
+import { compileTemplate } from 'test-utils';
 import { createElement, LightningElement } from '../../main';
 import track from "../track";
 import readonly from "../readonly";
 
+const emptyTemplate = compileTemplate(`<template></template>`);
+
 describe('track.ts', () => {
     describe('integration', () => {
         it('should support setting a tracked property in constructor', () => {
@@ -56,6 +59,7 @@ describe('track.ts', () => {
                 render() {
                     counter++;
                     this.foo.x;
+                    return emptyTemplate;
                 }
             }
             MyComponent.track = { foo: 1 };
@@ -81,6 +85,7 @@ describe('track.ts', () => {
                 render() {
                     counter++;
                     this.foo.x;
+                    return emptyTemplate;
                 }
             }
             MyComponent.track = { foo: 1 };
@@ -194,6 +199,7 @@ describe('track.ts', () => {
             class MyComponent extends LightningElement {
                 render() {
                     this.foo = 1;
+                    return emptyTemplate;
                 }
             }
             MyComponent.track = { foo: {  } };

diff --git a/packages/lwc-engine/src/framework/decorators/__tests__/wire.spec.ts b/packages/lwc-engine/src/framework/decorators/__tests__/wire.spec.ts
@@ -1,6 +1,9 @@
+import { compileTemplate } from 'test-utils';
 import { createElement, LightningElement } from '../../main';
 import wire from "../wire";
 
+const emptyTemplate = compileTemplate(`<template></template>`);
+
 describe('wire.ts', () => {
     describe('integration', () => {
         it('should support setting a wired property in constructor', () => {
@@ -55,6 +58,7 @@ describe('wire.ts', () => {
                 render() {
                     counter++;
                     this.foo.x;
+                    return emptyTemplate;
                 }
             }
             MyComponent.wire = { foo: {} };
@@ -82,6 +86,7 @@ describe('wire.ts', () => {
                 render() {
                     counter++;
                     this.foo.x;
+                    return emptyTemplate;
                 }
             }
             MyComponent.wire = { foo: {} };
@@ -197,6 +202,7 @@ describe('wire.ts', () => {
             class MyComponent extends LightningElement {
                 render() {
                     this.foo = 1;
+                    return emptyTemplate;
                 }
             }
             MyComponent.wire = { foo: {  } };

diff --git a/packages/lwc-engine/src/framework/invoker.ts b/packages/lwc-engine/src/framework/invoker.ts
@@ -93,13 +93,10 @@ export function invokeComponentRenderMethod(vm: VM): VNodes {
 
     try {
         const html = callHook(component, render);
-        if (isFunction(html)) {
-            result = evaluateTemplate(vm, html);
-        } else if (!isUndefined(html)) {
-            if (process.env.NODE_ENV !== 'production') {
-                assert.fail(`The template rendered by ${vm} must return an imported template tag (e.g.: \`import html from "./${vm.def.name}.html"\`) or undefined, instead, it has returned ${html}.`);
-            }
+        if (process.env.NODE_ENV !== 'production') {
+            assert.isTrue(isFunction(html), `The template rendered by ${vm} must return an imported template tag (e.g.: \`import html from "./${vm.def.name}.html"\`), instead, it has returned ${html}.`);
         }
+        result = evaluateTemplate(vm, html);
     } catch (e) {
         error = Object(e);
     } finally {

diff --git a/packages/lwc-engine/src/framework/template.ts b/packages/lwc-engine/src/framework/template.ts
@@ -69,11 +69,9 @@ export function evaluateTemplate(vm: VM, html: Template): Array<VNode|null> {
     const { component, context, cmpSlots, cmpTemplate } = vm;
     // reset the cache memoizer for template when needed
     if (html !== cmpTemplate) {
-        if (!isUndefined(cmpTemplate)) {
-            // It is important to reset the content to avoid reusing similar elements generated from a different
-            // template, because they could have similar IDs, and snabbdom just rely on the IDs.
-            resetShadowRoot(vm);
-        }
+        // It is important to reset the content to avoid reusing similar elements generated from a different
+        // template, because they could have similar IDs, and snabbdom just rely on the IDs.
+        resetShadowRoot(vm);
 
         // Check that the template was built by the compiler
         if (!isTemplateRegistered(html)) {