-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
queryParams + tracked performance issue #18715
Comments
I've noticed this issue as well. Exactly the same behavior you described.. |
Hmm, since it isn't completely obvious in a smaller app scenario I'm not sure how to dig into this. Does someone have time to make a reproduction that we could use (with a quick bulleted list of steps to follow to see the issue)? |
In our app, setting |
I use @Tracked query params here but don't see slowness. I had to create queryParamsObj to pass the query params to components see: https://github.com/broerse/ember-cli-blog/blob/master/app/controllers/posts.js#L8 |
faced this issue as well, everything hangs here |
- Query params are technically owned (managed) by the ember framework so we should be using set to notify them to update - https://discuss.emberjs.com/t/query-params-tracked/17467/21 - emberjs/ember.js#18715
- Query params are technically owned (managed) by the ember framework so we should be using set to notify them to update - https://discuss.emberjs.com/t/query-params-tracked/17467/21 - emberjs/ember.js#18715 Co-authored-by: Kevin Hinterlong <[email protected]>
We noticed this happening too. The performance became massively degraded when we added a second |
Interestingly I also don't see this issue even though I had to add a bunch of |
I was also unsuccessful in creating a limited reproduction for this, but I've created a working reproduction with a minimal change set in this PR ilios/common#1269 (ember 3.17) Steps to tests:
Once authenticated you'll see our dashboard.
|
I was bitten by this yesterday, spent quite a time trying to understand what was going on. Ember 3.14.3, Firefox and Chrome. |
Running into this as well, managed to find a workaround using a tracked backing property and getter/setter: queryParams = ['prop']
@tracked _prop = 'default'
get prop() {
return this._prop
}
set prop(value) {
this._prop = value
} |
Running into this problem as well. Props to @njoyard for the workaround. Using Ember 3.14.3. |
We are also running into this, @njoyard workaround fixed the issue. Using Ember 3.16.6 |
Same here:
I have noticed considerable slowness. But I understood that there was something very wrong when I saw hundreds of |
Noticing similar issues with lots of warning messages like this. |
+1 Ember 3.19, tested in Chrome (84.0.4147.105) and Firefox (78.0.2), only occurs in Firefox |
@Samsinite we don't know for sure since |
@pzuraq sorry, I should have worded that better. With insight and knowledge of the changes in that PR, and what they affected, do you think the fix could have fixed some or most of the performance regressions reported in this issue? |
Yes, I believe so. There was one such performance issue that we found in our app that the fix resolved, and I think it's likely that it will fix other performance issues as well. |
Yes, I believe that #19138 fixed this. Closing for now (can reopen if we get a repro that shows it is still an issue).. |
Yes!! My issue and pretty broad reproduction in #18715 (comment) is resolved in Ember 3.22.0. After updating I no longer see the delays and crashing I was seeing before. 🎉!! Thanks! |
We noticed some performance issues when decorating query param properties with
@tracked
and using it in a template. Removing tracked (and using set instead) fixes the problem. Removing the property name from the queryParams array also fixes the issue.I'll try to create a reproduction project, but it's not noticeable in clean / small projects. I'll investigate more when I find some time.
When searching the discord I discovered there are more people who have this problem, but it seems no issue was logged yet.
Relevant Discord discussions:
https://discordapp.com/channels/480462759797063690/608346628163633192/638740754650628098
The text was updated successfully, but these errors were encountered: