Merge pull request #3586 from ilandikov/refactor/dynamic-destination-path

refactor: reduce memory usage with dynamic destination path

Author: claremacrae

src/Scripting/TasksFile.ts

@@ -1,6 +1,5 @@
 import { type CachedMetadata, type FrontMatterCache, type Reference, getAllTags, parseFrontMatterTags } from 'obsidian';
-import type { Link } from '../Task/Link';
-import { LinkResolver } from '../Task/LinkResolver';
+import { Link } from '../Task/Link';
 
 export type OptionalTasksFile = TasksFile | undefined;
 
@@ -36,7 +35,7 @@ export class TasksFile {
     }
 
     private createLinks(obsidianRawLinks: Reference[] | undefined) {
-        return obsidianRawLinks?.map((link) => LinkResolver.getInstance().resolve(link, this.path)) ?? [];
+        return obsidianRawLinks?.map((link) => new Link(link, this.path)) ?? [];
     }
 
     /**

src/Task/Link.ts

@@ -1,20 +1,18 @@
 import type { Reference } from 'obsidian';
 import type { TasksFile } from '../Scripting/TasksFile';
+import { LinkResolver } from './LinkResolver';
 
 export class Link {
     private readonly rawLink: Reference;
     private readonly pathContainingLink: string;
-    private readonly _destinationPath: string | null;
 
     /**
      * @param {Reference} rawLink - The raw link from Obsidian cache.
      * @param {string} pathContainingLink - The path of the file where this link is located.
-     * @param {string | undefined} destinationPath - The path of the note being linked tio.
      */
-    constructor(rawLink: Reference, pathContainingLink: string, destinationPath?: string) {
+    constructor(rawLink: Reference, pathContainingLink: string) {
         this.rawLink = rawLink;
         this.pathContainingLink = pathContainingLink;
-        this._destinationPath = destinationPath ?? null;
     }
 
     /**
@@ -80,7 +78,7 @@ export class Link {
      * See {@link LinkResolver} docs for more info.
      */
     public get destinationPath(): string | null {
-        return this._destinationPath;
+        return LinkResolver.getInstance().getDestinationPath(this.rawLink, this.pathContainingLink) ?? null;
     }
 
     /**

src/Task/LinkResolver.ts

@@ -1,12 +1,11 @@
 import type { Reference } from 'obsidian';
-import { Link } from './Link';
 
 export type GetFirstLinkpathDestFn = (rawLink: Reference, sourcePath: string) => string | null;
 
 const defaultGetFirstLinkpathDestFn = (_rawLink: Reference, _sourcePath: string) => null;
 
 /**
- * An abstraction to populate {@link Link.destinationPath}.
+ * An abstraction to implement {@link Link.destinationPath}.
  *
  * See also:
  * - `src/main.ts` - search for `LinkResolver.getInstance()`
@@ -25,13 +24,8 @@ export class LinkResolver {
     public resetGetFirstLinkpathDestFn() {
         this.getFirstLinkpathDestFn = defaultGetFirstLinkpathDestFn;
     }
-
-    public resolve(rawLink: Reference, pathContainingLink: string) {
-        return new Link(
-            rawLink,
-            pathContainingLink,
-            this.getFirstLinkpathDestFn(rawLink, pathContainingLink) ?? undefined,
-        );
+    public getDestinationPath(rawLink: Reference, pathContainingLink: string) {
+        return this.getFirstLinkpathDestFn(rawLink, pathContainingLink) ?? undefined;
     }
 
     /**

src/Task/ListItem.ts

@@ -3,8 +3,7 @@ import type { TasksFile } from '../Scripting/TasksFile';
 import type { Task } from './Task';
 import type { TaskLocation } from './TaskLocation';
 import { TaskRegularExpressions } from './TaskRegularExpressions';
-import type { Link } from './Link';
-import { LinkResolver } from './LinkResolver';
+import { Link } from './Link';
 
 export class ListItem {
     // The original line read from file.
@@ -211,7 +210,7 @@ export class ListItem {
     public get outlinks(): Readonly<Link[]> {
         return this.rawLinksInFileBody
             .filter((link) => link.position.start.line === this.lineNumber)
-            .map((link) => LinkResolver.getInstance().resolve(link, this.file.path));
+            .map((link) => new Link(link, this.file.path));
     }
 
     /**

tests/Scripting/TaskProperties.test.ts

@@ -3,7 +3,6 @@
  */
 
 import moment from 'moment';
-import type { Reference } from 'obsidian';
 import { Status } from '../../src/Statuses/Status';
 
 import { TaskBuilder } from '../TestingTools/TaskBuilder';
@@ -159,9 +158,7 @@ describe('task', () => {
 
     it('links', () => {
         // This is getting annoying, having to do this repeatedly.
-        LinkResolver.getInstance().setGetFirstLinkpathDestFn((rawLink: Reference, sourcePath: string) =>
-            getFirstLinkpathDest(rawLink, sourcePath),
-        );
+        LinkResolver.getInstance().setGetFirstLinkpathDestFn(getFirstLinkpathDest);
 
         const tasks = readTasksFromSimulatedFile(links_everywhere);
         verifyFieldDataFromTasksForReferenceDocs(tasks, [

tests/Task/Link.test.ts

@@ -1,4 +1,3 @@
-import type { Reference } from 'obsidian';
 import { TasksFile } from '../../src/Scripting/TasksFile';
 import { Link } from '../../src/Task/Link';
 import internal_heading_links from '../Obsidian/__test_data__/internal_heading_links.json';
@@ -7,7 +6,6 @@ import link_in_task_markdown_link from '../Obsidian/__test_data__/link_in_task_m
 import link_in_task_wikilink from '../Obsidian/__test_data__/link_in_task_wikilink.json';
 import link_is_broken from '../Obsidian/__test_data__/link_is_broken.json';
 
-import link_in_file_body from '../Obsidian/__test_data__/link_in_file_body.json';
 import links_everywhere from '../Obsidian/__test_data__/links_everywhere.json';
 import { allCacheSampleData } from '../Obsidian/AllCacheSampleData';
 import type { SimulatedFile } from '../Obsidian/SimulatedFile';
@@ -20,7 +18,11 @@ import { getFirstLinkpathDest, getFirstLinkpathDestFromData } from '../__mocks__
 function getLink(data: any, index: number) {
     const rawLink = data.cachedMetadata.links[index];
     const destinationPath = getFirstLinkpathDestFromData(data, rawLink);
-    return new Link(rawLink, data.filePath, destinationPath);
+
+    const resolver = LinkResolver.getInstance();
+    resolver.setGetFirstLinkpathDestFn(() => destinationPath);
+
+    return new Link(rawLink, data.filePath);
 }
 
 describe('linkClass', () => {
@@ -261,31 +263,6 @@ describe('linkClass', () => {
         // []() and [alias]() are not detected by the obsidian parser as a link
     });
 
-    describe('destinationPath tests', () => {
-        it('should accept and return destinationPath', () => {
-            const data = link_in_file_body;
-            const rawLink = data.cachedMetadata.links[0];
-            expect(rawLink.original).toEqual('[[yaml_tags_is_empty]]');
-            expect(rawLink.link).toEqual('yaml_tags_is_empty');
-
-            const destinationPath = 'Test Data/yaml_tags_is_empty.md';
-            const link = new Link(rawLink, data.filePath, destinationPath);
-
-            expect(link.destinationPath).toEqual(destinationPath);
-        });
-
-        it('should return null path if destinationPath not supplied', () => {
-            const data = link_in_file_body;
-            const rawLink = data.cachedMetadata.links[0];
-            expect(rawLink.original).toEqual('[[yaml_tags_is_empty]]');
-            expect(rawLink.link).toEqual('yaml_tags_is_empty');
-
-            const link = new Link(rawLink, data.filePath);
-
-            expect(link.destinationPath).toBeNull();
-        });
-    });
-
     describe('linksTo() tests', () => {
         it('matches filenames', () => {
             const link = getLink(links_everywhere, 0);
@@ -329,9 +306,7 @@ describe('linkClass', () => {
 
 describe('visualise links', () => {
     beforeAll(() => {
-        LinkResolver.getInstance().setGetFirstLinkpathDestFn((rawLink: Reference, sourcePath: string) => {
-            return getFirstLinkpathDest(rawLink, sourcePath);
-        });
+        LinkResolver.getInstance().setGetFirstLinkpathDestFn(getFirstLinkpathDest);
     });
 
     afterAll(() => {

tests/Task/LinkResolver.test.ts

@@ -1,6 +1,7 @@
 import type { Reference } from 'obsidian';
 import link_in_file_body from '../Obsidian/__test_data__/link_in_file_body.json';
 import { LinkResolver } from '../../src/Task/LinkResolver';
+import { Link } from '../../src/Task/Link';
 
 describe('LinkResolver', () => {
     let rawLink: Reference;
@@ -10,40 +11,49 @@ describe('LinkResolver', () => {
     });
 
     it('should resolve a link via local instance', () => {
-        const resolver = new LinkResolver();
-        const link = resolver.resolve(rawLink, link_in_file_body.filePath);
+        const link = new Link(rawLink, link_in_file_body.filePath);
 
         expect(link.originalMarkdown).toEqual('[[yaml_tags_is_empty]]');
         expect(link.destinationPath).toBeNull();
     });
 
     it('should resolve a link via global instance', () => {
-        const link = LinkResolver.getInstance().resolve(rawLink, link_in_file_body.filePath);
+        const link = new Link(rawLink, link_in_file_body.filePath);
 
         expect(link.originalMarkdown).toEqual('[[yaml_tags_is_empty]]');
         expect(link.destinationPath).toBeNull();
     });
 
     it('should allow a function to be supplied, to find the destination of a link', () => {
-        const resolver = new LinkResolver();
-        resolver.setGetFirstLinkpathDestFn((_rawLink: Reference, _sourcePath: string) => 'Hello World.md');
+        const resolver = LinkResolver.getInstance();
+        resolver.setGetFirstLinkpathDestFn(() => 'Hello World.md');
 
-        const link = resolver.resolve(rawLink, link_in_file_body.filePath);
+        const link = new Link(rawLink, link_in_file_body.filePath);
         expect(link.destinationPath).toEqual('Hello World.md');
     });
 
     it('should allow the global instance to be reset', () => {
         const globalInstance = LinkResolver.getInstance();
-        globalInstance.setGetFirstLinkpathDestFn(
-            (_rawLink: Reference, _sourcePath: string) => 'From Global Instance.md',
-        );
+        globalInstance.setGetFirstLinkpathDestFn(() => 'From Global Instance.md');
 
-        const link1 = globalInstance.resolve(rawLink, link_in_file_body.filePath);
+        const link1 = new Link(rawLink, link_in_file_body.filePath);
         expect(link1.destinationPath).toEqual('From Global Instance.md');
 
         globalInstance.resetGetFirstLinkpathDestFn();
 
-        const link2 = globalInstance.resolve(rawLink, link_in_file_body.filePath);
+        const link2 = new Link(rawLink, link_in_file_body.filePath);
         expect(link2.destinationPath).toBeNull();
     });
+
+    it('resetting global instance affects pre-existing links', () => {
+        const globalInstance = LinkResolver.getInstance();
+        globalInstance.setGetFirstLinkpathDestFn(() => 'From Global Instance.md');
+
+        const link1 = new Link(rawLink, link_in_file_body.filePath);
+        expect(link1.destinationPath).toEqual('From Global Instance.md');
+
+        globalInstance.resetGetFirstLinkpathDestFn();
+
+        expect(link1.destinationPath).toBeNull();
+    });
 });