Skip to content

Shallow options #193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

Shallow options #193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8b06707
See if we can replace deep cloning with something more sane. Keeps
jzaefferer Apr 27, 2011
6411cdf
Rename $.extend2 to $.widget.extend
jzaefferer May 2, 2011
fc5fab2
Widget: Clean up extend tests
jzaefferer May 7, 2011
684fae7
Wdiget: More cleanup on extend tests
jzaefferer May 10, 2011
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions tests/unit/widget/widget.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
<script src="../testsuite.js"></script>

<script src="widget_core.js"></script>
<script src="widget_extend.js"></script>

<script src="../swarminject.js"></script>
</head>
Expand Down
108 changes: 108 additions & 0 deletions tests/unit/widget/widget_extend.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
test("$.widget.extend(Object, Object)", function() {
expect(27);

var settings = { xnumber1: 5, xnumber2: 7, xstring1: "peter", xstring2: "pan" },
options = { xnumber2: 1, xstring2: "x", xxx: "newstring" },
optionsCopy = { xnumber2: 1, xstring2: "x", xxx: "newstring" },
merged = { xnumber1: 5, xnumber2: 1, xstring1: "peter", xstring2: "x", xxx: "newstring" },
deep1 = { foo: { bar: true } },
deep1copy = { foo: { bar: true } },
deep2 = { foo: { baz: true }, foo2: document },
deep2copy = { foo: { baz: true }, foo2: document },
deepmerged = { foo: { bar: true, baz: true }, foo2: document },
arr = [1, 2, 3],
nestedarray = { arr: arr },
ret;

$.widget.extend(settings, options);
deepEqual( settings, merged, "Check if extended: settings must be extended" );
deepEqual( options, optionsCopy, "Check if not modified: options must not be modified" );

$.widget.extend(deep1, deep2);
deepEqual( deep1.foo, deepmerged.foo, "Check if foo: settings must be extended" );
deepEqual( deep2.foo, deep2copy.foo, "Check if not deep2: options must not be modified" );
equal( deep1.foo2, document, "Make sure that a deep clone was not attempted on the document" );

strictEqual( $.widget.extend({}, nestedarray).arr, arr, "Don't clone arrays" );
ok( $.isPlainObject( $.widget.extend({ arr: arr }, { arr: {} }).arr ), "Cloned object heve to be an plain object" );

var empty = {};
var optionsWithLength = { foo: { length: -1 } };
$.widget.extend(empty, optionsWithLength);
deepEqual( empty.foo, optionsWithLength.foo, "The length property must copy correctly" );

empty = {};
var optionsWithDate = { foo: { date: new Date } };
$.widget.extend(empty, optionsWithDate);
deepEqual( empty.foo, optionsWithDate.foo, "Dates copy correctly" );

var myKlass = function() {};
var customObject = new myKlass();
var optionsWithCustomObject = { foo: { date: customObject } };
empty = {};
$.widget.extend(empty, optionsWithCustomObject);
strictEqual( empty.foo.date, customObject, "Custom objects copy correctly (no methods)" );

// Makes the class a little more realistic
myKlass.prototype = { someMethod: function(){} };
empty = {};
$.widget.extend(empty, optionsWithCustomObject);
strictEqual( empty.foo.date, customObject, "Custom objects copy correctly" );

ret = $.widget.extend({ foo: 4 }, { foo: new Number(5) } );
equal( ret.foo, 5, "Wrapped numbers copy correctly" );

var nullUndef;
nullUndef = $.widget.extend({}, options, { xnumber2: null });
strictEqual( nullUndef.xnumber2, null, "Check to make sure null values are copied");

nullUndef = $.widget.extend({}, options, { xnumber2: undefined });
strictEqual( nullUndef.xnumber2, options.xnumber2, "Check to make sure undefined values are not copied");

nullUndef = $.widget.extend({}, options, { xnumber0: null });
strictEqual( nullUndef.xnumber0, null, "Check to make sure null values are inserted");

var target = {};
var recursive = { foo:target, bar:5 };
$.widget.extend(target, recursive);
deepEqual( target, { foo: {}, bar: 5 }, "Check to make sure a recursive obj doesn't go never-ending loop by not copying it over" );

ret = $.widget.extend( { foo: [] }, { foo: [0] } ); // 1907
equal( ret.foo.length, 1, "Check to make sure a value with coersion 'false' copies over when necessary to fix #1907" );

ret = $.widget.extend( { foo: "1,2,3" }, { foo: [1, 2, 3] } );
strictEqual( typeof ret.foo, "object", "Check to make sure values equal with coersion (but not actually equal) overwrite correctly" );

ret = $.widget.extend( { foo:"bar" }, { foo:null } );
strictEqual( typeof ret.foo, "object", "Make sure a null value doesn't crash with deep extend, for #1908" );

var obj = { foo:null };
$.widget.extend( obj, { foo:"notnull" } );
equal( obj.foo, "notnull", "Make sure a null value can be overwritten" );

function func() {}
$.widget.extend(func, { key: "value" } );
equal( func.key, "value", "Verify a function can be extended" );

var defaults = { xnumber1: 5, xnumber2: 7, xstring1: "peter", xstring2: "pan" },
defaultsCopy = { xnumber1: 5, xnumber2: 7, xstring1: "peter", xstring2: "pan" },
options1 = { xnumber2: 1, xstring2: "x" },
options1Copy = { xnumber2: 1, xstring2: "x" },
options2 = { xstring2: "xx", xxx: "newstringx" },
options2Copy = { xstring2: "xx", xxx: "newstringx" },
merged2 = { xnumber1: 5, xnumber2: 1, xstring1: "peter", xstring2: "xx", xxx: "newstringx" };

var settings = $.widget.extend({}, defaults, options1, options2);
deepEqual( settings, merged2, "Check if extended: settings must be extended" );
deepEqual( defaults, defaultsCopy, "Check if not modified: options1 must not be modified" );
deepEqual( options1, options1Copy, "Check if not modified: options1 must not be modified" );
deepEqual( options2, options2Copy, "Check if not modified: options2 must not be modified" );

var input = {
key: [ 1, 2, 3 ]
}
var output = $.widget.extend( {}, input );
deepEqual( input, output, "don't clone arrays" );
input.key[0] = 10;
deepEqual( input, output, "don't clone arrays" );
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't array handling already tested on line 28?

31 changes: 24 additions & 7 deletions ui/jquery.ui.widget.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ $.widget = function( name, base, prototype ) {
$[ namespace ] = $[ namespace ] || {};
// create the constructor using $.extend() so we can carry over any
// static properties stored on the existing constructor (if there is one)
$[ namespace ][ name ] = $.extend( function( options, element ) {
$[ namespace ][ name ] = $.widget.extend( function( options, element ) {
// allow instantiation without "new" keyword
if ( !this._createWidget ) {
return new $[ namespace ][ name ]( options, element );
Expand All @@ -55,7 +55,7 @@ $.widget = function( name, base, prototype ) {
// we need to make the options hash a property directly on the new instance
// otherwise we'll modify the options hash on the prototype that we're
// inheriting from
basePrototype.options = $.extend( true, {}, basePrototype.options );
basePrototype.options = $.widget.extend( {}, basePrototype.options );
$.each( prototype, function( prop, value ) {
if ( $.isFunction( value ) ) {
prototype[ prop ] = (function() {
Expand Down Expand Up @@ -83,7 +83,7 @@ $.widget = function( name, base, prototype ) {
}());
}
});
$[ namespace ][ name ].prototype = $.extend( true, basePrototype, {
$[ namespace ][ name ].prototype = $.widget.extend( basePrototype, {
namespace: namespace,
widgetName: name,
widgetEventPrefix: name,
Expand All @@ -93,6 +93,23 @@ $.widget = function( name, base, prototype ) {
$.widget.bridge( name, $[ namespace ][ name ] );
};

$.widget.extend = function( target ) {
var input = slice.call( arguments, 1 ),
inputIndex = 0,
inputLength = input.length,
key,
value;
for ( ; inputIndex < inputLength; inputIndex++ ) {
for ( key in input[ inputIndex ] ) {
value = input[ inputIndex ][ key ];
if (input[ inputIndex ].hasOwnProperty( key ) && value !== undefined ) {
target[ key ] = $.isPlainObject( value ) ? $.widget.extend( {}, target[ key ], value ) : value;
}
}
}
return target;
};

$.widget.bridge = function( name, object ) {
$.fn[ name ] = function( options ) {
var isMethodCall = typeof options === "string",
Expand All @@ -101,7 +118,7 @@ $.widget.bridge = function( name, object ) {

// allow multiple hashes to be passed on init
options = !isMethodCall && args.length ?
$.extend.apply( null, [ true, options ].concat(args) ) :
$.widget.extend.apply( null, [ options ].concat(args) ) :
options;

if ( isMethodCall ) {
Expand Down Expand Up @@ -163,7 +180,7 @@ $.Widget.prototype = {
_createWidget: function( options, element ) {
element = $( element || this.defaultElement || this )[ 0 ];
this.element = $( element );
this.options = $.extend( true, {},
this.options = $.widget.extend( {},
this.options,
this._getCreateOptions(),
options );
Expand Down Expand Up @@ -218,7 +235,7 @@ $.Widget.prototype = {

if ( arguments.length === 0 ) {
// don't return a reference to the internal hash
return $.extend( {}, this.options );
return $.widget.extend( {}, this.options );
}

if ( typeof key === "string" ) {
Expand All @@ -230,7 +247,7 @@ $.Widget.prototype = {
parts = key.split( "." );
key = parts.shift();
if ( parts.length ) {
curOption = options[ key ] = $.extend( true, {}, this.options[ key ] );
curOption = options[ key ] = $.widget.extend( {}, this.options[ key ] );
for ( i = 0; i < parts.length - 1; i++ ) {
curOption[ parts[ i ] ] = curOption[ parts[ i ] ] || {};
curOption = curOption[ parts[ i ] ];
Expand Down