Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Component bindings: {} validation is too restrictive (does not allow dollar sign) #15586

Closed
christopherthielen opened this issue Jan 7, 2017 · 1 comment

Comments

@christopherthielen
Copy link

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

Currently, a component can be declared with a binding which includes a dollar sign:

.component('foo', { 
  bindings: { '$fooFactory': '<' }
});
// This works OK
...
<foo $foo-factory="$ctrl.$fooFactory"></foo>

However, when re-mapping the input binding to a different attribute name, the mapping validation rejects the "$" because it uses /\w+/

.component('foo', { 
  bindings: { 'factory': '<$fooFactory' }
});
// Error: [$compile:iscp] Invalid controller bindings definition for directive 'foo'. Definition: {... factory: '<$fooFactory' ...}
...
<foo $foo-factory="$ctrl.$fooFactory"></foo>

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://plnkr.co or similar (template: http://plnkr.co/edit/tpl:yBpEi4).

Create a component which binds an attribute with a dollar sign to some other controller variable

      app.component('foo', {
        bindings: { factory: '<$fooFactory' },
        template: `<h1>foo</h1>factory: <pre>{{ $ctrl.factory }}</pre>`,
      });     

This plunker shows
http://plnkr.co/edit/GNXR5uf05iW5yhdGpceL?p=preview

What is the expected behavior?

Since the input attribute (with a dollar sign) works when binding to a controller variable of the same name, I expect that the input attribute (with a dollar sign) should work when binding to a variable with a different name.

What is the motivation / use case for changing the behavior?

In UI-Router, resolve data can be bound to component inputs. One extremely important value for ui-router users is $transition$, which is the DI token for the current Transition object.

The general advice to ui-router users is to bind the transition to the component, if they need information about the current transition, i.e.:

.component('foo', {
  bindings: { $transition$: '<' }
});

However, $transition$ is admittedly unwieldy, and some users prefer to rename it when binding to a component.

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.

The isolate bindings validation happens at least back to ng 1.2.32: http://plnkr.co/edit/NOVaM1UlRENnTEwjjTm5?p=preview

Other information (e.g. stacktraces, related issues, suggestions how to fix)

@gkalpak
Copy link
Member

gkalpak commented Jan 9, 2017

Sounds reasonable. PRs welcome 😃

gkalpak pushed a commit that referenced this issue Jan 11, 2017
Previously, when using an alias for an isolate scope or `bindings` property
(e.g. `alias: '<attrName'` instead of `attrName: '<'`), a `$compile:iscp` error
was thrown if the attribute name contained a "$".
This commit removes the error by changing the regex to allow "$" characters in
the attribute name when using a property alias.

Fixes: #15586

Closes #15594
gkalpak pushed a commit that referenced this issue Jan 11, 2017
Previously, when using an alias for an isolate scope or `bindings` property
(e.g. `alias: '<attrName'` instead of `attrName: '<'`), a `$compile:iscp` error
was thrown if the attribute name contained a "$".
This commit removes the error by changing the regex to allow "$" characters in
the attribute name when using a property alias.

Fixes: #15586

Closes #15594
ellimist pushed a commit to ellimist/angular.js that referenced this issue Mar 15, 2017
Previously, when using an alias for an isolate scope or `bindings` property
(e.g. `alias: '<attrName'` instead of `attrName: '<'`), a `$compile:iscp` error
was thrown if the attribute name contained a "$".
This commit removes the error by changing the regex to allow "$" characters in
the attribute name when using a property alias.

Fixes: angular#15586

Closes angular#15594
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants