Bug with conditionally nested components using stache #967

Closed
moschel opened this Issue May 6, 2014 · 6 comments

Comments

Projects
None yet
4 participants
@moschel
Contributor

moschel commented May 6, 2014

Added a test for this here.

If a component contains a nested component, that child component's "inserted" method will be called in an infinite loop if the parent renders with stache. This works fine with Mustache.

@moschel

This comment has been minimized.

Show comment
Hide comment
@moschel

moschel May 6, 2014

Contributor

This test will break with Uncaught RangeError: Maximum call stack size exceeded

Contributor

moschel commented May 6, 2014

This test will break with Uncaught RangeError: Maximum call stack size exceeded

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer May 6, 2014

Contributor

The problem is that when {{#if}} calls options.fn(), that calls hydrate to create the fragment, which eventually calls tagHandler which itself creates the instance of the child component. When the child component is created, it calls this.attr(data) on its scope which sets up a binding on _keys for the {{#if}}.

When the child component changes its scope attributes, it has the effect if re-running the {{#if}}.

To fix this, options.fn() should be "shielded" from a parent binding.

Contributor

justinbmeyer commented May 6, 2014

The problem is that when {{#if}} calls options.fn(), that calls hydrate to create the fragment, which eventually calls tagHandler which itself creates the instance of the child component. When the child component is created, it calls this.attr(data) on its scope which sets up a binding on _keys for the {{#if}}.

When the child component changes its scope attributes, it has the effect if re-running the {{#if}}.

To fix this, options.fn() should be "shielded" from a parent binding.

justinbmeyer added a commit that referenced this issue May 6, 2014

@ccummings ccummings added the Bug label May 7, 2014

@ccummings ccummings added this to the 2.1.1 milestone May 16, 2014

justinbmeyer added a commit that referenced this issue May 16, 2014

@ccummings

This comment has been minimized.

Show comment
Hide comment
@ccummings

ccummings May 16, 2014

Contributor

Fixed in master

Contributor

ccummings commented May 16, 2014

Fixed in master

@ccummings ccummings closed this May 16, 2014

@alexisabril

This comment has been minimized.

Show comment
Hide comment
@alexisabril

alexisabril May 26, 2014

Contributor

Possibly still an issue as I'm able to reproduce this if the computed value is updated: http://jsfiddle.net/Gd38M/1/

This is with jQuery 1.11.0 and CanJS 2.1.1

Edit: corrected fiddle.

Contributor

alexisabril commented May 26, 2014

Possibly still an issue as I'm able to reproduce this if the computed value is updated: http://jsfiddle.net/Gd38M/1/

This is with jQuery 1.11.0 and CanJS 2.1.1

Edit: corrected fiddle.

@alexisabril

This comment has been minimized.

Show comment
Hide comment
@alexisabril

alexisabril May 27, 2014

Contributor

Reviewed with @daffl. Re-opening and placing in 2.1.2 for further review.

Contributor

alexisabril commented May 27, 2014

Reviewed with @daffl. Re-opening and placing in 2.1.2 for further review.

@alexisabril alexisabril reopened this May 27, 2014

@alexisabril alexisabril modified the milestones: 2.1.2, 2.1.1 May 27, 2014

@alexisabril

This comment has been minimized.

Show comment
Hide comment
@alexisabril

alexisabril May 28, 2014

Contributor

Closing issue. The components in the fiddle are sharing the same template, so the infinite loop was human error.

Contributor

alexisabril commented May 28, 2014

Closing issue. The components in the fiddle are sharing the same template, so the infinite loop was human error.

gsmeets pushed a commit to gsmeets/canjs that referenced this issue Aug 15, 2014

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