nothingislost / obsidian-query-control

An experimental Obsidian plugin that adds controls to embedded queries
MIT License
278 stars 16 forks source link

Console error "Cannot read properties of undefined (reading 'file')" happening at random times throughout the day #19

Open GitMurf opened 1 year ago

GitMurf commented 1 year ago

I cannot figure out how to reproduce this but I see this several times a day at different random times. It seems to happen most consistently when I run a Templater script that creates a new file with something like this: let newFileObj = await this.app.vault.create(newFilePath, pageContent);

But I have no clue if that is the culprit or not. I also find these log errors randomly throughout the day when I go to check my console for something else.

image

GitMurf commented 1 year ago

@nothingislost I think I have gotten to the bottom of the cause of the errors and pinpointed it in the code. I don't quite know how to best fix it but this Loom video walks you through the problem as well as my hunch for the cause of it (search query rendered views seem to not be being cleaned up and duplicates created with broken property for parent property).

https://www.loom.com/share/1b5f1ad2a2d94bb7860a8ada8df09a9d

GitMurf commented 1 year ago

Specifically it is being caused by this (parent property Null when "views" aren't cleaned up):

https://github.com/nothingislost/obsidian-query-control/blob/93add4bb67e0b189fa2914ee25c9025018f52c81/src/search-renderer.ts#L54-L56

GitMurf commented 1 year ago

Here I logged it showing when the issue occurs the parent property on this.match is missing.

image

GitMurf commented 1 year ago

Again though, all of the details are included in my Loom video above.

GitMurf commented 1 year ago

So it looks like changing getFile() to the following stops the error but I think there is still a question of why when using the show more/less context button it seems to not cleanup old entries and they continue to multiply every time you press the button (memory leak).

getFile() {
    return isFifteenPlus ? this.match.parentDom.file : this.match.parent.file;
}

Based on the stack trace of the error it looks like this (screenshot below from Obsidian main app code) is where it is stemming from in the code. It seems to do with resolving links which is interesting. It also seems like in my further testing that these errors only arise when you have a search query result that has a [[link]] in it and then must be calling this resolveLinks on file change.

image

GitMurf commented 1 year ago

After further research and diving in, I believe the "duplicates" (memory leak) is being caused by this section where adding new components for markdown rendering the results and it does not cleanup the old components. So when you flip from less context to show more context, it leaves the old items behind... and vice versa. And as you continue to toggle the show more context it duplicates them every time which can grow pretty quickly.

https://github.com/nothingislost/obsidian-query-control/blob/93add4bb67e0b189fa2914ee25c9025018f52c81/src/main.ts#L457-L467

GitMurf commented 1 year ago

Ok @nothingislost here is code you can run directly in the console log to find the "leaked" (leftover) components and also remove them. Clearly this isn't the solution but hopefully it gives you what you need to identify the real problem and where the best place in the code of the plugin is to clean these up. The problem is that it isn't clear to me where/how you can match up old components to their new ones. So instead need to test whether components are connected to the DOM or not (but I have a feeling there are issues with this logic as well).

let allLeafChildren = app.workspace.activeLeaf.view._children;
allLeafChildren.forEach(eachLeafChild => {
    let childType = "";
    if(eachLeafChild.dom) {
        if(eachLeafChild.dom.el.classList.contains("mod-global-search")) {
            childType = "Global Search View";
            console.log("Leaf Child Component Type:", childType);
            const bRenderMd = eachLeafChild.dom.renderMarkdown;
            console.log(`${childType} renderMarkdown on:`, bRenderMd);
            const domEl = eachLeafChild.dom.el;
            console.log(`${childType} isConnected:`, domEl.isConnected);
            const rendComponent = eachLeafChild.dom.parent.renderComponent;
            let removeComponents = [];
            rendComponent._children.forEach(eachChild => {
                console.log(`${childType} child isConnected:`,eachChild.containerEl.isConnected);
                if(!eachChild.containerEl.isConnected) {
                    removeComponents.push(eachChild);
                }
            })
            let remCtr = 0;
            removeComponents.forEach(eachComp => {
                //console.log(eachComp);
                rendComponent.removeChild(eachComp);
                remCtr++;
            })
            if(remCtr > 0) { console.log(`Removed ${remCtr} ${childType} old stale components`); }
        } else if(eachLeafChild.dom.el.classList.contains("search-result-container")) {
            childType = "Embedded Search Query Component";
            console.log("Leaf Child Component Type:", childType);
            const bRenderMd = eachLeafChild.dom.renderMarkdown;
            console.log(`${childType} renderMarkdown on:`, bRenderMd);
            const domEl = eachLeafChild.dom.el;
            console.log(`${childType} isConnected:`, domEl.isConnected);
            const rendComponent = eachLeafChild.renderComponent;
            let removeComponents = [];
            rendComponent._children.forEach(eachChild => {
                console.log(`${childType} child isConnected:`,eachChild.containerEl.isConnected);
                if(!eachChild.containerEl.isConnected) {
                    removeComponents.push(eachChild);
                }
            })
            let remCtr = 0;
            removeComponents.forEach(eachComp => {
                //console.log(eachComp);
                rendComponent.removeChild(eachComp);
                remCtr++;
            })
            if(remCtr > 0) { console.log(`Removed ${remCtr} ${childType} old stale components`); }
        }
    } else if(eachLeafChild.backlinkDom) {
        childType = "Backlinks Section";
        console.log("Leaf Child Component Type:", childType);
        const bRenderMd = eachLeafChild.backlinkDom.renderMarkdown;
        console.log(`${childType} renderMarkdown on:`, bRenderMd);
        const domEl = eachLeafChild.backlinkDom.el;
        console.log(`${childType} isConnected:`, domEl.isConnected);
        const rendComponent = eachLeafChild.renderComponent;
        let removeComponents = [];
        rendComponent._children.forEach(eachChild => {
            console.log(`${childType} child isConnected:`,eachChild.containerEl.isConnected);
            if(!eachChild.containerEl.isConnected) {
                removeComponents.push(eachChild);
            }
        })
        let remCtr = 0;
        removeComponents.forEach(eachComp => {
            //console.log(eachComp);
            rendComponent.removeChild(eachComp);
            remCtr++;
        })
        if(remCtr > 0) { console.log(`Removed ${remCtr} ${childType} old stale components`); }
    } else if(eachLeafChild.type && eachLeafChild.view) {
        if(eachLeafChild.type === "preview") {
            childType = "Markdown Preview (Ignore)";
            console.log("Leaf Child Component Type:", childType);
        }
    } else if(eachLeafChild.containerEl) {
        if(eachLeafChild.containerEl.classList.contains("image-embed")) {
            childType = "Image Embed (Ignore)";
            console.log("Leaf Child Component Type:", childType);
        } else if(eachLeafChild.containerEl.classList.contains("markdown-embed-block")) {
            childType = "Markdown Embed Block (Ignore)";
            console.log("Leaf Child Component Type:", childType);
        }
    }
    if(!childType){console.error("Did not recognize Leaf Child Component:", eachLeafChild)}
})