diff --git a/src/jquery/core.js b/src/jquery/core.js index d9cb256a..d11403b7 100644 --- a/src/jquery/core.js +++ b/src/jquery/core.js @@ -1,4 +1,4 @@ -import { migrateWarnProp, migratePatchAndWarnFunc, migratePatchAndInfoFunc } from "../main.js"; +import { migratePatchAndWarnFunc, migratePatchAndInfoFunc } from "../main.js"; import "../disablePatches.js"; var arr = [], @@ -92,9 +92,9 @@ migratePatchAndWarnFunc( jQuery, "isWindow", migratePatchAndInfoFunc( jQuery, "proxy", jQuery.proxy, "proxy", "jQuery.proxy() is deprecated" ); -migrateWarnProp( jQuery.fn, "push", push, "push", +migratePatchAndWarnFunc( jQuery.fn, "push", push, "push", "jQuery.fn.push() is removed; use .add() or convert to an array" ); -migrateWarnProp( jQuery.fn, "sort", sort, "sort", +migratePatchAndWarnFunc( jQuery.fn, "sort", sort, "sort", "jQuery.fn.sort() is removed; convert to an array before sorting" ); -migrateWarnProp( jQuery.fn, "splice", splice, "splice", +migratePatchAndWarnFunc( jQuery.fn, "splice", splice, "splice", "jQuery.fn.splice() is removed; use .slice() or .not() with .eq()" ); diff --git a/src/main.js b/src/main.js index 849bef56..dfad6f67 100644 --- a/src/main.js +++ b/src/main.js @@ -70,38 +70,71 @@ export function migrateInfo( code, msg ) { migrateMessageInternal( code, msg, "info" ); } -function migrateMessagePropInternal( +function migratePatchPropInternal( obj, prop, value, code, msg, migrateMessageFn ) { + var orig = obj[ prop ]; Object.defineProperty( obj, prop, { configurable: true, enumerable: true, + get: function() { - migrateMessageFn( code, msg ); - return value; + if ( jQuery.migrateIsPatchEnabled( code ) ) { + + // If `msg` not provided, do not message; more sophisticated + // messaging logic is most likely embedded in `value`. + if ( msg ) { + migrateMessageFn( code, msg ); + } + + return value; + } + + return orig; }, + set: function( newValue ) { - migrateMessageFn( code, msg ); - value = newValue; + if ( jQuery.migrateIsPatchEnabled( code ) ) { + + // See the comment in the getter. + if ( msg ) { + migrateMessageFn( code, msg ); + } + } + + // If a new value was set, apply it regardless if + // the patch is later disabled or not. + orig = value = newValue; } } ); } export function migrateWarnProp( obj, prop, value, code, msg ) { - migrateMessagePropInternal( obj, prop, value, code, msg, migrateWarn ); + if ( !msg ) { + throw new Error( "No warning message provided" ); + } + migratePatchPropInternal( obj, prop, value, code, msg, migrateWarn ); } export function migrateInfoProp( obj, prop, value, code, msg ) { - migrateMessagePropInternal( obj, prop, value, code, msg, migrateInfo ); + if ( !msg ) { + throw new Error( "No warning message provided" ); + } + migratePatchPropInternal( obj, prop, value, code, msg, migrateInfo ); +} + +export function migratePatchProp( obj, prop, value, code ) { + migratePatchPropInternal( obj, prop, value, code ); } -function migrateMessageFuncInternal( +// The value of the "Func" APIs is that for method we want to allow +// checking for the method existence without triggering a warning. +// For other deprecated properties, we do need to warn on access. +function migratePatchFuncInternal( obj, prop, newFunc, code, msg, migrateMessageFn ) { - var finalFunc, - origFunc = obj[ prop ]; - obj[ prop ] = function() { + function wrappedNewFunc() { // If `msg` not provided, do not warn; more sophisticated warnings // logic is most likely embedded in `newFunc`, in that case here @@ -111,34 +144,28 @@ function migrateMessageFuncInternal( migrateMessageFn( code, msg ); } - // Since patches can be disabled & enabled dynamically, we - // need to decide which implementation to run on each invocation. - finalFunc = jQuery.migrateIsPatchEnabled( code ) ? - newFunc : - - // The function may not have existed originally so we need a fallback. - ( origFunc || jQuery.noop ); + return newFunc.apply( this, arguments ); + } - return finalFunc.apply( this, arguments ); - }; + migratePatchPropInternal( obj, prop, wrappedNewFunc, code ); } export function migratePatchAndWarnFunc( obj, prop, newFunc, code, msg ) { if ( !msg ) { throw new Error( "No warning message provided" ); } - return migrateMessageFuncInternal( obj, prop, newFunc, code, msg, migrateWarn ); + return migratePatchFuncInternal( obj, prop, newFunc, code, msg, migrateWarn ); } export function migratePatchAndInfoFunc( obj, prop, newFunc, code, msg ) { if ( !msg ) { throw new Error( "No info message provided" ); } - return migrateMessageFuncInternal( obj, prop, newFunc, code, msg, migrateInfo ); + return migratePatchFuncInternal( obj, prop, newFunc, code, msg, migrateInfo ); } export function migratePatchFunc( obj, prop, newFunc, code ) { - return migrateMessageFuncInternal( obj, prop, newFunc, code ); + return migratePatchFuncInternal( obj, prop, newFunc, code ); } if ( window.document.compatMode === "BackCompat" ) { diff --git a/test/unit/migrate.js b/test/unit/migrate.js index e3c2ffb7..a70ba9a2 100644 --- a/test/unit/migrate.js +++ b/test/unit/migrate.js @@ -46,7 +46,7 @@ QUnit.test( "jQuery.migrateDeduplicateMessages", function( assert ) { } ); QUnit.test( "disabling/enabling patches", function( assert ) { - assert.expect( 14 ); + assert.expect( 27 ); var elem = jQuery( "
" ); @@ -62,6 +62,10 @@ QUnit.test( "disabling/enabling patches", function( assert ) { true, "patch enabled by default (proxy)" ); assert.strictEqual( jQuery.migrateIsPatchEnabled( "shorthand-deprecated-v3" ), true, "patch enabled by default (shorthand-deprecated-v3)" ); + assert.strictEqual( jQuery.migrateIsPatchEnabled( "push" ), + true, "patch enabled by default (push)" ); + assert.strictEqual( jQuery.migrateIsPatchEnabled( "isArray" ), + true, "patch enabled by default (isArray)" ); expectMessage( assert, "pre-on-methods (default)", function() { jQuery().bind(); @@ -72,24 +76,47 @@ QUnit.test( "disabling/enabling patches", function( assert ) { expectMessage( assert, "shorthand-deprecated-v3 (default)", function() { jQuery().click(); } ); + expectMessage( assert, "push (default)", function() { + jQuery().push(); + } ); + expectMessage( assert, "isArray (default)", function() { + jQuery.isArray(); + } ); + + expectNoMessage( assert, "push access without calling (default)", function() { + assert.strictEqual( typeof jQuery().push, "function", + "access check doesn't trigger a message (push)" ); + } ); + expectNoMessage( assert, "isArray access without calling (default)", function() { + assert.strictEqual( typeof jQuery.isArray, "function", + "access check doesn't trigger a message (isArray)" ); + } ); - jQuery.migrateDisablePatches( "pre-on-methods", "proxy" ); + jQuery.migrateDisablePatches( "pre-on-methods", "proxy", "push", "isArray" ); assert.strictEqual( jQuery.migrateIsPatchEnabled( "pre-on-methods" ), false, "patch disabled (pre-on-methods)" ); assert.strictEqual( jQuery.migrateIsPatchEnabled( "proxy" ), false, "patch disabled (proxy)" ); assert.strictEqual( jQuery.migrateIsPatchEnabled( "shorthand-deprecated-v3" ), true, "patch still enabled (shorthand-deprecated-v3)" ); + assert.strictEqual( jQuery.migrateIsPatchEnabled( "push" ), + false, "patch disabled (push)" ); - expectNoMessage( assert, "pre-on-methods (default)", function() { + expectNoMessage( assert, "pre-on-methods (disabled)", function() { jQuery().bind(); } ); - expectNoMessage( assert, "proxy (default)", function() { + expectNoMessage( assert, "proxy (disabled)", function() { jQuery.proxy( jQuery.noop ); } ); - expectMessage( assert, "shorthand-deprecated-v3 (default)", function() { + expectMessage( assert, "shorthand-deprecated-v3 (not disabled)", function() { jQuery().click(); } ); + expectNoMessage( assert, "push (disabled)", function() { + assert.strictEqual( jQuery().push, undefined, "`push` patch no longer defined" ); + } ); + expectNoMessage( assert, "isArray (disabled)", function() { + assert.strictEqual( jQuery.isArray, undefined, "`jQuery.isArray` patch no longer defined" ); + } ); jQuery.migrateDisablePatches( "shorthand-deprecated-v3" ); assert.strictEqual( jQuery.migrateIsPatchEnabled( "shorthand-deprecated-v3" ),