From 61fb35319746f8f87ca4262807dfe4f311bb266d Mon Sep 17 00:00:00 2001 From: Steve Gravrock Date: Sat, 22 May 2021 09:01:55 -0700 Subject: [PATCH] Deprecate access to Suite objects via `this` in describes Jasmine 1.x exposed Suite objects to user code as the `this` in describe functions. That should have been removed in 2.0 but it was missed. It will be removed in 4.0. This change adds a deprecation warning if anything on a describe's `this` is accessed. The deprecation warning relies on Proxy, and won't work in environments that don't have it. Among Jasmine's supported environments, that's Safari 9, Safari 8, and all versions of IE. In those browsers, a describe's `this` will still be a Suite for now, but there will be no deprecation warnings. --- lib/jasmine-core/jasmine.js | 36 +++++++++++++++++++++++++- spec/core/EnvSpec.js | 43 ++++++++++++++++++++++++++++++++ spec/helpers/checkForProxy.js | 17 +++++++++++++ spec/support/jasmine-browser.js | 1 + spec/support/jasmine.json | 1 + src/core/Env.js | 2 +- src/core/deprecatingThisProxy.js | 32 ++++++++++++++++++++++++ src/core/requireCore.js | 1 + 8 files changed, 131 insertions(+), 2 deletions(-) create mode 100644 spec/helpers/checkForProxy.js create mode 100644 src/core/deprecatingThisProxy.js diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js index c5cae4f0..15b3193c 100644 --- a/lib/jasmine-core/jasmine.js +++ b/lib/jasmine-core/jasmine.js @@ -65,6 +65,7 @@ var getJasmineRequireObj = (function(jasmineGlobal) { j$.Clock = jRequire.Clock(); j$.DelayedFunctionScheduler = jRequire.DelayedFunctionScheduler(j$); j$.Env = jRequire.Env(j$); + j$.deprecatingThisProxy = jRequire.deprecatingThisProxy(j$); j$.StackTrace = jRequire.StackTrace(j$); j$.ExceptionFormatter = jRequire.ExceptionFormatter(j$); j$.ExpectationFilterChain = jRequire.ExpectationFilterChain(); @@ -2139,7 +2140,7 @@ getJasmineRequireObj().Env = function(j$) { var declarationError = null; try { - specDefinitions.call(suite); + specDefinitions.call(j$.deprecatingThisProxy(suite, self)); } catch (e) { declarationError = e; } @@ -3673,6 +3674,39 @@ getJasmineRequireObj().DelayedFunctionScheduler = function(j$) { return DelayedFunctionScheduler; }; +/* eslint-disable compat/compat */ +// TODO: Remove this in the next major release. +getJasmineRequireObj().deprecatingThisProxy = function(j$) { + var msg = "Access to 'this' in describe functions is deprecated."; + + try { + new Proxy({}, {}); + } catch (e) { + // Environment does not support Poxy. + return function(suite) { + return suite; + }; + } + + function DeprecatingThisProxyHandler(env) { + this._env = env; + } + + DeprecatingThisProxyHandler.prototype.get = function(target, prop, receiver) { + this._env.deprecated(msg); + return target[prop]; + }; + + DeprecatingThisProxyHandler.prototype.set = function(target, prop, value) { + this._env.deprecated(msg); + return (target[prop] = value); + }; + + return function(suite, env) { + return new Proxy(suite, new DeprecatingThisProxyHandler(env)); + }; +}; + getJasmineRequireObj().errors = function() { function ExpectationFailed() {} diff --git a/spec/core/EnvSpec.js b/spec/core/EnvSpec.js index 53b7dc05..20c3c17e 100644 --- a/spec/core/EnvSpec.js +++ b/spec/core/EnvSpec.js @@ -445,4 +445,47 @@ describe('Env', function() { done(); }); }); + + it("deprecates access to 'this' in describes", function() { + jasmine.getEnv().requireProxy(); + var msg = "Access to 'this' in describe functions is deprecated.", + ran = false; + spyOn(env, 'deprecated'); + + env.describe('a suite', function() { + expect(this.description).toEqual('a suite'); + expect(env.deprecated).toHaveBeenCalledWith(msg); + env.deprecated.calls.reset(); + + this.foo = 1; + expect(env.deprecated).toHaveBeenCalledWith(msg); + expect(this.foo).toEqual(1); + env.deprecated.calls.reset(); + + expect(this.getFullName()).toEqual('a suite'); + expect(env.deprecated).toHaveBeenCalledWith(msg); + env.deprecated.calls.reset(); + + env.it('has a spec'); + ran = true; + }); + + expect(ran).toBeTrue(); + }); + + // TODO: Remove this in the next major version. Suites were never meant to be + // exposed via describe 'this' in >= 2.0, and user code should not rely on it. + // This spec is just here to make sure we don't break user code that *does* + // rely on it in older browsers (without Proxy) while deprecating it. + it("sets 'this' to the Suite in describes", function() { + var suiteThis; + spyOn(env, 'deprecated'); + + env.describe('a suite', function() { + suiteThis = this; + env.it('has a spec'); + }); + + expect(suiteThis).toBeInstanceOf(jasmineUnderTest.Suite); + }); }); diff --git a/spec/helpers/checkForProxy.js b/spec/helpers/checkForProxy.js new file mode 100644 index 00000000..b4062d0b --- /dev/null +++ b/spec/helpers/checkForProxy.js @@ -0,0 +1,17 @@ +/* eslint-disable compat/compat */ +(function(env) { + function hasProxyConstructor() { + try { + new Proxy({}, {}); + return true; + } catch (e) { + return false; + } + } + + env.requireProxy = function() { + if (!hasProxyConstructor()) { + env.pending('Environment does not support Proxy'); + } + }; +})(jasmine.getEnv()); diff --git a/spec/support/jasmine-browser.js b/spec/support/jasmine-browser.js index b471ee2b..57d966e0 100644 --- a/spec/support/jasmine-browser.js +++ b/spec/support/jasmine-browser.js @@ -22,6 +22,7 @@ module.exports = { 'helpers/BrowserFlags.js', 'helpers/checkForArrayBuffer.js', 'helpers/checkForMap.js', + 'helpers/checkForProxy.js', 'helpers/checkForSet.js', 'helpers/checkForSymbol.js', 'helpers/checkForTypedArrays.js', diff --git a/spec/support/jasmine.json b/spec/support/jasmine.json index 68878471..b7775f2b 100644 --- a/spec/support/jasmine.json +++ b/spec/support/jasmine.json @@ -9,6 +9,7 @@ "helpers/generator.js", "helpers/checkForArrayBuffer.js", "helpers/checkForMap.js", + "helpers/checkForProxy.js", "helpers/checkForSet.js", "helpers/checkForSymbol.js", "helpers/checkForTypedArrays.js", diff --git a/src/core/Env.js b/src/core/Env.js index 1d950402..c3985a28 100644 --- a/src/core/Env.js +++ b/src/core/Env.js @@ -1137,7 +1137,7 @@ getJasmineRequireObj().Env = function(j$) { var declarationError = null; try { - specDefinitions.call(suite); + specDefinitions.call(j$.deprecatingThisProxy(suite, self)); } catch (e) { declarationError = e; } diff --git a/src/core/deprecatingThisProxy.js b/src/core/deprecatingThisProxy.js new file mode 100644 index 00000000..6f924b18 --- /dev/null +++ b/src/core/deprecatingThisProxy.js @@ -0,0 +1,32 @@ +/* eslint-disable compat/compat */ +// TODO: Remove this in the next major release. +getJasmineRequireObj().deprecatingThisProxy = function(j$) { + var msg = "Access to 'this' in describe functions is deprecated."; + + try { + new Proxy({}, {}); + } catch (e) { + // Environment does not support Poxy. + return function(suite) { + return suite; + }; + } + + function DeprecatingThisProxyHandler(env) { + this._env = env; + } + + DeprecatingThisProxyHandler.prototype.get = function(target, prop, receiver) { + this._env.deprecated(msg); + return target[prop]; + }; + + DeprecatingThisProxyHandler.prototype.set = function(target, prop, value) { + this._env.deprecated(msg); + return (target[prop] = value); + }; + + return function(suite, env) { + return new Proxy(suite, new DeprecatingThisProxyHandler(env)); + }; +}; diff --git a/src/core/requireCore.js b/src/core/requireCore.js index d5ea06a6..b2faf0fe 100644 --- a/src/core/requireCore.js +++ b/src/core/requireCore.js @@ -43,6 +43,7 @@ var getJasmineRequireObj = (function(jasmineGlobal) { j$.Clock = jRequire.Clock(); j$.DelayedFunctionScheduler = jRequire.DelayedFunctionScheduler(j$); j$.Env = jRequire.Env(j$); + j$.deprecatingThisProxy = jRequire.deprecatingThisProxy(j$); j$.StackTrace = jRequire.StackTrace(j$); j$.ExceptionFormatter = jRequire.ExceptionFormatter(j$); j$.ExpectationFilterChain = jRequire.ExpectationFilterChain();