Skip to content

Commit 8b06707

Browse files
jzaeffererjzaefferer
committed
See if we can replace deep cloning with something more sane. Keeps
semantics mostly the same, but without cloning anything but plain objects, e.g. not cloning of arrays
1 parent 60d4e0a commit 8b06707

File tree

3 files changed

+138
-6
lines changed

3 files changed

+138
-6
lines changed

‎tests/unit/widget/extend2.js

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
test("jQuery.extend(Object, Object)", function() {
2+
expect(28);
3+
4+
var settings = { xnumber1: 5, xnumber2: 7, xstring1: "peter", xstring2: "pan" },
5+
options = { xnumber2: 1, xstring2: "x", xxx: "newstring" },
6+
optionsCopy = { xnumber2: 1, xstring2: "x", xxx: "newstring" },
7+
merged = { xnumber1: 5, xnumber2: 1, xstring1: "peter", xstring2: "x", xxx: "newstring" },
8+
deep1 = { foo: { bar: true } },
9+
deep1copy = { foo: { bar: true } },
10+
deep2 = { foo: { baz: true }, foo2: document },
11+
deep2copy = { foo: { baz: true }, foo2: document },
12+
deepmerged = { foo: { bar: true, baz: true }, foo2: document },
13+
arr = [1, 2, 3],
14+
nestedarray = { arr: arr };
15+
16+
jQuery.extend2(settings, options);
17+
same( settings, merged, "Check if extended: settings must be extended" );
18+
same( options, optionsCopy, "Check if not modified: options must not be modified" );
19+
jQuery.extend2(settings, null, options);
20+
same( settings, merged, "Check if extended: settings must be extended" );
21+
same( options, optionsCopy, "Check if not modified: options must not be modified" );
22+
23+
jQuery.extend2(deep1, deep2);
24+
same( deep1.foo, deepmerged.foo, "Check if foo: settings must be extended" );
25+
same( deep2.foo, deep2copy.foo, "Check if not deep2: options must not be modified" );
26+
equals( deep1.foo2, document, "Make sure that a deep clone was not attempted on the document" );
27+
28+
ok( jQuery.extend2({}, nestedarray).arr === arr, "Don't clone arrays" );
29+
ok( jQuery.isPlainObject( jQuery.extend2({ arr: arr }, { arr: {} }).arr ), "Cloned object heve to be an plain object" );
30+
31+
var empty = {};
32+
var optionsWithLength = { foo: { length: -1 } };
33+
jQuery.extend2(empty, optionsWithLength);
34+
same( empty.foo, optionsWithLength.foo, "The length property must copy correctly" );
35+
36+
empty = {};
37+
var optionsWithDate = { foo: { date: new Date } };
38+
jQuery.extend2(empty, optionsWithDate);
39+
same( empty.foo, optionsWithDate.foo, "Dates copy correctly" );
40+
41+
var myKlass = function() {};
42+
var customObject = new myKlass();
43+
var optionsWithCustomObject = { foo: { date: customObject } };
44+
empty = {};
45+
jQuery.extend2(empty, optionsWithCustomObject);
46+
ok( empty.foo && empty.foo.date === customObject, "Custom objects copy correctly (no methods)" );
47+
48+
// Makes the class a little more realistic
49+
myKlass.prototype = { someMethod: function(){} };
50+
empty = {};
51+
jQuery.extend2(empty, optionsWithCustomObject);
52+
ok( empty.foo && empty.foo.date === customObject, "Custom objects copy correctly" );
53+
54+
var ret = jQuery.extend2({ foo: 4 }, { foo: new Number(5) } );
55+
ok( ret.foo == 5, "Wrapped numbers copy correctly" );
56+
57+
var nullUndef;
58+
nullUndef = jQuery.extend2({}, options, { xnumber2: null });
59+
ok( nullUndef.xnumber2 === null, "Check to make sure null values are copied");
60+
61+
nullUndef = jQuery.extend2({}, options, { xnumber2: undefined });
62+
ok( nullUndef.xnumber2 === options.xnumber2, "Check to make sure undefined values are not copied");
63+
64+
nullUndef = jQuery.extend2({}, options, { xnumber0: null });
65+
ok( nullUndef.xnumber0 === null, "Check to make sure null values are inserted");
66+
67+
// TODO weird test
68+
/*
69+
var target = {};
70+
var recursive = { foo:target, bar:5 };
71+
jQuery.extend2(target, recursive);
72+
same( target, { bar:5 }, "Check to make sure a recursive obj doesn't go never-ending loop by not copying it over" );
73+
*/
74+
75+
var ret = jQuery.extend(true, { foo: [] }, { foo: [0] } ); // 1907
76+
equals( ret.foo.length, 1, "Check to make sure a value with coersion 'false' copies over when necessary to fix #1907" );
77+
78+
var ret = jQuery.extend(true, { foo: "1,2,3" }, { foo: [1, 2, 3] } );
79+
ok( typeof ret.foo != "string", "Check to make sure values equal with coersion (but not actually equal) overwrite correctly" );
80+
81+
var ret = jQuery.extend(true, { foo:"bar" }, { foo:null } );
82+
ok( typeof ret.foo !== "undefined", "Make sure a null value doesn't crash with deep extend, for #1908" );
83+
84+
var obj = { foo:null };
85+
jQuery.extend(true, obj, { foo:"notnull" } );
86+
equals( obj.foo, "notnull", "Make sure a null value can be overwritten" );
87+
88+
function func() {}
89+
jQuery.extend(func, { key: "value" } );
90+
equals( func.key, "value", "Verify a function can be extended" );
91+
92+
var defaults = { xnumber1: 5, xnumber2: 7, xstring1: "peter", xstring2: "pan" },
93+
defaultsCopy = { xnumber1: 5, xnumber2: 7, xstring1: "peter", xstring2: "pan" },
94+
options1 = { xnumber2: 1, xstring2: "x" },
95+
options1Copy = { xnumber2: 1, xstring2: "x" },
96+
options2 = { xstring2: "xx", xxx: "newstringx" },
97+
options2Copy = { xstring2: "xx", xxx: "newstringx" },
98+
merged2 = { xnumber1: 5, xnumber2: 1, xstring1: "peter", xstring2: "xx", xxx: "newstringx" };
99+
100+
var settings = jQuery.extend({}, defaults, options1, options2);
101+
same( settings, merged2, "Check if extended: settings must be extended" );
102+
same( defaults, defaultsCopy, "Check if not modified: options1 must not be modified" );
103+
same( options1, options1Copy, "Check if not modified: options1 must not be modified" );
104+
same( options2, options2Copy, "Check if not modified: options2 must not be modified" );
105+
106+
var input = {
107+
key: [ 1, 2, 3 ]
108+
}
109+
var output = jQuery.extend2( {}, input );
110+
deepEqual( input, output, "don't clone arrays" );
111+
input.key[0] = 10;
112+
deepEqual( input, output, "don't clone arrays" );
113+
});

‎tests/unit/widget/widget.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
<script src="../testsuite.js"></script>
1515

1616
<script src="widget_core.js"></script>
17+
<script src="extend2.js"></script>
1718

1819
<script src="../swarminject.js"></script>
1920
</head>

‎ui/jquery.ui.widget.js

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,24 @@
1111

1212
var slice = Array.prototype.slice;
1313

14+
function extend( target ) {
15+
var input = slice.call( arguments, 1 ),
16+
inputIndex = 0,
17+
inputLength = input.length,
18+
key,
19+
value;
20+
for ( ; inputIndex < inputLength; inputIndex++ ) {
21+
for ( key in input[ inputIndex ] ) {
22+
value = input[ inputIndex ][ key ];
23+
if (input[ inputIndex ].hasOwnProperty( key ) && value !== undefined ) {
24+
target[ key ] = $.isPlainObject( value ) ? extend( {}, target[ key ], value ) : value;
25+
}
26+
}
27+
}
28+
return target;
29+
}
30+
$.extend2 = extend;
31+
1432
var _cleanData = $.cleanData;
1533
$.cleanData = function( elems ) {
1634
for ( var i = 0, elem; (elem = elems[i]) != null; i++ ) {
@@ -55,7 +73,7 @@ $.widget = function( name, base, prototype ) {
5573
// we need to make the options hash a property directly on the new instance
5674
// otherwise we'll modify the options hash on the prototype that we're
5775
// inheriting from
58-
basePrototype.options = $.extend( true, {}, basePrototype.options );
76+
basePrototype.options = extend( {}, basePrototype.options );
5977
$.each( prototype, function( prop, value ) {
6078
if ( $.isFunction( value ) ) {
6179
prototype[ prop ] = (function() {
@@ -83,7 +101,7 @@ $.widget = function( name, base, prototype ) {
83101
}());
84102
}
85103
});
86-
$[ namespace ][ name ].prototype = $.extend( true, basePrototype, {
104+
$[ namespace ][ name ].prototype = extend( basePrototype, {
87105
namespace: namespace,
88106
widgetName: name,
89107
widgetEventPrefix: name,
@@ -101,7 +119,7 @@ $.widget.bridge = function( name, object ) {
101119

102120
// allow multiple hashes to be passed on init
103121
options = !isMethodCall && args.length ?
104-
$.extend.apply( null, [ true, options ].concat(args) ) :
122+
extend.apply( null, [ options ].concat(args) ) :
105123
options;
106124

107125
if ( isMethodCall ) {
@@ -163,7 +181,7 @@ $.Widget.prototype = {
163181
_createWidget: function( options, element ) {
164182
element = $( element || this.defaultElement || this )[ 0 ];
165183
this.element = $( element );
166-
this.options = $.extend( true, {},
184+
this.options = extend( {},
167185
this.options,
168186
this._getCreateOptions(),
169187
options );
@@ -218,7 +236,7 @@ $.Widget.prototype = {
218236

219237
if ( arguments.length === 0 ) {
220238
// don't return a reference to the internal hash
221-
return $.extend( {}, this.options );
239+
return extend( {}, this.options );
222240
}
223241

224242
if ( typeof key === "string" ) {
@@ -230,7 +248,7 @@ $.Widget.prototype = {
230248
parts = key.split( "." );
231249
key = parts.shift();
232250
if ( parts.length ) {
233-
curOption = options[ key ] = $.extend( true, {}, this.options[ key ] );
251+
curOption = options[ key ] = extend( {}, this.options[ key ] );
234252
for ( i = 0; i < parts.length - 1; i++ ) {
235253
curOption[ parts[ i ] ] = curOption[ parts[ i ] ] || {};
236254
curOption = curOption[ parts[ i ] ];

0 commit comments

Comments
 (0)