Skip to content

Commit

Permalink
bugfix: handle null and undefined in setProperties
Browse files Browse the repository at this point in the history
This implicitly worked for as long as `this.setProperties` existed,
because it ultimately forwards to Ember's `setProperties`, which also
implicitly works with this. Although this is not public API, and indeed
is not desirable, changing it (implicitly and accidentally!) will break
many consumers who are relying on it (most likely to end up happening
transitively, e.g. via local/custom test helpers).

(cherry picked from commit 90b1685)
  • Loading branch information
chriskrycho committed Jun 15, 2023
1 parent 51804d7 commit 98b1bf2
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 7 deletions.
20 changes: 13 additions & 7 deletions addon-test-support/@ember/test-helpers/setup-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -464,14 +464,20 @@ export default function setupContext<T extends object>(
'You cannot call `this.setProperties` when passing a component to `render()` (the rendered component does not have access to the test context)'
);
} else {
let setCalls = SetUsage.get(context);

if (SetUsage.get(context) === undefined) {
setCalls = [];
SetUsage.set(context, setCalls);
// 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));
}

setCalls?.push(...Object.keys(hash));
}
return setProperties(context, hash);
});
Expand Down
22 changes: 22 additions & 0 deletions tests/unit/setup-context-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,28 @@ module('setupContext', function (hooks) {
);
});

test('this.setProperties handles `null` and `undefined` (back-compat only!)', function (assert) {
assert.expect(3);

context.setProperties();
assert.ok(
true,
'this.setProperties works with no args (for back-compat only)'
);

context.setProperties(null);
assert.ok(
true,
'this.setProperties works with `null` (for back-compat only)'
);

context.setProperties(undefined);
assert.ok(
true,
'this.setProperties works with `undefined` (for back-compat only)'
);
});

test('it sets up this.get', function (assert) {
context.set('foo', 'bar');

Expand Down

0 comments on commit 98b1bf2

Please sign in to comment.