Merge pull request #1527 from emberjs/remove-incorrect-assertions

Remove incorrect assertions

Author: NullVoxPopuli

addon/src/setup-context.ts

@@ -355,11 +355,6 @@ export function getWarningsDuringCallback(
   return getWarningsDuringCallbackForContext(context, callback);
 }
 
-// This WeakMap is used to track whenever a component is rendered in a test so that we can throw
-// assertions when someone uses `this.{set,setProperties}` while rendering a component.
-export const ComponentRenderMap = new WeakMap<BaseContext, true>();
-export const SetUsage = new WeakMap<BaseContext, Array<string>>();
-
 /**
   Used by test framework addons to setup the provided context for testing.
 
@@ -432,24 +427,8 @@ export default function setupContext<T extends object>(
         // SAFETY: in all of these `defineProperty` calls, we can't actually guarantee any safety w.r.t. the corresponding field's type in `TestContext`
         value(key: any, value: any): unknown {
           const ret = run(function () {
-            if (ComponentRenderMap.has(context)) {
-              assert(
-                'You cannot call `this.set` when passing a component to `render()` (the rendered component does not have access to the test context).',
-              );
-            } else {
-              let setCalls = SetUsage.get(context);
-
-              if (setCalls === undefined) {
-                setCalls = [];
-                SetUsage.set(context, setCalls);
-              }
-
-              setCalls?.push(key);
-            }
-
             return set(context, key, value);
           });
-
           return ret;
         },
         writable: false,
@@ -461,26 +440,6 @@ export default function setupContext<T extends object>(
         // SAFETY: in all of these `defineProperty` calls, we can't actually guarantee any safety w.r.t. the corresponding field's type in `TestContext`
         value(hash: any): unknown {
           const ret = run(function () {
-            if (ComponentRenderMap.has(context)) {
-              assert(
-                'You cannot call `this.setProperties` when passing a component to `render()` (the rendered component does not have access to the test context)',
-              );
-            } else {
-              // While neither the types nor the API documentation indicate that passing `null` or
-              // `undefined` to `setProperties` is allowed, it works and *has worked* for a long
-              // time, so there is considerable real-world code which relies on the fact that it
-              // does in fact work. Checking and no-op-ing here handles that.
-              if (hash != null) {
-                let setCalls = SetUsage.get(context);
-
-                if (SetUsage.get(context) === undefined) {
-                  setCalls = [];
-                  SetUsage.set(context, setCalls);
-                }
-
-                setCalls?.push(...Object.keys(hash));
-              }
-            }
             return setProperties(context, hash);
           });
 

addon/src/setup-rendering-context.ts

@@ -11,11 +11,9 @@ import settled from './settled.ts';
 import getRootElement from './dom/get-root-element.ts';
 import type { Owner } from './build-owner.ts';
 import getTestMetadata from './test-metadata.ts';
-import { assert } from '@ember/debug';
 import { runHooks } from './helper-hooks.ts';
 import hasEmberVersion from './has-ember-version.ts';
 import isComponent from './-internal/is-component.ts';
-import { ComponentRenderMap, SetUsage } from './setup-context.ts';
 
 // the built in types do not provide types for @ember/template-compilation
 // eslint-disable-next-line @typescript-eslint/ban-ts-comment
@@ -134,20 +132,6 @@ export function render(
       const ownerToRenderFrom = options?.owner || owner;
 
       if (isComponent(templateFactoryOrComponent)) {
-        // We use this to track when `render` is used with a component so that we can throw an
-        // assertion if `this.{set,setProperty} is used in the same test
-        ComponentRenderMap.set(context, true);
-
-        const setCalls = SetUsage.get(context);
-
-        if (setCalls !== undefined) {
-          assert(
-            `You cannot call \`this.set\` or \`this.setProperties\` when passing a component to \`render\`, but they were called for the following properties:\n${setCalls
-              .map((key) => `  - ${key}`)
-              .join('\n')}`,
-          );
-        }
-
         context = {
           ProvidedComponent: templateFactoryOrComponent,
         };

test-app/tests/unit/setup-rendering-context-test.js

@@ -654,76 +654,6 @@ module('setupRenderingContext', function (hooks) {
           assert.equal(this.element.textContent, 'the value of foo is foo');
         });
 
-        test('setting properties on the test context *before* rendering a component throws an assertion', async function (assert) {
-          assert.expect(1);
-          const template = precompileTemplate(
-            'the value of foo is {{this.foo}}'
-          );
-
-          class Foo extends GlimmerComponent {}
-
-          const component = setComponentTemplate(template, Foo);
-
-          this.set('foo', 'FOO');
-          this.set('bar', 'BAR');
-          this.setProperties({
-            baz: 'BAZ',
-            baq: 'BAQ',
-          });
-
-          let error;
-          try {
-            await render(component);
-          } catch (err) {
-            error = err;
-          } finally {
-            assert.equal(
-              error.toString(),
-              `Error: Assertion Failed: You cannot call \`this.set\` or \`this.setProperties\` when passing a component to \`render\`, but they were called for the following properties:
-  - foo
-  - bar
-  - baz
-  - baq`
-            );
-          }
-        });
-
-        test('setting properties on the test context *after* rendering a component throws an assertion', async function (assert) {
-          const template = precompileTemplate(
-            'the value of foo is {{this.foo}}'
-          );
-
-          class Foo extends GlimmerComponent {}
-
-          const component = setComponentTemplate(template, Foo);
-
-          await render(component);
-
-          assert.throws(
-            () => this.set('foo', 'FOO'),
-            (err) => {
-              return err
-                .toString()
-                .includes(
-                  'You cannot call `this.set` when passing a component to `render()` (the rendered component does not have access to the test context).'
-                );
-            },
-            'errors on this.set'
-          );
-
-          assert.throws(
-            () => this.setProperties({ foo: 'bar?' }),
-            (err) => {
-              return err
-                .toString()
-                .includes(
-                  'You cannot call `this.setProperties` when passing a component to `render()` (the rendered component does not have access to the test context)'
-                );
-            },
-            'errors on this.setProperties'
-          );
-        });
-
         test('setting properties on the test context when rendering a template does not throw an assertion', async function (assert) {
           const template = precompileTemplate(
             'the value of foo is {{this.foo}}'
@@ -811,76 +741,6 @@ module('setupRenderingContext', function (hooks) {
           assert.equal(this.element.textContent, 'the value of foo is foo');
         });
 
-        test('setting properties on the test context *before* rendering a component throws an assertion', async function (assert) {
-          assert.expect(1);
-          const template = precompileTemplate(
-            'the value of foo is {{this.foo}}'
-          );
-
-          class Foo extends GlimmerComponent {}
-
-          const component = setComponentTemplate(template, Foo);
-
-          this.set('foo', 'FOO');
-          this.set('bar', 'BAR');
-          this.setProperties({
-            baz: 'BAZ',
-            baq: 'BAQ',
-          });
-
-          let error;
-          try {
-            await render(component);
-          } catch (err) {
-            error = err;
-          } finally {
-            assert.equal(
-              error.toString(),
-              `Error: Assertion Failed: You cannot call \`this.set\` or \`this.setProperties\` when passing a component to \`render\`, but they were called for the following properties:
-  - foo
-  - bar
-  - baz
-  - baq`
-            );
-          }
-        });
-
-        test('setting properties on the test context *after* rendering a component throws an assertion', async function (assert) {
-          const template = precompileTemplate(
-            'the value of foo is {{this.foo}}'
-          );
-
-          class Foo extends GlimmerComponent {}
-
-          const component = setComponentTemplate(template, Foo);
-
-          await render(component);
-
-          assert.throws(
-            () => this.set('foo', 'FOO'),
-            (err) => {
-              return err
-                .toString()
-                .includes(
-                  'You cannot call `this.set` when passing a component to `render()` (the rendered component does not have access to the test context).'
-                );
-            },
-            'errors on this.set'
-          );
-
-          assert.throws(
-            () => this.setProperties({ foo: 'bar?' }),
-            (err) => {
-              return err
-                .toString()
-                .includes(
-                  'You cannot call `this.setProperties` when passing a component to `render()` (the rendered component does not have access to the test context)'
-                );
-            },
-            'errors on this.setProperties'
-          );
-        });
-
         test('setting properties on the test context when rendering a template does not throw an assertion', async function (assert) {
           const template = precompileTemplate(
             'the value of foo is {{this.foo}}'