Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove incorrect assertions #1527

Merged
merged 1 commit into from
Jan 17, 2025
Merged

Remove incorrect assertions #1527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 0 additions & 41 deletions addon/src/setup-context.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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,
Expand All @@ -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);
});

Expand Down
16 changes: 0 additions & 16 deletions addon/src/setup-rendering-context.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
};
Expand Down
140 changes: 0 additions & 140 deletions test-app/tests/unit/setup-rendering-context-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}}'
Expand Down Expand Up @@ -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}}'
Expand Down
Loading