From 98b1bf22e8d456bb41c13b0a95e352977fd098ba Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Thu, 15 Jun 2023 09:05:11 -0600 Subject: [PATCH] bugfix: handle `null` and `undefined` in `setProperties` 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 90b16859757ffe570e289af32b9bc15986b292de) --- .../@ember/test-helpers/setup-context.ts | 20 +++++++++++------ tests/unit/setup-context-test.js | 22 +++++++++++++++++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/addon-test-support/@ember/test-helpers/setup-context.ts b/addon-test-support/@ember/test-helpers/setup-context.ts index 5ff4c5d1b..87e88ac40 100644 --- a/addon-test-support/@ember/test-helpers/setup-context.ts +++ b/addon-test-support/@ember/test-helpers/setup-context.ts @@ -464,14 +464,20 @@ export default function setupContext( '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); }); diff --git a/tests/unit/setup-context-test.js b/tests/unit/setup-context-test.js index 291756029..76f05bab1 100644 --- a/tests/unit/setup-context-test.js +++ b/tests/unit/setup-context-test.js @@ -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');