Markers panels stuck/flashy

[jrieken]

• work on Enable strictPropertyInitialization #78168
• use the "TS - Strict Init" task
• collapse all in the markers panel
• expand and select a file from the markers panel
• make changes, save
• try to expand another item
• 🐛 nothing happens, the selection flashes weirdly, it takes a while to recover

Screen Capture 05.08.19, 10.37.56.mp4.zip

[jrieken]

/cc @joaomoreno

[joaomoreno]

Disco mode!

[jrieken]

👯‍♂

[sandy081]

@joaomoreno I guess this flickering is coming from the tree. This happens when tree model is updated. I created a simple sample to repro this. Here is the extension that generates same errors continuously. When you hover on the first problem, it keeps flickering while the model gets updated.

'use strict';

import * as vscode from 'vscode';

export function timeout(millis: number): Promise<void> | Promise<void> {
    return new Promise((resolve, reject) => {
        const handle = setTimeout(resolve, millis);
    });
}

async function createDiagnostics(diagnosticsCollection: vscode.DiagnosticCollection) {
    for (let i = 0; i < 10000; i++) {
        const diagnostics: vscode.Diagnostic[] = [];
        const diagnostic1 = new vscode.Diagnostic(
            new vscode.Range(new vscode.Position(10, 1), new vscode.Position(10, 10)), 'some error', vscode.DiagnosticSeverity.Error);
        diagnostic1.code = 'test';
        const diagnostic2 = new vscode.Diagnostic(
            new vscode.Range(new vscode.Position(10, 1), new vscode.Position(10, 10)), 'some error', vscode.DiagnosticSeverity.Error);
        diagnostic2.code = 'test';
        const diagnostic3 = new vscode.Diagnostic(
            new vscode.Range(new vscode.Position(10, 1), new vscode.Position(10, 10)), 'some error', vscode.DiagnosticSeverity.Error);
        diagnostic3.code = 'test';
        const diagnostic4 = new vscode.Diagnostic(
            new vscode.Range(new vscode.Position(10, 1), new vscode.Position(10, 10)), 'some error', vscode.DiagnosticSeverity.Error);
        diagnostic4.code = 'test';
        diagnostics.push(diagnostic1, diagnostic2, diagnostic3, diagnostic4);
        diagnosticsCollection.set(vscode.Uri.file('abc'), diagnostics);
        await timeout(50);
    }
}

export function activate(context: vscode.ExtensionContext) {
    const diagnosticsCollection = vscode.languages.createDiagnosticCollection('test');
    createDiagnostics(diagnosticsCollection);
};

export function deactivate() {
}

Let me know if something has to be done as the tree consumer.

[joaomoreno]

@sandy081, a bit more work is necessary to get that sample to run:

• There was a syntax error in the snippet, I've fixed it
• The file path points to /abc. I've changed it to point to $WORKSPACEROOT/abc
• One needs to have a file called abc in the workspace to repro, with at least 11 lines of content
• Also activationEvents needs to be *

---

Nothing is stuck. This is the mouse :hover CSS selector which flickers when the DOM node gets replaced in the DOM, which happens because of the updateChildren setChildren call the markers panel makes.

There's not much I can do here, unless using something like DOM tree diffing like what React does.

As a tree user you can minimize the updates to the tree by only updating tree nodes that actually have changed. Currently the markers panel fully replaces the whole tree model on any change. If it knew which file had its markers modified, it could simply update that file only.

[sandy081]

@joaomoreno I only see setChildren method on the ObjectTree. Are there methods to add or remove children?

[joaomoreno]

Yeah, setChildren. You can call it on any node. Right now we're always calling it on the root.

[sandy081]

@joaomoreno I mean, If there are new resources are added or removed on root, I do not see an update method in tree to update root's children. If there is one, I would use it update (add/remove) children on root.

Right now I prepared a fix that that If a resource is updated, I am calling setChildren on resource otherwise I am resetting the root by calling setChildren on root.

This fix prevents flickering for the given example. But the flickering can still happen if there are new resources added or removed as it updates the complete tree.

[joaomoreno]

Sure, that's fine. You can only fix that by going one level deeper in the abstraction stack and use the IndexTree, which gives you full mutation control on the tree with the .splice(location, deleteCount, tree nodes to insert) API. But I would say that's overkill for you.

Another idea here would be to create a React-like tree diff algorithm for minimizing index updates on object trees. But that's a whole different scale of work.

[sandy081]

Above fix had multiple improvements

1. I grouped multiple URI changes into single render
2. I separated URI updates with additions and removals.

With this I already see the problems view is smooth with strict property initialization task. So, I would close this.

[joaomoreno]

Very cool! 👏