#9472 closed bug (notabug)

Opened August 01, 2013 09:00PM UTC

Closed August 02, 2013 10:32AM UTC

Last modified August 02, 2013 12:56PM UTC

deep copy of options in _createWidget is problematic

Reported by: rattigan Owned by: rattigan
Priority: minor Milestone: none
Component: ui.widget Version: 1.10.3
Keywords: Cc:
Blocked by: Blocking:
Description

I just spent a while trying to figure out why my graph rendering widget wasn't working. It turned out that the object graph I was passing in via options was being deep copied. This meant that I could not rely on ref1 === ref2 on members of the graph. Besides this loss of referential identity I'm also concerned by the potential for:

  • Performance impact on large graphs
  • stack overflows for options containing cyclic objects

At the least, I think you should provide an option to disable this behavior on custom widgets. Workaround is to override _createWidget, but this is ugly.

This was introduced after a discussion here: https://groups.google.com/forum/#!topic/jquery-ui-dev/r8DuMEXYjyU

I think that the downsides were not fully appreciated at the time.

Attachments (0)
Change History (4)

Changed August 01, 2013 09:05PM UTC by scottgonzalez comment:1

component: ui.core → ui.widget
owner: → rattigan
status: new → pending

It's assumed that data sources are arrays, which we copy by reference. We'll need a reduced test case showing problems to even consider a change, but this is highly unlikely to change.

Changed August 01, 2013 10:52PM UTC by rattigan comment:2

status: pending → new

My mistake, I was not aware of this behaviour - I'm using an old version prior to this change:

https://github.com/jquery/jquery-ui/commit/b9153258b0f0edbff49496ed16d2aa93bec07d95

I don't see any documentation regarding this behaviour; it might be worth adding, since this is fairly subtle treatment of the options object.

Please feel free to close the ticket!

Changed August 02, 2013 10:32AM UTC by tj.vantoll comment:3

resolution: → notabug
status: new → closed

It would be nice to have the copy by reference of arrays documented somewhere because that has caused confusion for several users. Not sure where though.

scott.gonzalez: Is 

$.widget.extend
something we would want publicly documented or is that intended for internal use only? Ditto 
$.widget.bridge
.

Changed August 02, 2013 12:56PM UTC by scottgonzalez comment:4

I've filed https://github.com/jquery/api.jqueryui.com/issues/159 to track the documentation issue.

I don't think we want to document $.widget.extend(), but $.widget.bridge() is public. It was created specifically to allow third party developers to use custom constructors. I've filed an issue for that as well: https://github.com/jquery/api.jqueryui.com/issues/160

The bridge was inspired by Fluid's that-ist bridge, was prototyped when I was trying to highlight the pain points of inheritance for jQuery developers, and was subsequently explained by Eric Hynds.