#9472 closed bug (notabug)
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.
Change History (4)
comment:1 Changed 10 years ago by
Component: | ui.core → ui.widget |
---|---|
Owner: | set to rattigan |
Status: | new → pending |
comment:2 Changed 10 years ago by
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!
comment:3 Changed 10 years ago by
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
.
comment:4 Changed 10 years ago by
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.
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.