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

UpgradeAdapter sets incorrect keys in ng1 directive controller #8856

Closed
palmanis opened this issue May 26, 2016 · 9 comments
Closed

UpgradeAdapter sets incorrect keys in ng1 directive controller #8856

palmanis opened this issue May 26, 2016 · 9 comments

Comments

@palmanis
Copy link

palmanis commented May 26, 2016

  • I'm submitting a bug report

Current behavior
If ng1 directive controller has properties bound using bindToController and if the instance variable name is different from the attribute being read, then while using it through UpgradeAdapter, the controller gets all those properties with the attribute names and not the instance variable names.
for e.g.

controller: function () {
  this.first;
  this.second;
  this.third;
},
bindToController: {
  first: '<firstValue',
  second: '=?secondValue',
  third: '=?'
},
controllerAs: 'vm'

The when you use the directive upgraded from UpgradeAdapter, then upon inspecting the controller these show up as:

vm.firstValue
vm.?secondValue
vm.?

Expected/desired behavior
The values in controller should be populated in the variables first, second and third.

  • If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via
    Here's a plunkr to reproduce/see this issue.
    https://plnkr.co/edit/0FCNd7bQ9zM3jKTViD2t?p=preview
    If you notice i use the upgraded directive as:
    <ng1-directive [first]="'one'" [second]="'two'">
    And i have printed all the controller variables in the output and they end up as:
    First:
    Second:
    FirstValue: one
    Entire Controller: {"?secondValue":"two","firstValue":"one"}
  • What is the motivation / use case for changing the behavior?
    Because of this bug, directives which have such bindings (which are genuinely required in some cases) do not work as expected. And frankly this is one of the numerous bugs we are coming across while using the UpgradeAdapter, using which is a pretty frustrating experience. It definitely does not work as it's expected to as per the guide.
  • Please tell us about your environment:
  • Angular version: 2.0.0-beta.17
  • Browser: [all | Chrome XX | Firefox XX | IE XX | Safari XX | Mobile Chrome XX | Android X.X Web Browser | iOS XX Safari | iOS XX UIWebView | iOS XX WKWebView ]
@wawyed
Copy link

wawyed commented Apr 6, 2017

Any updates on this issue? Seems quite important to being left for this long.

@gkalpak
Copy link
Member

gkalpak commented Apr 10, 2017

I am 92.456% sure that this has been fixed. @wawyed, are you still seeing this error with the latest version?

@gkalpak
Copy link
Member

gkalpak commented Apr 11, 2017

So, it hasn't been fixed for the "old" dynamic version, but it works correctly for upgrade/static 😞

@wawyed
Copy link

wawyed commented Apr 11, 2017

We are using dynamic upgrade adapter.

@wawyed
Copy link

wawyed commented Apr 13, 2017

So.. Is this going to get fixed? Do I need to raise a different issue?

@gkalpak
Copy link
Member

gkalpak commented Apr 13, 2017

Yes, it is going to get fixed (on 4.x at least), but I strongly recommend to start using upgrade/static as soon as possible 😃

@wawyed
Copy link

wawyed commented Apr 13, 2017

Why? Is the dynamic upgrade going to be removed?

@gkalpak
Copy link
Member

gkalpak commented Apr 13, 2017

@wawyed, I am not sure about that (nor is it for me to decide). The reason I recommend upgrade/static is that is supports AoT and has a better API imo.

gkalpak added a commit to gkalpak/angular that referenced this issue Apr 18, 2017
…dings

Previously, when using a different property/attribute name for an upgraded
component's binding (e.g. `bindings: {propName: '<attrName'}`), the property and
attribute names where swapped (e.g. using `attrName` as the property name and
`propName` as the attribute name). This resulted in unexpected behavior.

This commit fixes this using the correct name for properties and attributes.

This only affects `upgrade/dynamic`. `upgrade/static` works correctly already.

Fixes angular#8856
gkalpak added a commit to gkalpak/angular that referenced this issue Apr 18, 2017
…dings

Previously, when using a different property/attribute name for an upgraded
component's binding (e.g. `bindings: {propName: '<attrName'}`), the property and
attribute names were swapped (e.g. using `attrName` as the property name and
`propName` as the attribute name). This resulted in unexpected behavior.

This commit fixes this using the correct names for properties and attributes.

This only affects `upgrade/dynamic`. `upgrade/static` works correctly already.

Fixes angular#8856
gkalpak added a commit to gkalpak/angular that referenced this issue Apr 22, 2017
…dings

Previously, when using a different property/attribute name for an upgraded
component's binding (e.g. `bindings: {propName: '<attrName'}`), the property and
attribute names were swapped (e.g. using `attrName` as the property name and
`propName` as the attribute name). This resulted in unexpected behavior.

This commit fixes this using the correct names for properties and attributes.

This only affects `upgrade/dynamic`. `upgrade/static` works correctly already.

Fixes angular#8856
gkalpak added a commit to gkalpak/angular that referenced this issue Apr 22, 2017
…dings

Previously, when using a different property/attribute name for an upgraded
component's binding (e.g. `bindings: {propName: '<attrName'}`), the property and
attribute names were swapped (e.g. using `attrName` as the property name and
`propName` as the attribute name). This resulted in unexpected behavior.

This commit fixes this using the correct names for properties and attributes.

This only affects `upgrade/dynamic`. `upgrade/static` works correctly already.

Fixes angular#8856
gkalpak added a commit to gkalpak/angular that referenced this issue Apr 23, 2017
…dings

Previously, when using a different property/attribute name for an upgraded
component's binding (e.g. `bindings: {propName: '<attrName'}`), the property and
attribute names were swapped (e.g. using `attrName` as the property name and
`propName` as the attribute name). This resulted in unexpected behavior.

This commit fixes this using the correct names for properties and attributes.

This only affects `upgrade/dynamic`. `upgrade/static` works correctly already.

Fixes angular#8856
asnowwolf pushed a commit to asnowwolf/angular that referenced this issue Aug 11, 2017
…dings (angular#16128)

Previously, when using a different property/attribute name for an upgraded
component's binding (e.g. `bindings: {propName: '<attrName'}`), the property and
attribute names were swapped (e.g. using `attrName` as the property name and
`propName` as the attribute name). This resulted in unexpected behavior.

This commit fixes this using the correct names for properties and attributes.

This only affects `upgrade/dynamic`. `upgrade/static` works correctly already.

Fixes angular#8856

PR Close angular#16128
juleskremer pushed a commit to juleskremer/angular that referenced this issue Aug 28, 2017
…dings (angular#16128)

Previously, when using a different property/attribute name for an upgraded
component's binding (e.g. `bindings: {propName: '<attrName'}`), the property and
attribute names were swapped (e.g. using `attrName` as the property name and
`propName` as the attribute name). This resulted in unexpected behavior.

This commit fixes this using the correct names for properties and attributes.

This only affects `upgrade/dynamic`. `upgrade/static` works correctly already.

Fixes angular#8856

PR Close angular#16128
@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 11, 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

Successfully merging a pull request may close this issue.

4 participants