Data: Patch $._data as well

Also, extract the util to patch a prototype as we'll need it in the event
module as well.

Closes gh-569
Ref gh-570

Author: mgol

src/jquery/data.js

@@ -1,86 +1,45 @@
-import { migratePatchFunc, migrateWarn } from "../main.js";
+import { migratePatchFunc } from "../main.js";
+import { patchProto } from "../utils.js";
 
 function patchDataProto( original, options ) {
-	var i,
+	var warningId = options.warningId,
 		apiName = options.apiName,
-		isInstanceMethod = options.isInstanceMethod,
+		isInstanceMethod = options.isInstanceMethod;
 
-		// `Object.prototype` keys are not enumerable so list the
-		// official ones here. An alternative would be wrapping
-		// data objects with a Proxy but that creates additional issues
-		// like breaking object identity on subsequent calls.
-		objProtoKeys = [
-			"__proto__",
-			"__defineGetter__",
-			"__defineSetter__",
-			"__lookupGetter__",
-			"__lookupSetter__",
-			"hasOwnProperty",
-			"isPrototypeOf",
-			"propertyIsEnumerable",
-			"toLocaleString",
-			"toString",
-			"valueOf"
-		],
-
-		// Use a null prototype at the beginning so that we can define our
-		// `__proto__` getter & setter. We'll reset the prototype afterwards.
-		intermediateDataObj = Object.create( null );
-
-	for ( i = 0; i < objProtoKeys.length; i++ ) {
-		( function( key ) {
-			Object.defineProperty( intermediateDataObj, key, {
-				get: function() {
-					migrateWarn( "data-null-proto",
-						"Accessing properties from " + apiName +
-						" inherited from Object.prototype is removed" );
-					return ( key + "__cache" ) in intermediateDataObj ?
-						intermediateDataObj[ key + "__cache" ] :
-						Object.prototype[ key ];
-				},
-				set: function( value ) {
-					migrateWarn( "data-null-proto",
-						"Setting properties from " + apiName +
-						" inherited from Object.prototype is removed" );
-					intermediateDataObj[ key + "__cache" ] = value;
-				}
-			} );
-		} )( objProtoKeys[ i ] );
-	}
-
-	Object.setPrototypeOf( intermediateDataObj, Object.prototype );
-
-	return function jQueryDataProtoPatched() {
+	return function apiWithProtoPatched() {
 		var result = original.apply( this, arguments );
 
 		if ( arguments.length !== ( isInstanceMethod ? 0 : 1 ) || result === undefined ) {
 			return result;
 		}
 
-		// Insert an additional object in the prototype chain between `result`
-		// and `Object.prototype`; that intermediate object proxies properties
-		// to `Object.prototype`, warning about their usage first.
-		Object.setPrototypeOf( result, intermediateDataObj );
+		patchProto( result, {
+			warningId: warningId,
+			apiName: apiName
+		} );
 
 		return result;
 	};
 }
 
-// Yes, we are patching jQuery.data twice; here & above. This is necessary
-// so that each of the two patches can be independently disabled.
 migratePatchFunc( jQuery, "data",
 	patchDataProto( jQuery.data, {
+		warningId: "data-null-proto",
 		apiName: "jQuery.data()",
-		isPrivateData: false,
+		isInstanceMethod: false
+	} ),
+	"data-null-proto" );
+migratePatchFunc( jQuery, "_data",
+	patchDataProto( jQuery._data, {
+		warningId: "data-null-proto",
+		apiName: "jQuery._data()",
 		isInstanceMethod: false
 	} ),
 	"data-null-proto" );
 migratePatchFunc( jQuery.fn, "data",
 	patchDataProto( jQuery.fn.data, {
+		warningId: "data-null-proto",
 		apiName: "jQuery.fn.data()",
-		isPrivateData: true,
 		isInstanceMethod: true
 	} ),
 	"data-null-proto" );
-
-// TODO entry in warnings.md

src/utils.js

@@ -1,5 +1,64 @@
+import { migrateWarn } from "./main.js";
+
 export function camelCase( string ) {
 	return string.replace( /-([a-z])/g, function( _, letter ) {
 		return letter.toUpperCase();
 	} );
 }
+
+// Make `object` inherit from `Object.prototype` via an additional object
+// in between; that intermediate object proxies properties
+// to `Object.prototype`, warning about their usage first.
+export function patchProto( object, options ) {
+	var i,
+		warningId = options.warningId,
+		apiName = options.apiName,
+
+		// `Object.prototype` keys are not enumerable so list the
+		// official ones here. An alternative would be wrapping
+		// objects with a Proxy but that creates additional issues
+		// like breaking object identity on subsequent calls.
+		objProtoKeys = [
+			"__proto__",
+			"__defineGetter__",
+			"__defineSetter__",
+			"__lookupGetter__",
+			"__lookupSetter__",
+			"hasOwnProperty",
+			"isPrototypeOf",
+			"propertyIsEnumerable",
+			"toLocaleString",
+			"toString",
+			"valueOf"
+		],
+
+		// Use a null prototype at the beginning so that we can define our
+		// `__proto__` getter & setter. We'll reset the prototype afterward.
+		intermediateObj = Object.create( null );
+
+	for ( i = 0; i < objProtoKeys.length; i++ ) {
+		( function( key ) {
+			Object.defineProperty( intermediateObj, key, {
+				get: function() {
+					migrateWarn( warningId,
+						"Accessing properties from " + apiName +
+						" inherited from Object.prototype is removed" );
+					return ( key + "__cache" ) in intermediateObj ?
+						intermediateObj[ key + "__cache" ] :
+						Object.prototype[ key ];
+				},
+				set: function( value ) {
+					migrateWarn( warningId,
+						"Setting properties from " + apiName +
+						" inherited from Object.prototype is removed" );
+					intermediateObj[ key + "__cache" ] = value;
+				}
+			} );
+		} )( objProtoKeys[ i ] );
+	}
+
+	Object.setPrototypeOf( intermediateObj, Object.prototype );
+	Object.setPrototypeOf( object, intermediateObj );
+
+	return object;
+}

test/unit/jquery/data.js

@@ -1,21 +1,25 @@
 QUnit.module( "data" );
 
 QUnit.test( "properties from Object.prototype", function( assert ) {
-	assert.expect( 6 );
+	assert.expect( 8 );
 
 	var div = jQuery( "<div>" ).appendTo( "#qunit-fixture" );
 
 	div.data( "foo", "bar" );
+	jQuery._data( div[ 0 ], "baz", "qaz" );
 
 	expectNoMessage( assert, "Regular properties", function() {
 		assert.strictEqual( div.data( "foo" ), "bar", "data access" );
 		assert.strictEqual( jQuery.data( div[ 0 ], "foo" ), "bar", "data access (static method)" );
+		assert.strictEqual( jQuery._data( div[ 0 ], "baz" ), "qaz", "private data access" );
 	} );
 
-	expectMessage( assert, "Properties from Object.prototype", 2, function() {
+	expectMessage( assert, "Properties from Object.prototype", 3, function() {
 		assert.ok( div.data().hasOwnProperty( "foo" ),
 			"hasOwnProperty works" );
 		assert.ok( jQuery.data( div[ 0 ] ).hasOwnProperty( "foo" ),
 			"hasOwnProperty works (static method)" );
+		assert.ok( jQuery._data( div[ 0 ] ).hasOwnProperty( "baz" ),
+			"hasOwnProperty works (private data)" );
 	} );
 } );