Bug Tracker

Modify

Ticket #7115 (closed feature: invalid)

Opened 4 years ago

Last modified 3 years ago

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
Blocking: Blocked by:

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

comment:1 Changed 4 years ago by john

  • Owner set to john
  • Status changed from new to assigned
  • Component changed from unfiled to templates

comment:2 Changed 4 years ago by BorisMoore

  • Owner changed from john to BorisMoore
  • Status changed from assigned to new

comment:3 Changed 4 years ago by BorisMoore

  • Status changed from new to assigned

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

comment:4 follow-up: ↓ 5 Changed 3 years ago by rwaldron

  • Cc BorisMoore added
  • Priority changed from undecided to low

Boris, has there been any updates here?

comment:5 in reply to: ↑ 4 Changed 3 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 3 years ago by rwaldron

  • Owner changed from BorisMoore to rwaldron
  • Milestone 1.4.3 deleted

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

comment:7 Changed 3 years ago by BorisMoore

  • Owner changed from rwaldron 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 3 years ago by BorisMoore

  • Status changed from assigned to closed
  • Type changed from bug to feature
  • Resolution set to invalid

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.

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.