Skip to content
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

Template local variables with dash (-) in them do not work #1447

Closed
bradlygreen opened this issue Apr 18, 2015 · 15 comments
Closed

Template local variables with dash (-) in them do not work #1447

bradlygreen opened this issue Apr 18, 2015 · 15 comments
Assignees

Comments

@bradlygreen
Copy link
Contributor

That is, this works:

<input #newtodo>
<button (click)="addTodo(newtodo.domElement.value)">Add Todo</button>

But this does not:

<input #new-todo>
<button (click)="addTodo(new-todo.domElement.value)">Add Todo</button>

Nor does this:

<input #newTodo>
<button (click)="addTodo(newTodo.domElement.value)">Add Todo</button>

If it's not actually supposed to work, we should warn when users do this.

@pkozlowski-opensource
Copy link
Member

I believe that it behaves "as currently designed":

  • dash-case (#new-todo) doesn't work since there is a conversion in the compiler that converts dash-cased variable names to the camel-cased one (so one would access it using newTodo.domElement.value). This conversion happens since HTML isn't case-sensitive and browsers will lower-case all the attribute names. Without this convention it would be impossible to declare variables with mixed case.
  • camel-case (#newTodo) doesn't work since ... HTML isn't case-sensitive and browsers will lower-case all the attribute names :-) So when we get to parsing the DOM and reading the attributes we are seeing #newtodo instead of #newTodo...

So, to make <input #new-todo> work one needs to write: <button (click)="addTodo(newTodo.domElement.value)">. We can't really make <input #newTodo> work since browsers will lower-case those. Here is a plunker showing how to make those 2 cases functional: http://plnkr.co/edit/09jjAeIN6ejh6TII5Zl6?p=preview

Now, while the above explains what is going on, it doesn't change the fact that things can get confusing. @mhevery do you think we can do things better here, apart from having clear docs on the subject?

@bradlygreen
Copy link
Contributor Author

Yep, I understand the reasons why we don't support these. Can we find a
way to warn users?
On Apr 20, 2015 1:27 AM, "Pawel Kozlowski" notifications@github.com wrote:

I believe that it behaves "as currently designed":

  • dash-case (#new-todo) doesn't work since there is a conversion in
    the compiler that converts dash-cased variable names to the camel-cased one
    (so one would access it using newTodo.domElement.value). This
    conversion happens since HTML isn't case-sensitive and browsers will
    lower-case all the attribute names. Without this convention it would be
    impossible to declare variables with mixed case.
  • camel-case (#newTodo) doesn't work since ... HTML isn't
    case-sensitive and browsers will lower-case all the attribute names :-) So
    when we get to parsing the DOM and reading the attributes we are seeing
    #newtodo instead of #newTodo...

So, to make <input #new-todo> work one needs to write: <button
(click)="addTodo(newTodo.domElement.value)">. We can't really make <input
#newTodo> work since browsers will lower-case those. Here is a plunker
showing how to make those 2 cases functional:
http://plnkr.co/edit/09jjAeIN6ejh6TII5Zl6?p=preview

Now, while the above explains what is going on, it doesn't change the fact
that things can get confusing. @mhevery https://github.com/mhevery do
you think we can do things better here, apart from having clear docs on the
subject?


Reply to this email directly or view it on GitHub
#1447 (comment).

@pkozlowski-opensource
Copy link
Member

There are few ideas from @rkirov in #591 on how to tackle #newTodo. Implementing them would mean that we need to parse templates as string, before "giving" them to a browser. This is totally doable but would add more code / processing so to be done in the dev mode only.

Dealing with #new-todo is harder since it is a valid name - I can't think of a good solution here atm...

@mhevery
Copy link
Contributor

mhevery commented Apr 23, 2015

I think this should work. (ie it should be fixed, @pkozlowski-opensource can you send out a PR for it.)

<input #new-todo>
<button (click)="addTodo(newTodo.domElement.value)">Add Todo</button>

@pkozlowski-opensource
Copy link
Member

I'm not sure how this could work as a browser will convert <input #newTodo> into <input #newtodo> before we've got a chance to do anything :-(

@mhevery are you suggesting that we should do case-insensitive lookup of local variables?

@mhevery
Copy link
Contributor

mhevery commented Apr 23, 2015

@pkozlowski-opensource sorry, had a typo, fixed my example. Yes #new-todo should be accessed as newTodo.

@pkozlowski-opensource
Copy link
Member

@mhevery but this works already (I mean, conversion of #new-todo to newTodo for locals: http://plnkr.co/edit/oQM238NNOIHns89QrxDB?p=preview

@mhevery
Copy link
Contributor

mhevery commented Apr 23, 2015

ohh, than there is nothing to do. :-), only to throw a useful error for the user.

@pkozlowski-opensource
Copy link
Member

@mhevery right, the problem is that I don't really know when to throw an error:

  • if a user writes <input #newTodo> we see <input #newtodo> in the compiler, so we can't catch error there
  • if a user writes <input #new-todo> is is legit as well

We should be throwing if a user writes <input #newTodo> but we would have to do this by parsing templates as strings, before a browser parses it. This is what @rkirov proposed in #591

I'm available to discuss this in details if needed.

@mhevery
Copy link
Contributor

mhevery commented Apr 23, 2015

I see, so there really is not any good way of doing this. Let's punt on this for now.

@cehoffman
Copy link

Could this not be detected in the expression parser? Is there any way to have a valid variable name with a - there?

@mhevery
Copy link
Contributor

mhevery commented May 1, 2015

@cehoffman browser parser turns all attributes to lower case before we get a chance to look at them. Angular parser can not detect a-b since it is a valid subtraction.

@mhevery mhevery modified the milestones: M*: Later, Backlog May 29, 2015
@mhevery
Copy link
Contributor

mhevery commented May 30, 2015

not feasible. :-(

@mhevery mhevery closed this as completed May 30, 2015
@svrcekmichal
Copy link

@mhevery maybe just mention that in docs somewhere? I have created value newTodo and it cost me bit of time to spot that newtodo in browser

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants