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

Moving aliasing special properties of ngRepeat from ngInit to ngRepeat #5623

Closed
vucalur opened this issue Jan 3, 2014 · 11 comments
Closed

Comments

@vucalur
Copy link
Contributor

vucalur commented Jan 3, 2014

Currently the only one appropriate use of ngInit is aliasing $index and other special properties of ngRepeat.

I'd like suggest moving this functionality to ngRepeat.
It could look like:

<li ng-repeat="city in cities track by $index, $index as cityIndex, $middle as isCityMiddle">

Pros:

  • Aliasing is needed for ngRepeat, not ngInit.

Cons:

  • Will add more functionality to ngRepeat.
    Right now it's one of the biggest directives if LoC is concerned.

Also, just to make writing hello-worlds on plunker a little tougher, removing ngInit entirely could be considered.
Yes, I know I'm crazy, but this could be only done if there are really few hackish solutions which employ ngInit.
Just wanted to know that you guys think of this.

@caitp
Copy link
Contributor

caitp commented Jan 3, 2014

I don't really see the benefit, it would be a breaking change and not a particularly useful one.

I'm all for your proposed documentation changes, but this seems like it's taking it a bit far

@ghost ghost assigned tbosch Jan 4, 2014
@tbosch
Copy link
Contributor

tbosch commented Jan 4, 2014

Actually it's a nice idea. Would you like to create a PR for this?

@caitp
Copy link
Contributor

caitp commented Jan 4, 2014

I can see how it could be useful, but there are downsides to it:

Even without replacing ngInit, it makes the ngRepeat expression more complicated, which is a downside, even if it looks nice.

Removing ngInit entirely is a breaking change with no particularly good reason (it's a mistake to assume ngRepeat is the only use of ngInit) --- without removing ngInit, it's duplicating code and making ngRepeat more complicated without any real tangible benefit.

So I'm just not seeing it. But, if that's what people want to do then okay --- I just don't see how it's a worthwhile thing, I'd be much happier about making a comment about ngInit in the ngRepeat docs.

@gsklee
Copy link
Contributor

gsklee commented Jan 5, 2014

Wow, when was that tagline being added? It's certainly not there at least prior to v1.1.5. I've been using ngInit to bootstrap the app with initial data from the server to save requests, which is a widely known optimization technique. So it's definitely not only about aliasing special properties in ngRepeat, the name of the directive has clearly suggested that - that's why it's called ngInit, not ngRepeatAlias. I'm very interested to know the reasoning behind adding that tagline (anyone knows the issue number?)

Aside from that - I agree with @caitp that this will further complicate the syntax and implementation of ngRepeat and, basing on @IgorMinar's comment to the initially proposed change in #5557 I guess this probably will not gonna work.

@gsklee
Copy link
Contributor

gsklee commented Jan 5, 2014

Found the issue that added the tagline: #4366

@vucalur
Copy link
Contributor Author

vucalur commented Jan 5, 2014

You guys are right. Thanks for your constructive opinions.

  • Removing ngInit - certainly not, 'cause there may be dozens of unusual usages. As for the docs, perhaps suggesting that ngInit is excused only when aliasing ngRepeat variables isn't such a good idea. As @gsklee has mentioned, there are other 'valid' usages too.

My updated stance towards adding the feature directly to ngRepeat:

  • Against, since it would add unnecessary complexity to ngRepeat.

The only benefit it would bring is not introducing excessive scope variable (depending on a concept: aliasing vs. renaming) - in the example we have 2: cityIndex and $index.

@IgorMinar
Copy link
Contributor

we should solve this in 1.3.x. I'd like to see less magic in ngRepeat but we shouldn't make the syntax too complicated.

@gsklee
Copy link
Contributor

gsklee commented Jan 7, 2014

@IgorMinar anything concrete on how a less magical ngRepeat should look like?

@btford btford removed the gh: issue label Aug 20, 2014
@petebacondarwin
Copy link
Member

I don't think we should be changing or removing ngInit. It is there and it does a few jobs well, even if it can be abused. Moreover, I now agree with @gsklee that we should moderate the tag line in the docs to something not quite so severe.

Regarding ngRepeat, I am torn...

@petebacondarwin petebacondarwin modified the milestones: 1.5.x - migration-facilitation, 1.3.x - superluminal-nudge Sep 9, 2015
@petebacondarwin
Copy link
Member

Adding these extra clauses to the ngRepeat expression is adding too much burden to an already overburdened syntax. For now I think we should stick with ngInit and make the note about it in the docs a bit clearer.

@petebacondarwin
Copy link
Member

Changed the note in the ngInit docs: 010d9b6

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants