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:

Description

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.

Attachments (0)

Change History (8)

Changed October 05, 2010 01:02PM UTC by john comment:1

component:	unfiled → templates
owner:	→ john
status:	new → assigned

Changed October 05, 2010 06:01PM UTC by BorisMoore comment:2

owner:	john → BorisMoore
status:	assigned → new

Changed October 06, 2010 11:24PM UTC by BorisMoore comment:3

status:	new → assigned

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

Changed November 02, 2010 09:11PM UTC by rwaldron comment:4

cc:	→ BorisMoore
priority:	undecided → low

Boris, has there been any updates here?

Changed November 02, 2010 11:47PM UTC by BorisMoore comment:5

Replying to [comment:4 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...

Changed November 03, 2010 02:01AM UTC by rwaldron comment:6

milestone:	1.4.3
owner:	BorisMoore → rwaldron

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

http://github.com/jquery/jquery-tmpl/pull/19

Changed November 20, 2010 05:05PM UTC by BorisMoore comment:7

owner:	rwaldron → 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.

Changed November 23, 2010 02:22AM UTC by BorisMoore comment:8

resolution:	→ invalid
status:	assigned → closed
type:	bug → feature

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.

Side navigation

#7115 closed feature (invalid)