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

Passing a function reference with @ runs it #2116

Closed
dylanrtt opened this Issue Dec 3, 2015 · 17 comments

Comments

Projects
None yet
3 participants
@dylanrtt
Contributor

dylanrtt commented Dec 3, 2015

http://jsbin.com/veyisiwesi/edit?html,js,console

<foo-bar {child}="@parent"></foo-bar>

The parent function shouldn't run until called. Worked in 2.3.2. If I use a 2-way binding, it runs twice (but I would never do that with a proto function).

@justinbmeyer justinbmeyer added the bug label Dec 3, 2015

@justinbmeyer justinbmeyer added this to the 2.3.5 milestone Dec 3, 2015

@justinbmeyer justinbmeyer self-assigned this Dec 3, 2015

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Dec 3, 2015

Contributor

I might do a quick fix so that <foo-bar {@child}="@parent"></foo-bar> works.

Contributor

justinbmeyer commented Dec 3, 2015

I might do a quick fix so that <foo-bar {@child}="@parent"></foo-bar> works.

@dylanrtt

This comment has been minimized.

Show comment
Hide comment
@dylanrtt

dylanrtt Dec 3, 2015

Contributor

No rush for me. I'll just stay in 2.3.2 for now. I saw you fixed some of my other issues on master but there's just too many 2.3.3 problems right now for me to upgrade yet.

Contributor

dylanrtt commented Dec 3, 2015

No rush for me. I'll just stay in 2.3.2 for now. I saw you fixed some of my other issues on master but there's just too many 2.3.3 problems right now for me to upgrade yet.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Dec 3, 2015

Contributor

This one was caused by unifying all binding logic. Essentially, a compute is made for child. When the compute is set to a function, computes automatically re-read themselves to get their new value.

In this case "child" is read, which ends up calling the function. Instead if @child was read, things will work. This is also needed for two-way binding.

Contributor

justinbmeyer commented Dec 3, 2015

This one was caused by unifying all binding logic. Essentially, a compute is made for child. When the compute is set to a function, computes automatically re-read themselves to get their new value.

In this case "child" is read, which ends up calling the function. Instead if @child was read, things will work. This is also needed for two-way binding.

@dylanrtt

This comment has been minimized.

Show comment
Hide comment
@dylanrtt

dylanrtt Dec 3, 2015

Contributor

Are you suggesting that <foo-bar {@child}="@parent"></foo-bar> should be the correct syntax for 1-way binding a function parent-to-child? I think you did something similar for child-to-parent...

Contributor

dylanrtt commented Dec 3, 2015

Are you suggesting that <foo-bar {@child}="@parent"></foo-bar> should be the correct syntax for 1-way binding a function parent-to-child? I think you did something similar for child-to-parent...

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Dec 3, 2015

Contributor

Yes, it's the necessary syntax. Unless we want to special case things.

Contributor

justinbmeyer commented Dec 3, 2015

Yes, it's the necessary syntax. Unless we want to special case things.

@dylanrtt

This comment has been minimized.

Show comment
Hide comment
@dylanrtt

dylanrtt Dec 3, 2015

Contributor

Fine by me. Whatever makes things easier.

Maybe.

Contributor

dylanrtt commented Dec 3, 2015

Fine by me. Whatever makes things easier.

Maybe.

@dylanrtt

This comment has been minimized.

Show comment
Hide comment
@dylanrtt

dylanrtt Dec 3, 2015

Contributor

@justinbmeyer Are you still planning to do #2065 eventually or just leave it with double @ as the norm?

Contributor

dylanrtt commented Dec 3, 2015

@justinbmeyer Are you still planning to do #2065 eventually or just leave it with double @ as the norm?

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Dec 3, 2015

Contributor

@dylanrtt the reason is that {^method}="parent" would call method and set parent to its return value, while {^@method}="@parent" would export the function itself to @parent.

Regarding #2065, yes, I might be able to special case @ so it's not needed on both sides in 1-way bindings. Basically, I can prevent the "setter" compute from having to ever read itself.

But I want to think about it a bit first.

Contributor

justinbmeyer commented Dec 3, 2015

@dylanrtt the reason is that {^method}="parent" would call method and set parent to its return value, while {^@method}="@parent" would export the function itself to @parent.

Regarding #2065, yes, I might be able to special case @ so it's not needed on both sides in 1-way bindings. Basically, I can prevent the "setter" compute from having to ever read itself.

But I want to think about it a bit first.

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

@dylanrtt

This comment has been minimized.

Show comment
Hide comment
@dylanrtt

dylanrtt Dec 3, 2015

Contributor

The @ modifier makes sense when reading values, not writing them. Adding @ to the destination has no meaning. With {^@method}="@parent", you are not assigning to @parent, but to parent.

I suppose a 2-way binding like {(@method)}="@parent" is actually a bit tricky since it's setting @parent to method and @method to parent.

Contributor

dylanrtt commented Dec 3, 2015

The @ modifier makes sense when reading values, not writing them. Adding @ to the destination has no meaning. With {^@method}="@parent", you are not assigning to @parent, but to parent.

I suppose a 2-way binding like {(@method)}="@parent" is actually a bit tricky since it's setting @parent to method and @method to parent.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Dec 3, 2015

Contributor

Could it have meaning? Say we had data like:

var compute = can.compute();
var map = new Map({compute: compute});

Would there be a difference between @compute and compute when setting?

Contributor

justinbmeyer commented Dec 3, 2015

Could it have meaning? Say we had data like:

var compute = can.compute();
var map = new Map({compute: compute});

Would there be a difference between @compute and compute when setting?

@dylanrtt

This comment has been minimized.

Show comment
Hide comment
@dylanrtt

dylanrtt Dec 3, 2015

Contributor

Interesting. I suppose that would depend on what the current behavior is, which I originally thought (I could be wrong) is that it always assigns directly to the property. That is, the hypothetical @ destination syntax would keep the current behavior.

If I'm right so far, omitting @ would allow you to instead call a function/compute on the destination with the assigned value, but that behavior can already be achieved by defining the component that way, so having the syntax flexibility would allow the component to be used multiple ways.

Then you may have to consider a scenario where the developer wants different read/write functionality when using @ on both sides in a 2-way binding.

My initial thought is that we should keep it a read modifier and call it a day.

Contributor

dylanrtt commented Dec 3, 2015

Interesting. I suppose that would depend on what the current behavior is, which I originally thought (I could be wrong) is that it always assigns directly to the property. That is, the hypothetical @ destination syntax would keep the current behavior.

If I'm right so far, omitting @ would allow you to instead call a function/compute on the destination with the assigned value, but that behavior can already be achieved by defining the component that way, so having the syntax flexibility would allow the component to be used multiple ways.

Then you may have to consider a scenario where the developer wants different read/write functionality when using @ on both sides in a 2-way binding.

My initial thought is that we should keep it a read modifier and call it a day.

@dylanrtt dylanrtt changed the title from Passing a function reference with @ runs it in 2.3.3 to Passing a function reference with @ runs it Dec 3, 2015

@pYr0x

This comment has been minimized.

Show comment
Hide comment

pYr0x commented Dec 3, 2015

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Dec 3, 2015

Contributor

@pYr0x what needs to be documented better? There's an example showing how to export a parent function, with an example.

Contributor

justinbmeyer commented Dec 3, 2015

@pYr0x what needs to be documented better? There's an example showing how to export a parent function, with an example.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Dec 3, 2015

Contributor

@pYr0x well, we should probably add a similar example for the other direction: http://canjs.com/docs/can.view.bindings.toChild.html

@dylanrtt I've made it possible to drop the @ on the receiving side of a one way binding. So {child}="@method" and {^@method}="parent" will work. I think a lot of people will try this. And it should work. It's likely that even if there could be a difference between:

  • {child}="@method"
  • {@child}="@method"

That difference would only matter if in fact child pointed to a compute and we could detect that and do the right thing, while still allowing {child}="@method" for the vast majority of use cases.

I think the docs should continue to show @ on both sides. I think it's good guidance because they are needed for two way bindings.

Contributor

justinbmeyer commented Dec 3, 2015

@pYr0x well, we should probably add a similar example for the other direction: http://canjs.com/docs/can.view.bindings.toChild.html

@dylanrtt I've made it possible to drop the @ on the receiving side of a one way binding. So {child}="@method" and {^@method}="parent" will work. I think a lot of people will try this. And it should work. It's likely that even if there could be a difference between:

  • {child}="@method"
  • {@child}="@method"

That difference would only matter if in fact child pointed to a compute and we could detect that and do the right thing, while still allowing {child}="@method" for the vast majority of use cases.

I think the docs should continue to show @ on both sides. I think it's good guidance because they are needed for two way bindings.

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

fixes #2117 by preventing double child initialization and makes it po…
…ssible to send a function without @ on both sides for #2116

@daffl daffl closed this in #2118 Dec 3, 2015

@pYr0x

This comment has been minimized.

Show comment
Hide comment
@pYr0x

pYr0x Jan 4, 2016

@justinbmeyer on the documentation http://canjs.com/docs/can.view.bindings.toParent.html#section_ExportingFunctions

<my-panel {add-panel}="@*addPanel" title="CanJS">CanJS Content</my-panel>

Shouldn't the @ not be on both sides?

pYr0x commented Jan 4, 2016

@justinbmeyer on the documentation http://canjs.com/docs/can.view.bindings.toParent.html#section_ExportingFunctions

<my-panel {add-panel}="@*addPanel" title="CanJS">CanJS Content</my-panel>

Shouldn't the @ not be on both sides?

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Jan 4, 2016

Contributor

@pYr0x it works both ways.

Contributor

justinbmeyer commented Jan 4, 2016

@pYr0x it works both ways.

@pYr0x

This comment has been minimized.

Show comment
Hide comment
@pYr0x

pYr0x Jan 4, 2016

@justinbmeyer but you said

I think the docs should continue to show @ on both sides. I think it's good guidance because they are needed for two way bindings.

we should write a documentation, which is uniform with all github issues and forum posts

pYr0x commented Jan 4, 2016

@justinbmeyer but you said

I think the docs should continue to show @ on both sides. I think it's good guidance because they are needed for two way bindings.

we should write a documentation, which is uniform with all github issues and forum posts

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