-
Notifications
You must be signed in to change notification settings - Fork 6.7k
datepicker: watchers count #2613
Comments
If think that this is a problem that should be thought about, because I see some 'lag' while typing in the input box in my app where on one view I have two datepickers and a lot of other stuff. |
+1 Same for us :-( |
+1 |
1 similar comment
+1 |
+299 watchers for me! |
+7000 watchers. I use a table with date pickers. Bad bad idea apparently... |
+1, each datepicker adds ~300 watchers. |
I looked through the templates and none of them are using 1.3's BindOnce. I assume they should be considering it is a calendar and the dates shouldn't change too often? Thing is NG-IF is being use so should take care of most of the watchers but it isn't. Had to stop using this. |
+1 It's great problem. |
I was looking into this problem (because we have it too). One solution would be to do an ng-if on the popup. My thought was to have it just do ng-if="isOpen", but that doesn't work as it loses the data stored in the model then, for some reason. The other option is to create another variable that a user can pass in to say when they want it to be initialised. This would be bound to ng-if and would mean that once it is opened the first time, it is brought into the html and the watchers are assigned. This would help people who have multiple, as it ends up being a big problem. However, when trying to do it, I have 5 tests failing. I reckon these are because of the slight performance hit that you get when using ng-if meaning that the popup isn't ready for when it tries to click a date. I'll work on this a bit later tomorrow, but it's at least something that can be done. |
@YaManicKill See this pull request for the ng-if implementation: #1915 |
@RobJacobs Ah, yeah that's exactly what I was working on. Any plans to actually merge it in? |
+1 |
+1 |
There's simply too many watchers on the templates. Doing a quick look through of the popup.html, the ng-repeat that is building the day/week/cell grid, for each day displayed:
which adds up to 5 watches per cell in the grid. @cpaof You need to wrap the datepicker popup in an ng-if statement until #1915 get's merged. The plunker mentioned in the first post shows how to do that. |
@RobJacobs I decided to create a monkey patched version of the bower library in the end. My use case was the date picker to appear on focus of the input field where the suggested https://github.com/cpaof/bootstrap-bower for anyone that's interested. |
We are planning on tackling the performance issue with the datepicker once we release 0.13.0. |
Looking into this issue and looks like #2619 adds 40 watchers on the typical use case, even if no custom class was provided. |
I think we can continue in #3770 |
- Add new class manipulation helper directive that uses 1 watcher as opposed to watchers for each element with ng-class - Optimize disabled state - Optimize class binding Closes #2613 Closes #3451 Closes #3770 Closes #5065 BREAKING CHANGE: For those using custom templates, the changes result in necessary changes being made to the templates to match the new class changing syntax
I've been looking at ways to improve performance in my app. I've read that reducing number of unnecessary watchers is something that can help.
Using a simple watch counter I see that the number of watchers increases by 296 when a datepicker is introduced into the page. See this plunker for a demo. AFAIK this is due to the html template using ng-repeat to create the table of days.
Some questions:
The text was updated successfully, but these errors were encountered: