Opened 12 years ago
Closed 12 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: |
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.
Change History (8)
comment:1 Changed 12 years ago by
Component: | unfiled → templates |
---|---|
Owner: | set to john |
Status: | new → assigned |
comment:2 Changed 12 years ago by
Owner: | changed from john to BorisMoore |
---|---|
Status: | assigned → new |
comment:3 Changed 12 years ago by
Status: | new → assigned |
---|
comment:4 follow-up: 5 Changed 12 years ago by
Cc: | BorisMoore added |
---|---|
Priority: | undecided → low |
Boris, has there been any updates here?
comment:5 Changed 12 years ago by
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 12 years ago by
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 12 years ago by
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 12 years ago by
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.
Yes, makes sense. I will make a fix for this soon. Thanks.