fix: use more helpful error message for invalid render() return value…

… @W-14785069 (#3943)

* fix: use more helpful error message for invalid render() return value

* fix: use tag name in error instead of [object Object]

* test(render): assert helpful error thrown on invalid render

* chore: remove unnecessary optional parameter

* fix: use suggested matcher

* refactor(template): remove redundant validation
The `isTemplateRegistered` check inside `runWithBoundaryProtection`
is equivalent, but more robust, and with a more clear error message.

* test: expect non-dev error

* fix: print tag name instead of "[object Object]"

* fix: assert template is defined before using it

* fix: consolidate errors that cover the same case

an undefined template cannot be registered, so the `isTemplateRegistered` check is sufficient

packages/@lwc/engine-core/src/framework/template.ts

@@ -9,7 +9,6 @@ import {
     assert,
     create,
     isArray,
-    isFunction,
     isNull,
     isTrue,
     isUndefined,
@@ -202,12 +201,6 @@ export const parseSVGFragment = buildParseFragmentFn((html, renderer) => {
 
 export function evaluateTemplate(vm: VM, html: Template): VNodes {
     if (process.env.NODE_ENV !== 'production') {
-        assert.isTrue(
-            isFunction(html),
-            `evaluateTemplate() second argument must be an imported template instead of ${toString(
-                html
-            )}`
-        );
         // in dev-mode, we support hot swapping of templates, which means that
         // the component instance might be attempting to use an old version of
         // the template, while internally, we have a replacement for it.
@@ -231,6 +224,17 @@ export function evaluateTemplate(vm: VM, html: Template): VNodes {
             tro.observe(() => {
                 // Reset the cache memoizer for template when needed.
                 if (html !== cmpTemplate) {
+                    // Check that the template was built by the compiler.
+                    if (!isTemplateRegistered(html)) {
+                        throw new TypeError(
+                            `Invalid template returned by the render() method on ${
+                                vm.tagName
+                            }. It must return an imported template (e.g.: \`import html from "./${
+                                vm.def.name
+                            }.html"\`), instead, it has returned: ${toString(html)}.`
+                        );
+                    }
+
                     if (process.env.NODE_ENV !== 'production') {
                         validateLightDomTemplate(html, vm);
                     }
@@ -244,15 +248,6 @@ export function evaluateTemplate(vm: VM, html: Template): VNodes {
                         resetComponentRoot(vm);
                     }
 
-                    // Check that the template was built by the compiler.
-                    if (!isTemplateRegistered(html)) {
-                        throw new TypeError(
-                            `Invalid template returned by the render() method on ${vm}. It must return an imported template (e.g.: \`import html from "./${
-                                vm.def.name
-                            }.html"\`), instead, it has returned: ${toString(html)}.`
-                        );
-                    }
-
                     vm.cmpTemplate = html;
 
                     // Create a brand new template cache for the swapped templated.

packages/@lwc/integration-karma/test/api/registerTemplate/index.spec.js

@@ -24,7 +24,7 @@ it('should throw if a component tries to use a template that is not registered',
     // This will always throw synchronously because the component has no API version, and thus defaults
     // to the oldest (i.e. synthetic custom element lifecycle events)
     expect(func).toThrowError(TypeError);
-    expect(func).toThrowError(/Invalid template returned by the render\(\) method on \[.*\]\./);
+    expect(func).toThrowError(/Invalid template returned by the render\(\) method on x-test/);
 });
 
 it('should not throw if the template is registered first', () => {

packages/@lwc/integration-karma/test/component/LightningElement.render/index.spec.js

@@ -3,6 +3,7 @@ import { customElementCallbackReactionErrorListener } from 'test-utils';
 
 import DynamicTemplate, { template1, template2 } from 'x/dynamicTemplate';
 import RenderThrow from 'x/renderThrow';
+import RenderInvalid from 'x/renderInvalid';
 
 function testInvalidTemplate(type, template) {
     it(`throws an error if returns ${type}`, () => {
@@ -82,3 +83,10 @@ it('supports returning different templates', () => {
         expect(elm.shadowRoot.textContent).toBe('Template 2');
     });
 });
+
+it('throws an error when render() returns an invalid value', () => {
+    const elm = createElement('x-render-invalid', { is: RenderInvalid });
+    expect(() => document.body.appendChild(elm)).toThrowCallbackReactionError(
+        /Invalid template returned by the render\(\) method on x-render-invalid/
+    );
+});

packages/@lwc/integration-karma/test/component/LightningElement.render/x/renderInvalid/renderInvalid.js

@@ -0,0 +1,7 @@
+import { LightningElement } from 'lwc';
+
+export default class RenderInvalid extends LightningElement {
+    render() {
+        return undefined;
+    }
+}