Bug Tracker

Opened 7 years ago

Closed 6 years ago

#7115 closed feature (invalid)

jQuery.tmpl, options not (deep)extending data properly

Reported by: jhermsen Owned by: BorisMoore
Priority: low Milestone:
Component: templates Version: 1.4.2
Keywords: tmpl extend Cc: BorisMoore
Blocked by: Blocking:


I noticed a little problem in the extending of the newItem with options.

In the function newTmplItem there is

jQuery.extend( newItem, options, { nodes: [], parent: parentItem } );

instead of

jQuery.extend( true, newItem, options, { nodes: [], parent: parentItem } );

So currently if you try to extend data for example:

$("#mytemplate").tmpl(data, {data:{fixedvalue:'myvalue'}}).appendTo("#mydiv");

then the fixedvalue is the only data being mapped.

Change History (8)

comment:1 Changed 7 years ago by john

Component: unfiledtemplates
Owner: set to john
Status: newassigned

comment:2 Changed 7 years ago by BorisMoore

Owner: changed from john to BorisMoore
Status: assignednew

comment:3 Changed 7 years ago by BorisMoore

Status: newassigned

Yes, makes sense. I will make a fix for this soon. Thanks.

comment:4 Changed 6 years ago by Rick Waldron

Cc: BorisMoore added
Priority: undecidedlow

Boris, has there been any updates here?

comment:5 in reply to:  4 Changed 6 years ago by BorisMoore

Replying to rwaldron:

Boris, has there been any updates here?

No, I'm on vacation, and wanted to do this fix along with a few others which I will need to do. Also right after vacation I have some other high priority work for jQuery to do. So expect a fix along with some other fixes but not for a couple of weeks. Not forgotten though...

comment:6 Changed 6 years ago by Rick Waldron

Milestone: 1.4.3
Owner: changed from BorisMoore to Rick Waldron

Well, for the sake of getting this bug fixed and closed, here is a patch and pull request all ready to roll:


comment:7 Changed 6 years ago by BorisMoore

Owner: changed from Rick Waldron to BorisMoore

The original design for options did not target a scenario of deep merging of data. Making the options merge deep breaks some other functionality, as revealed by the fact that the change breaks both the movies3.html sample and the tabsWrapNested.html sample.

OTOH it is certainly an interesting idea to be able to pass in default data values or additional methods on the data object, via options. The deep merge actually means (apart from breaking other features, as mentioned above) that a data field value or method passed in through options replaces the value on every data instance. More useful semantics in my view would be to support sparse or jagged data, and some data optimization scenarios, by providing values as default values (fallbacks if undefined) so that if the value is defined on the data item, it is not replaced by the value passed in in options.

Fix coming which does provide these semantics, and does not break other behavior.

comment:8 Changed 6 years ago by BorisMoore

Resolution: invalid
Status: assignedclosed
Type: bugfeature

It turns out that this approach to setting default data values has a lot of issues. Calling .tmpl or $.tmpl should not modify data values. I am incubating another way of addressing the scenario. In the meantime, I have a fix which prevents being able to modify data values through the options map on .tmpl.

Resolving as invalid, since it is not intended or appropriate to do a deep merge of options with the tmplItem.

Note: See TracTickets for help on using tickets.