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

refactor(common): remove deprecated `NgFor` #18758

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
7 participants
@ocombe
Copy link
Contributor

commented Aug 17, 2017

PR Type

What kind of change does this PR introduce?

[ ] Other... Please describe: removing deprecated code

What is the current behavior?

NgFor was no longer used and was just a reference to NgForOf.

What is the new behavior?

NgFor has been removed as it was deprecated since v4. Use NgForOf instead. This does not impact the use of*ngFor in your templates.

Does this PR introduce a breaking change?

[x] Yes
@vicb

vicb approved these changes Aug 17, 2017

refactor(common): remove deprecated `NgFor`
BREAKING CHANGE: `NgFor` has been removed as it was deprecated since v4. Use `NgForOf` instead. This does not impact the use of`*ngFor` in your templates.

@ocombe ocombe force-pushed the ocombe:remove-ng-for branch from da580bb to 72b4f8d Aug 18, 2017

@ocombe ocombe requested a review from juleskremer Aug 18, 2017

@ocombe

This comment has been minimized.

Copy link
Contributor Author

commented Aug 18, 2017

@juleskremer I've renamed "NgFor" to "NgForOf" in the template syntax guide since NgFor has been removed from the code. I could have kept "NgFor" instead to not confuse people between "NgForOf" and "*ngFor". Or we could add a not that explains that one is the name of the directive, and one is the name of the attribute to use. What do you think?
If it seems good to you, you can add the merge label

@mary-poppins

This comment has been minimized.

Copy link

commented Aug 18, 2017

@vicb

This comment has been minimized.

Copy link
Contributor

commented Aug 18, 2017

I think that using NgFor in the docs would be less confusing ?
@chuckjaz WDYT ?

@chuckjaz

This comment has been minimized.

Copy link
Member

commented Aug 18, 2017

I believe referring to a deprecated type such as NgFor is more confusing especially if it is removed entirely as implied by this PR.

The rename to NgForOf made room for a possible NgForIn (iterate the keys of an object). Having such a directive would make it much more clear why NgFor is not the right name for the current directive but, unfortunately, NgForIn was not added leaving this a bit confusing.

@mhevery mhevery closed this in ec56760 Aug 22, 2017

@ocombe ocombe deleted the ocombe:remove-ng-for branch Aug 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.