Closed Bug 1230299 Opened 4 years ago Closed 4 years ago
Removing "waiting for sources
..." and don't fetch sources twice
The initial message in the source listing is "Waiting for sources..." and it waits for LOAD_SOURCES and either shows sources or if it is empty shows "no sources". The problem is that once we are connected, when you reload the page, you already get all the sources as individual `newSource` notifications. So when you reload, we are actually fetching all the sources *twice*, and because we aren't using a declarative UI, it is even adding them to the UI twice, but the second time it adds it is basically a noop (but it still has to touch the DOM). I've noticed this before and it makes loading a lot of sources on the page slow. The *only* reason we need to call `loadSources()` when you reload is because we need to see if we get an empty array back. If there are no sources, we will never get any `newSource` notifications. But we show a loading message by default, so we have to know when to switch it to the "no sources" message. I don't think the loading message has much utility. I say we just remove it because it's causing a lot of complexity. Then it'll only call `loadSources` when the debugger is opened, but never again, because all subsequent sources will come in through `newSource` notifications.
Not sure if anyone has opinions about this, ni? fitzgen just to get someone else's opinion before I do it.
+1 as long as we don't get rid of the "no sources" message on documents without any JS.
Summary: Don't show a "waiting for sources..." message in source listing → Removing "waiting for sources..." and don't fetch sources twice
Definitely still going to show "No sources". I'm also go ahead and remove the extraneous `loadSources` call on tab navigation here.
Assignee: nobody → jlong
Status: NEW → ASSIGNED
Whoops forgot to update some of the tests. We no longer have a LOAD_SOURCES event because we don't know when to emit it now (because we don't explicitly fetch the sources when reloading). But that's fine because the `navigation` event comes in after all the `newSource` events have been fired so we can just wait on that.
New try push w/fixed tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=64a5dd44e263
Those tests had a bad parent commit. Rebased: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a8e21497c1d
You need to log in before you can comment on or make changes to this bug.