Skip to content
This repository has been archived by the owner on Sep 16, 2022. It is now read-only.

Template syntax should support tear-offs for event bindings #506

Closed
alorenzen opened this issue Jul 10, 2017 · 12 comments
Closed

Template syntax should support tear-offs for event bindings #506

alorenzen opened this issue Jul 10, 2017 · 12 comments

Comments

@alorenzen
Copy link
Contributor

Right now, we have to supply a method call for template bindings, using the magical $event variable.
<material-button (trigger)="onClick($event)"></material-button>

In order to be more "dart-y", and also to remove the need to remember the magic $event variable, we could instead support a tear-off
<material-button (trigger)="onClick"></material-button>

@alorenzen
Copy link
Contributor Author

Open question: would we handle onFoo() (no event object) as well as onFoo($event) (event object)?
I.e. would we try to infer the behavior based on the presence / absence of a variable (either in the @Output like (trigger), or in the handler, like onClick), or just support one use case (such as assume there is a single variable).

@matanlurey
Copy link
Contributor

I'm a fan of inferring based on a variable/lack-of. WDUT @hterkelsen?

@alorenzen
Copy link
Contributor Author

If that's too hard to land off the bat, we could start with one and then add support for inference.

Also, should we block this on the angular_ast integration?

@matanlurey
Copy link
Contributor

Probably. /cc @MichaelRFairhurst @mk13

@MichaelRFairhurst
Copy link
Contributor

What about something like this?

class Counter {
  int _count = 0;
  int get tick => ++count;
}

and in the tpl

<div (click)="tick">

And what about

<div (click)="counter.tick">

in short, this requires more specification. And I'm seeing a lot of surprising edge cases that will exist in any spec.

Its kind of like if dart suddenly added support for

  int myMethod() => foo;

Yes, practically you can look at that and figure out how this could be a tearoff....but the theory gets mucky and so its probably better to change the syntax

  int myMethod() = foo;

what about

  <div (click)=".foo">

? Though then we're getting back into some of the problems that pipe expressions had, where we're deviating from dart expression syntax which causes its own problems.

I like the idea of this. I'm hoping there's a more precise way to specify it.

@jonahwilliams
Copy link
Contributor

jonahwilliams commented Jul 11, 2017

As long as we're throwing ideas out there...

Off the top of my head, there are only two uses for the current template syntax for calling methods.

  1. Passing the magical $event argument.
  2. Capturing local variables from a directive (NgFor, NgFor, and NgFor).

Everything else, like passing literals can be done from the controller.

What if there was a annotation like @Handle() which allowed you to somehow declare what methods from the controller you wanted to use in the template, such that the angular compiler could infer what to do with just an identifier.

<div *ngFor="let foo of foos">
   <button (click)="handleFoo">Click Me</button>
</div>
@Handle(/*...some additional information maybe needed here */)
void handleFoo(Event event, Foo foo) {
  /// Do some stuff.
}

I'm not sure what information would be required. maybe the types would get you far enough, or maybe you would need the names of the locals.

(As a side note, my ulterior motive is I want a template syntax where dart expressions aren't required.)

@zoechi
Copy link

zoechi commented Jul 11, 2017

I like the original idea, but if it requires additional Angular syntax, I think it's not worth it.

@matanlurey matanlurey added this to the >5.0.0 <6.0.0 milestone Apr 12, 2018
@TedSander
Copy link
Contributor

@jonahwilliams there is another case. Passing a varref from a template directive:
<input #input></input><button (click)="onClick(input.value)">Submit</button>
It is nice when you don't really need the element in the controller and so don't want to add the extra overhead of a @ViewChild

@matanlurey
Copy link
Contributor

This is now in #1437!

@alorenzen
Copy link
Contributor Author

I haven't landed #1437 yet, so shouldn't we keep this open until then? The PR has a comment that will auto close this when landed.

@alorenzen alorenzen reopened this Jul 9, 2018
@matanlurey
Copy link
Contributor

#1437 is now synced in d72bcef? @alorenzen

@alorenzen
Copy link
Contributor Author

Oopps, sorry. I had a CL pending from before vacay, but forgot that it's a fix for this, not the PR itself. Yes, this can be closed.

chalin added a commit that referenced this issue Jul 17, 2018
- Fix typo in issue number: `#1849` -> #1489
- Add a reference to: Template syntax should support tear-offs for event bindings #506

cc @kwalrath
chalin added a commit that referenced this issue Jul 17, 2018
- Fix typo in issue number: `#1849` -> #1489
- Add a reference to: Template syntax should support tear-offs for event bindings #506

cc @kwalrath
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants