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

Problem with simple string attributes on downgraded component when ng-if is used #16212

Closed
th0r opened this issue Apr 20, 2017 · 3 comments · Fixed by #16511
Closed

Problem with simple string attributes on downgraded component when ng-if is used #16212

th0r opened this issue Apr 20, 2017 · 3 comments · Fixed by #16511

Comments

@th0r
Copy link

th0r commented Apr 20, 2017

I'm submitting a ... (check one with "x")

[x] bug report => search github for a similar issue or PR before submitting
[ ] feature request
[ ] support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

Let's say you have ng1+ng4 application with <test/> component written in ng4 and downgraded to ng1.

This component has some string input binding so there are a few ways to pass a value there:

  1. <test input="value"/>
  2. <test [input]="'value'"/>

The thing is if you use ng-if either on this component or on any of it's parents and use first way to pass an input value (using simple string attribute) it will be undefined on the call to ngOnInit or on the first call to ngOnChanges. It will be set only on the second call to ngOnChanges.

But in the second case of [input] binding everything works as expected.

See reproduction: http://plnkr.co/edit/qpZwej5dypPblBafnCVp?p=preview

Expected behavior

input value should be set before the call to ngOnInit and first call to ngOnChanges.

Minimal reproduction of the problem with instructions

Reproduction: http://plnkr.co/edit/qpZwej5dypPblBafnCVp?p=preview

Open browser console and you'll see input values for different cases in different component lifecycle hooks.

Press Toggle visibility button to print logs again.

You can also remove ng-if from the parent <div> and see the bug disappears: after page refresh it'll show valid input values in all lifecycle hooks.

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

Current behavior is a bug.

Please tell us about your environment:

I think it doesn't depend on environment.

  • Angular version: 4.0.2
  • Browser: Tested in Chrome 57
  • Language: TypeScript

  • Node (for AoT issues): -

@gkalpak
Copy link
Member

gkalpak commented Apr 21, 2017

Interesting. I don't think ngUpgrade has any special handling for [xyz] vs xyz. Investigating...

@gkalpak
Copy link
Member

gkalpak commented May 3, 2017

I don't think ngUpgrade has any special handling for [xyz] vs xyz.

It turns out it does 😃 Changes to xyz attributes are tracked using attrs.$observe() (here), while changes to [xyz](which are typically expressions) are tracked usingscope.$watch()` (there).

Under the hood, $observe() calls $evalAsync(), which adds callbacks to the asyncQueue. These callbacks will be run at the beginning of the next $digest iteration.

Note that there is another $watcher for running ngOnChanges for example. So, when we are already in the middle of a $digest (as happens when using ngIf), the new $watchers will be executed as part of the current $digest iteration, thus before the asyncQueue callbacks.

This is roughly equivalent to the following:

scope.$watch(..., () => {
  // The `ngIf` expression changed, let's insert the components...
  ...
  /* 1 */ scope.$evalAsync(/* Initialize the `xyz` attributes */);
  /* 2 */ scope.$watch(..., /* Update the `[xyz]` attributes */);
  /* 3 */ scope.$watch(..., /* Run `ngOnChanges()` */);
});

The above callbacks will be executed in the order: 2 -> 3 -> 1 -> 3

When this all happens outside of a $digest, then everythng works as expected, because asyncQueue callbacks will be executed before the $watchers on the next $digest.

Working on a fix...

gkalpak added a commit to gkalpak/angular that referenced this issue May 3, 2017
Previously, non-bracketed inputs (e.g. `xyz="foo"`) on downgraded components
were initialized using `attrs.$observe()` (whihc uses `$evalAsync()` under the
hood), while bracketed inputs (e.g. `[xyz]="'foo'"`) were initialized using
`$watch()`. When the downgraded component was created during a `$digest` (e.g.
by an `ng-if` watcher), the nno-bracketed inputs were not initialized in time
for the initial call to `ngOnChanges()` and `ngOnInit()`.

This commit fixes it by using `$watch()` to initialize all inputs. `$observe()`
is still used for subsequent updates, because it is more performant.

Fixes angular#16212
gkalpak added a commit to gkalpak/angular that referenced this issue May 3, 2017
Previously, non-bracketed inputs (e.g. `xyz="foo"`) on downgraded components
were initialized using `attrs.$observe()` (which uses `$evalAsync()` under the
hood), while bracketed inputs (e.g. `[xyz]="'foo'"`) were initialized using
`$watch()`. If the downgraded component was created during a `$digest` (e.g. by
an `ng-if` watcher), the non-bracketed inputs were not initialized in time for
the initial call to `ngOnChanges()` and `ngOnInit()`.

This commit fixes it by using `$watch()` to initialize all inputs. `$observe()`
is still used for subsequent updates on non-bracketed inputs, because it is more
performant.

Fixes angular#16212
gkalpak added a commit to gkalpak/angular that referenced this issue May 3, 2017
Previously, non-bracketed inputs (e.g. `xyz="foo"`) on downgraded components
were initialized using `attrs.$observe()` (which uses `$evalAsync()` under the
hood), while bracketed inputs (e.g. `[xyz]="'foo'"`) were initialized using
`$watch()`. If the downgraded component was created during a `$digest` (e.g. by
an `ng-if` watcher), the non-bracketed inputs were not initialized in time for
the initial call to `ngOnChanges()` and `ngOnInit()`.

This commit fixes it by using `$watch()` to initialize all inputs. `$observe()`
is still used for subsequent updates on non-bracketed inputs, because it is more
performant.

Fixes angular#16212
matsko pushed a commit that referenced this issue May 4, 2017
Previously, non-bracketed inputs (e.g. `xyz="foo"`) on downgraded components
were initialized using `attrs.$observe()` (which uses `$evalAsync()` under the
hood), while bracketed inputs (e.g. `[xyz]="'foo'"`) were initialized using
`$watch()`. If the downgraded component was created during a `$digest` (e.g. by
an `ng-if` watcher), the non-bracketed inputs were not initialized in time for
the initial call to `ngOnChanges()` and `ngOnInit()`.

This commit fixes it by using `$watch()` to initialize all inputs. `$observe()`
is still used for subsequent updates on non-bracketed inputs, because it is more
performant.

Fixes #16212
matsko pushed a commit that referenced this issue May 4, 2017
Previously, non-bracketed inputs (e.g. `xyz="foo"`) on downgraded components
were initialized using `attrs.$observe()` (which uses `$evalAsync()` under the
hood), while bracketed inputs (e.g. `[xyz]="'foo'"`) were initialized using
`$watch()`. If the downgraded component was created during a `$digest` (e.g. by
an `ng-if` watcher), the non-bracketed inputs were not initialized in time for
the initial call to `ngOnChanges()` and `ngOnInit()`.

This commit fixes it by using `$watch()` to initialize all inputs. `$observe()`
is still used for subsequent updates on non-bracketed inputs, because it is more
performant.

Fixes #16212
asnowwolf pushed a commit to asnowwolf/angular that referenced this issue Aug 11, 2017
Previously, non-bracketed inputs (e.g. `xyz="foo"`) on downgraded components
were initialized using `attrs.$observe()` (which uses `$evalAsync()` under the
hood), while bracketed inputs (e.g. `[xyz]="'foo'"`) were initialized using
`$watch()`. If the downgraded component was created during a `$digest` (e.g. by
an `ng-if` watcher), the non-bracketed inputs were not initialized in time for
the initial call to `ngOnChanges()` and `ngOnInit()`.

This commit fixes it by using `$watch()` to initialize all inputs. `$observe()`
is still used for subsequent updates on non-bracketed inputs, because it is more
performant.

Fixes angular#16212
juleskremer pushed a commit to juleskremer/angular that referenced this issue Aug 28, 2017
Previously, non-bracketed inputs (e.g. `xyz="foo"`) on downgraded components
were initialized using `attrs.$observe()` (which uses `$evalAsync()` under the
hood), while bracketed inputs (e.g. `[xyz]="'foo'"`) were initialized using
`$watch()`. If the downgraded component was created during a `$digest` (e.g. by
an `ng-if` watcher), the non-bracketed inputs were not initialized in time for
the initial call to `ngOnChanges()` and `ngOnInit()`.

This commit fixes it by using `$watch()` to initialize all inputs. `$observe()`
is still used for subsequent updates on non-bracketed inputs, because it is more
performant.

Fixes angular#16212
@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.

3 participants