Putting a can.compute on a Component scope throws #1086

Closed
matthewp opened this Issue Jun 13, 2014 · 7 comments

Comments

Projects
None yet
4 participants
@matthewp
Contributor

matthewp commented Jun 13, 2014

Will attach a test shortly.

matthewp added a commit to matthewp/canjs that referenced this issue Jun 13, 2014

@daffl daffl added this to the 2.1.2 milestone Jun 13, 2014

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Jun 13, 2014

Contributor

A "single" value compute like this will blow up, but why would you use that? A function compute should work just fine. prop: can.compute(function(){}).

A Component is probably not necessary. You could probably get this with a map:

can.Map.extend({
  prop: can.compute(false)
})

This probably has to do with compute's clone method.

Contributor

justinbmeyer commented Jun 13, 2014

A "single" value compute like this will blow up, but why would you use that? A function compute should work just fine. prop: can.compute(function(){}).

A Component is probably not necessary. You could probably get this with a map:

can.Map.extend({
  prop: can.compute(false)
})

This probably has to do with compute's clone method.

@matthewp

This comment has been minimized.

Show comment
Hide comment
@matthewp

matthewp Jun 13, 2014

Contributor

That's true, probably no reason to do it this way. One of our existing apps this was working in 2.0.4.

Contributor

matthewp commented Jun 13, 2014

That's true, probably no reason to do it this way. One of our existing apps this was working in 2.0.4.

@matthewp

This comment has been minimized.

Show comment
Hide comment
@matthewp

matthewp Jun 13, 2014

Contributor

Tested and this doesn't break on a can.Map.

Contributor

matthewp commented Jun 13, 2014

Tested and this doesn't break on a can.Map.

@daffl daffl modified the milestone: 2.1.2 Jun 16, 2014

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Jun 16, 2014

Contributor

This changed from 2.0 to 2.1. Closing this issue as it can be fixed by setting it as a property instead of a value compute as discussed.

Contributor

daffl commented Jun 16, 2014

This changed from 2.0 to 2.1. Closing this issue as it can be fixed by setting it as a property instead of a value compute as discussed.

@daffl daffl closed this Jun 16, 2014

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Jun 16, 2014

Contributor

I'm not sure it should be closed. It still technically a bug.

Contributor

justinbmeyer commented Jun 16, 2014

I'm not sure it should be closed. It still technically a bug.

@daffl daffl added this to the 2.1.3 milestone Jun 16, 2014

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Jun 16, 2014

Contributor

Ok. Reopening and putting it int 2.1.3.

Contributor

daffl commented Jun 16, 2014

Ok. Reopening and putting it int 2.1.3.

@daffl daffl reopened this Jun 16, 2014

@justinbmeyer justinbmeyer added the Bug label Jun 18, 2014

@daffl daffl modified the milestones: 2.1.4, 2.1.3 Jun 18, 2014

@daffl daffl modified the milestones: 2.2.0, 2.1.4 Nov 14, 2014

@daffl daffl modified the milestones: 2.3.0, 2.2.0 Feb 18, 2015

alexisabril pushed a commit that referenced this issue Oct 2, 2015

@alexisabril

This comment has been minimized.

Show comment
Hide comment
@alexisabril

alexisabril Oct 2, 2015

Contributor

This was fixed in a prior merge in minor. Typo in the test, adding @matthewp's test to a PR.

Contributor

alexisabril commented Oct 2, 2015

This was fixed in a prior merge in minor. Typo in the test, adding @matthewp's test to a PR.

@daffl daffl closed this Oct 22, 2015

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