Skip to main content

Search and Top Navigation

#8645 closed bug (wontfix)

Opened October 10, 2012 05:36AM UTC

Closed October 10, 2012 12:08PM UTC

Last modified January 30, 2013 04:54PM UTC

Array option in widget isn't cloned

Reported by: warsky Owned by:
Priority: minor Milestone: 1.9.1
Component: ui.widget Version: 1.9.0
Keywords: Cc:
Blocked by: Blocking:
Description

We built a number of jQuery widgets under 1.8. Many of these widgets had options that were arrays:

options :

myArrayOption : [1, 2, 3]

and this worked well. Not so under 1.9. Under 1.9, when the widget is instantiated, instead of making a copy of the array option, it makes a copy of the reference to the array. So, if you instantiate the widget multiple times, they all reference the same array and changing one changes them all. It appears the problem is caused because 1.9.0 uses the new $.widget.extend() instead of the regular jquery.extend() The $.widget form does not handle arrays properly.

The fix is simple, in $.widget.extend() change:

target[ key ] = $.isPlainObject( value ) ? $.widget.extend( {}, target[ key ], value ) : value;

to:

if ($.isArray(value)) {

target[key] = $.widget.extend([], target[ key ], value);

}

else {

target[ key ] = $.isPlainObject( value ) ? $.widget.extend( {}, target[ key ], value ) : value;

}

I have worked around it by simply overriding $.widget.extend, which also requires changing the first line from slice.call(...) to Array.prototype.slice.call(...)

I'm going to let someone else change make the change to the repository...

Attachments (0)
Change History (9)

Changed October 10, 2012 12:08PM UTC by scottgonzalez comment:1

resolution: → wontfix
status: newclosed

This is intentional. It's impossible to keep a reference if we do a copy, but it's easy for you to copy if that's what you want. The old behavior was very problematic.

Changed October 10, 2012 04:01PM UTC by warsky comment:2

OK, but in JavaScript an array is essentially just an object where you use indices to reference elements instead of names. Why wouldn't you want the behavior to be the same for arrays and objects? You copy objects, but not arrays.

Perhaps there is some place in the framework where this causes trouble that I don't know about but I definitely don't see why when you instantiate widgets of the same type with an option that is an array that you would want all those widgets to reference the same array. Sure, I can create a routine that does a copy, but that means two steps for instantiation, and means all existing code has to be changed to the two step instantiation process.

Changed October 10, 2012 05:26PM UTC by scottgonzalez comment:3

_comment0: There are a lot of cases where arrays are data sources and being able to modify the original and have it change the widget is important. \ \ It's not exactly a two step process, it's just the addition of a `.each()` \ \ ``` \ elements.each(function() { \ $( this ).widget({ option: array.slice() }); \ }); \ ``` \ \ Alternatively, you can change your widgets to do clones internally on create.1349890008641033

There are a lot of cases where arrays are data sources and being able to modify the original and have it change the widget is important.

It's not exactly a two step process, it's just the addition of a .each()

elements.each(function() {
    $( this ).widget({ option: array.slice() });
});

Alternatively, you can change your widgets to do clones internally on create.

Changed January 22, 2013 09:19PM UTC by richard.s.hall comment:4

Personally, I agree with the original poster. It seems somewhat odd that the behavior would be different between the two. Although, I slightly disagree with the poster in that I think this behavior doesn't make sense at all and it should always be pass by reference for all options. It was painful for me to learn that it wasn't this way, since it makes the most sense to me.

Can someone comment on why cloning happens in the first place and also on why it makes more sense to have different behavior based on argument type than being consistent?

I think it makes the most sense to always pass by reference regardless of the argument type, because, as Scott points out, it is easy to copy if that's what you want, but much more painful to keep references if it is copied by default.

Changed January 22, 2013 09:25PM UTC by scottgonzalez comment:5

Richard: It's because pass by reference is a huge surprise to almost everyone. People do multipleElements.widget( options ) all the time.

Changed January 22, 2013 10:26PM UTC by richard.s.hall comment:6

Replying to [comment:5 scott.gonzalez]:

Richard: It's because pass by reference is a huge surprise to almost everyone. People do multipleElements.widget( options ) all the time.

Yeah, I guess so.

However, it is still not really clear to me why this would be less surprising in the array case than in the object case, especially enough to justify an inconsistency in the API.

Seems like it would be better for you to provide some sort of mechanism for us to say explicitly what we want to have happen, rather than having you guys guess what we want.

Thanks for the response.

Changed January 30, 2013 12:45PM UTC by rogeriorc comment:7

http://jsfiddle.net/T4NvA/5/

Here's a example.

I instantiate 4 distinct Widgets, the ones that I don't pass the array parameters share the same reference.

Changed January 30, 2013 12:57PM UTC by scottgonzalez comment:8

You shouldn't define an array as the default value. You should be setting an empty array if no rows are passed on create.

Changed January 30, 2013 04:54PM UTC by warsky comment:9

OK, so basically you are saying that default values should be set in create, rather than in the options declaration? That should work, but certainly makes it less clean unless you set all the options in create.

My biggest problem is that there was a rather fundamental change made to a rather established and stable API that addresses limited use cases and that breaks existing software in a rather significant way that is not always readily apparent (i.e. it doesn't just create a javascript error). Browser based developers have enough troubles keeping up with subtle browser changes. Hell, much of the reason for using an API like jQuery is that it hides these browser differences and changes and let's us program against a consistent API. But this change shows that maybe the goal of jQuery isn't to be consistent; that I have to worry about some subtle break every time I upgrade jQuery.