New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stache templates leak for various reasons #2090

Closed
akagomez opened this Issue Nov 18, 2015 · 10 comments

Comments

Projects
None yet
4 participants
@akagomez
Contributor

akagomez commented Nov 18, 2015

While searching for the source of a memory leak in my app I found several ways to influence the leak by manipulating my templates.

I extrapolated my findings into a few tests and committed them here in the stache-leaks branch.

The basis of the test involves adding/removing 1000 items to the DOM several times. Each test renders the items in a slightly different way (i.e. with the <content /> tag, without, with/without parent nodes, using the {{@key}} helper, etc)

The tests can also be run on JSBin: http://output.jsbin.com/yowaru

stache-leaks

/cc @justinbmeyer @daffl @imjoshdean

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Nov 18, 2015

Contributor

The examples seem to be using mustache templates. Remember that if template is a string, it's assumed to be mustache.

Sent from my iPhone

On Nov 18, 2015, at 1:14 PM, Chris Gomez notifications@github.com wrote:

While searching for the source of a memory leak in my app I found several ways to influence the leak by manipulating my templates.

I extrapolated my findings into a few tests and committed them here in the stache-leaks branch.

The test involves adding/removing 1000 items to DOM several times. Each test renders the items in a slightly different way (i.e. with the tag, without, with/without parent nodes, using the {{@key}} helper, etc)

The tests can also be run on JSBin: http://output.jsbin.com/yowaru

/cc @justinbmeyer @daffl @imjoshdean


Reply to this email directly or view it on GitHub.

Contributor

justinbmeyer commented Nov 18, 2015

The examples seem to be using mustache templates. Remember that if template is a string, it's assumed to be mustache.

Sent from my iPhone

On Nov 18, 2015, at 1:14 PM, Chris Gomez notifications@github.com wrote:

While searching for the source of a memory leak in my app I found several ways to influence the leak by manipulating my templates.

I extrapolated my findings into a few tests and committed them here in the stache-leaks branch.

The test involves adding/removing 1000 items to DOM several times. Each test renders the items in a slightly different way (i.e. with the tag, without, with/without parent nodes, using the {{@key}} helper, etc)

The tests can also be run on JSBin: http://output.jsbin.com/yowaru

/cc @justinbmeyer @daffl @imjoshdean


Reply to this email directly or view it on GitHub.

@akagomez

This comment has been minimized.

Show comment
Hide comment
@akagomez

akagomez Nov 19, 2015

Contributor

Darn. @imjoshdean and I thought we covered that base here:

var template = can.stache(
    '<list-toggler-1></list-toggler-1>\n' +
    '<list-toggler-2></list-toggler-2>\n' +
    '<list-toggler-3></list-toggler-3>\n');
var frag = template();
$(document.body).append(frag);

I didn't realize stache/mustache could work together.

Okay. So. I've updated the tests to use stache explicitly. As a result 2 of the 3 test turned out to be false alarms. Then I tried a few more test cases, and found another. Test _#_1 is a doozy. Test _#_2 may not even be a leak.

stache-leaks

(I updated the branch and the JSBin)

Contributor

akagomez commented Nov 19, 2015

Darn. @imjoshdean and I thought we covered that base here:

var template = can.stache(
    '<list-toggler-1></list-toggler-1>\n' +
    '<list-toggler-2></list-toggler-2>\n' +
    '<list-toggler-3></list-toggler-3>\n');
var frag = template();
$(document.body).append(frag);

I didn't realize stache/mustache could work together.

Okay. So. I've updated the tests to use stache explicitly. As a result 2 of the 3 test turned out to be false alarms. Then I tried a few more test cases, and found another. Test _#_1 is a doozy. Test _#_2 may not even be a leak.

stache-leaks

(I updated the branch and the JSBin)

@daffl daffl modified the milestones: 2.3.3, 2.3.4 Nov 25, 2015

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Dec 1, 2015

Contributor

There's a <content> within an {{#each}}. This is not officially supported.

Contributor

justinbmeyer commented Dec 1, 2015

There's a <content> within an {{#each}}. This is not officially supported.

@justinbmeyer justinbmeyer added the bug label Dec 1, 2015

@justinbmeyer justinbmeyer self-assigned this Dec 1, 2015

@akagomez

This comment has been minimized.

Show comment
Hide comment
@akagomez

akagomez Dec 1, 2015

Contributor

From the can.Component.prototype.template documentation:

Use

The template specified by the template property works similar to the W3C Shadow DOM proposal. It represents the contents of a custom element, while being able to reposition the user provided source elements with the <content> tag.

There are three things to understand about a can.Component's template:

  • It is inserted into the component's tag.
  • It is rendered with access to the component instance's viewModel.
  • <content> tags within the template act as insertion points for the source elements.

Additionally it says:

Use the <content> element to place the source content in the component's element within the component's template.

Source: http://canjs.com/docs/can.Component.prototype.template.html

While the documentation doesn't explicitly state that a <content> tag can reside within an {{#each}} block, I can't see how it could have been interpreted that it wasn't. Especially since it uses the terminology "insertion points".

Contributor

akagomez commented Dec 1, 2015

From the can.Component.prototype.template documentation:

Use

The template specified by the template property works similar to the W3C Shadow DOM proposal. It represents the contents of a custom element, while being able to reposition the user provided source elements with the <content> tag.

There are three things to understand about a can.Component's template:

  • It is inserted into the component's tag.
  • It is rendered with access to the component instance's viewModel.
  • <content> tags within the template act as insertion points for the source elements.

Additionally it says:

Use the <content> element to place the source content in the component's element within the component's template.

Source: http://canjs.com/docs/can.Component.prototype.template.html

While the documentation doesn't explicitly state that a <content> tag can reside within an {{#each}} block, I can't see how it could have been interpreted that it wasn't. Especially since it uses the terminology "insertion points".

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Dec 1, 2015

Contributor

<content> is based off the W3C <content> tag. This type of use will be impossible with that standard.

Contributor

justinbmeyer commented Dec 1, 2015

<content> is based off the W3C <content> tag. This type of use will be impossible with that standard.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Dec 1, 2015

Contributor

It says "insertion points" for the "source elements" which mimics the spec language. It doesn't say a "re-rendering of the light/user subtemplate".

Contributor

justinbmeyer commented Dec 1, 2015

It says "insertion points" for the "source elements" which mimics the spec language. It doesn't say a "re-rendering of the light/user subtemplate".

justinbmeyer added a commit that referenced this issue Dec 2, 2015

@gsmeets

This comment has been minimized.

Show comment
Hide comment
@gsmeets

gsmeets Dec 26, 2015

Contributor

I would like you to seriously consider officially supporting this, or support it in some other way. Repeatable templates is a very important feature.

Contributor

gsmeets commented Dec 26, 2015

I would like you to seriously consider officially supporting this, or support it in some other way. Repeatable templates is a very important feature.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Dec 26, 2015

Contributor

Please propose an alternate API. Something like .

Sent from my iPhone

On Dec 26, 2015, at 9:09 AM, Guido Smeets notifications@github.com wrote:

I would like you to seriously consider officially supporting this, or support it in some other way. Repeatable templates is a very important feature.


Reply to this email directly or view it on GitHub.

Contributor

justinbmeyer commented Dec 26, 2015

Please propose an alternate API. Something like .

Sent from my iPhone

On Dec 26, 2015, at 9:09 AM, Guido Smeets notifications@github.com wrote:

I would like you to seriously consider officially supporting this, or support it in some other way. Repeatable templates is a very important feature.


Reply to this email directly or view it on GitHub.

@akagomez

This comment has been minimized.

Show comment
Hide comment
@akagomez

akagomez Dec 30, 2015

Contributor

@justinbmeyer How would <user-template> differ from <content>?

Contributor

akagomez commented Dec 30, 2015

@justinbmeyer How would <user-template> differ from <content>?

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Dec 30, 2015

Contributor

@akagomez <user-template> would behave how our current <content> tag works, leaving <content> to work like how the spec <content> tag works.

Contributor

justinbmeyer commented Dec 30, 2015

@akagomez <user-template> would behave how our current <content> tag works, leaving <content> to work like how the spec <content> tag works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment