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

feat(ngBindOnceDirs.js): implementation of simple bindOnce directives #6284

Closed
wants to merge 0 commits into from

Conversation

joshkurz
Copy link
Contributor

I have not seen or heard anything from the core team regarding the bindOnce issue, but I bet you guys are working on something, regarding the amount of +1s #5408 has received. But since I have not seen anything I thought someone would like this.

This is a super simple implementation which introduces many useful directives into the core with very little code. It has almost full parity with the popular bindonce library, in terms of the amount of directives it offers. It also works with promises, $timeouts, and any other values that are set after the initial digest.

I did not want to create all the Docs for each directive just yet. Wanted to get some feedback on whether or not this type of functionality would be merged in or not?

@mary-poppins
Copy link

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#6284)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@caitp
Copy link
Contributor

caitp commented Feb 17, 2014

There's been a little bit of talk on how to do this "smartly" for Angular 2.0, which it might be possible to backport into 1.x, although I'm not positive there. It comes up a tiny bit in here, I think the goal is to have a consistent and sensible design, rather than adding a bunch more directives, so that this also works for text node interpolation and stuff.

So, no guarantees about this PR landing, but if you want to pay attention to what's happening in the design, we can probably figure out a way to get some of that into Angular 1.x in the future

@joshkurz
Copy link
Contributor Author

I understand about not wanting to add in more directives to suite these needs as it means more overall maintenance. I like the idea of using {{:bind}} alongside of the regular style {{bind}}. I also like the idea of having just an attribute flag on a given directive's restriction that determines if it's bind once. Those are my favorite ideas so far.

What about object.observe? wouldn't that eliminate the need for custom watchers all together and get rid of the $digest cycle? I thought that was the direction Angular2.0 was heading in.

But anyways, I figured there would be push back to this PR, so no feelings will be hurt if its closed. I just wanted to get a simple idea out there to solve this issue today, and see what else people were thinking about.

@Pasvaz
Copy link

Pasvaz commented Feb 19, 2014

I'm the author of bindonce and I have to agree with @caitp, I think Angular should follow a different and more structural approach to this problem, bindonce already works in a similar way as this PR but it's faster, less costy in term of watchers, ready for production, well known and tested on the field by many websites and right now, it covers almost every ng-directive with the equivalent bo-directive so why we should duplicate the efforts trying to replicate it in Angular?
If you want to get rid of watchers using directives you should definitely try angular-bindonce

@mary-poppins
Copy link

I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let us know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

@joshkurz joshkurz added cla: no and removed cla: yes labels Feb 19, 2014
@ProLoser
Copy link
Contributor

Please check out #6354 for a better alternative proposal to bind-once

@caitp
Copy link
Contributor

caitp commented Feb 19, 2014

@ProLoser please comment on the Angular 2.0 design docs re: your ideas for bind-once/bind-sometimes functionality. That would be a much better way for you to get your ideas considered


Nevermind, I see that you did.

@joshkurz
Copy link
Contributor Author

@Pasvaz not sure why you think your implementation is faster... Do you have any evidence to back up your claims?

Here is a jsperf of bindonce vs this PR vs pure angular http://jsperf.com/ngbindonce

Looks like this PR is faster to me.

@Pasvaz
Copy link

Pasvaz commented Feb 20, 2014

The main difference is that this PR creates a watcher for each directive/attribute, it removes them later but until then, it keeps open the same amount of watchers as angulard does, while bindonce uses only one watcher for each entity no matters how much children it has, if bindonce is inside a repeater it doesn't uses watchers at all. So, until you don't make the data static, bindonce keeps an inferior number of watchers, this is what I meant by faster, not the plain render.

When the number of watchers is the same this code of course is faster, it is very much simpler: at the moment it does everything mosty using attributes and it doesn't deal with interpolation, object evaluation, flow control, cross browser issues.
The result of a bb-one-* directive is different from the respective ng-* directive while bo-* directives try to be totally equivalent to the ng-* directives, for istance you can do bo-class="{'red':isRed,'white':isWhite}" and expect it to work as ng-class does.

Please don't get me wrong, I'm not saying this code is not good, it is absolutely good (it uses the same principles and approaches of bindonce) and when it will deal with the above mentioned points it will be equivalent to bindonce, that's what I meant when I said efforts duplication.

@joshkurz joshkurz added cla: no and removed cla: yes labels Mar 2, 2014
@joshkurz joshkurz added cla: no and removed cla: yes labels Mar 18, 2014
@btford btford added this to the 1.3.0 milestone Mar 27, 2014
@joshkurz joshkurz added cla: no and removed cla: yes labels Mar 31, 2014
@joshkurz joshkurz closed this Apr 2, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants