From e4e232864d3b4c99ec1a44c17c4387534cafa25f Mon Sep 17 00:00:00 2001 From: Steve Gravrock Date: Fri, 21 May 2021 20:58:20 -0700 Subject: [PATCH] Don't expose Suite objects as `this` of describe functions This was a holdover from 1.x that should have been removed in 2.0, but was missed. Suite is meant to be private, and almost none of its methods can be safely called by user code. --- lib/jasmine-core/jasmine.js | 36 +------------------------------- spec/core/EnvSpec.js | 35 ++----------------------------- 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, 4 insertions(+), 121 deletions(-) delete mode 100644 spec/helpers/checkForProxy.js delete mode 100644 src/core/deprecatingThisProxy.js diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js index 25e70185..1e4c891c 100644 --- a/lib/jasmine-core/jasmine.js +++ b/lib/jasmine-core/jasmine.js @@ -65,7 +65,6 @@ 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(); @@ -1957,7 +1956,7 @@ getJasmineRequireObj().Env = function(j$) { var declarationError = null; try { - specDefinitions.call(j$.deprecatingThisProxy(suite, self)); + specDefinitions(); } catch (e) { declarationError = e; } @@ -3368,39 +3367,6 @@ 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 12000f34..23c0d373 100644 --- a/spec/core/EnvSpec.js +++ b/spec/core/EnvSpec.js @@ -441,38 +441,7 @@ describe('Env', function() { }); }); - 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() { + it("does not expose the suite as 'this'", function() { var suiteThis; spyOn(env, 'deprecated'); @@ -481,6 +450,6 @@ describe('Env', function() { env.it('has a spec'); }); - expect(suiteThis).toBeInstanceOf(jasmineUnderTest.Suite); + expect(suiteThis).not.toBeInstanceOf(jasmineUnderTest.Suite); }); }); diff --git a/spec/helpers/checkForProxy.js b/spec/helpers/checkForProxy.js deleted file mode 100644 index b4062d0b..00000000 --- a/spec/helpers/checkForProxy.js +++ /dev/null @@ -1,17 +0,0 @@ -/* 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 1fc16d01..9062bf8a 100644 --- a/spec/support/jasmine-browser.js +++ b/spec/support/jasmine-browser.js @@ -21,7 +21,6 @@ module.exports = { 'helpers/generator.js', 'helpers/BrowserFlags.js', 'helpers/checkForMap.js', - 'helpers/checkForProxy.js', 'helpers/checkForSet.js', 'helpers/checkForSymbol.js', 'helpers/checkForUrl.js', diff --git a/spec/support/jasmine.json b/spec/support/jasmine.json index d6369bf2..1a0ca3b2 100644 --- a/spec/support/jasmine.json +++ b/spec/support/jasmine.json @@ -8,7 +8,6 @@ "helpers/asyncAwait.js", "helpers/generator.js", "helpers/checkForMap.js", - "helpers/checkForProxy.js", "helpers/checkForSet.js", "helpers/checkForSymbol.js", "helpers/checkForUrl.js", diff --git a/src/core/Env.js b/src/core/Env.js index 813c4843..39c8cf81 100644 --- a/src/core/Env.js +++ b/src/core/Env.js @@ -987,7 +987,7 @@ getJasmineRequireObj().Env = function(j$) { var declarationError = null; try { - specDefinitions.call(j$.deprecatingThisProxy(suite, self)); + specDefinitions(); } catch (e) { declarationError = e; } diff --git a/src/core/deprecatingThisProxy.js b/src/core/deprecatingThisProxy.js deleted file mode 100644 index 6f924b18..00000000 --- a/src/core/deprecatingThisProxy.js +++ /dev/null @@ -1,32 +0,0 @@ -/* 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 13cc6cc5..aac94b87 100644 --- a/src/core/requireCore.js +++ b/src/core/requireCore.js @@ -43,7 +43,6 @@ 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();