batches: Fix multiple tabs racing for URL query state #40857
Conversation
After switching back to wildcard tabs, it was forgotten that before those components would not render at the same time. This causes two issues: - Going to the bulp operations tab doesn't reload so it's unclear where the bulk operation went (noticed that while debugging) - The Bulk operations, Executions and Changesets tabs will all three race for the `visible` (and other, likely) URL query param and keep updating it, leading to an infinite loop of updates until Chrome prevents the loop.
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 02fb2e8...316ecbe.
|
LawnGnome
left a comment
LawnGnome
left a comment
There was a problem hiding this comment.
Had to dig into the Wildcard implementation a bit to verify that this actually does what we want, but seems good to me.
|
We have some tests that cover these pages, technically, but I think they currently don't have data in all the tabs.. Maybe it would have! I can try that later. |
courier-new
left a comment
courier-new
left a comment
There was a problem hiding this comment.
Thanks so much, Erik! This is exactly why I asked for another set of eyes on that previous refactor... 🙈 Guess I should have asked more aggressively, heh. This shouldn't affect that bug, no! 🙌
|
Re:tests, do our e2e tests click the "show more" button/can we mock that somehow? That'd be cool if so! I haven't been able to get the e2e tests to work since I upgraded to the M2 MacBook though so good luck. 😅 |
|
Nice, that sounds like fun! 😢 |
After switching back to wildcard tabs, it was forgotten that before those components would not render at the same time. This causes two issues: - Going to the bulp operations tab doesn't reload so it's unclear where the bulk operation went (noticed that while debugging) - The Bulk operations, Executions and Changesets tabs will all three race for the `visible` (and other, likely) URL query param and keep updating it, leading to an infinite loop of updates until Chrome prevents the loop.
After switching back to wildcard tabs, it was forgotten that before those components would not render at the same time.
https://github.com/sourcegraph/sourcegraph/pull/40110 migrated to using the wildcard components directly, but forgot to port over the lazy parameter: https://github.com/sourcegraph/sourcegraph/pull/40110/files#diff-733709eadc832fc5f74d1eb476d37c6ae449569239cc0b8653f550cac409d1d1L127
I think this doesn't affect the other stuff that you wanted to address in that PR, correct @courier-new? At least I don't see why we couldn't use lazy with that new tabbing approach.
This missing parameter causes two issues:
visible(and other, likely) URL query param and keep updating it, leading to an infinite loop of updates, freezing up the page.Closes https://github.com/sourcegraph/customer/issues/1262
Test plan
Verified that the scenario described in https://github.com/sourcegraph/customer/issues/1262 doesn't happen anymore with this change.
App preview:
Check out the client app preview documentation to learn more.