Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion .jules/bolt.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 2024-05-24 - [Avoid `forEach` overhead in recursive codebase methods]

**Learning:** In highly recursive file and directory scanning processes (e.g., generating file tree structures), the repeated array `.forEach` calls generate unnecessary function closures and allocations which can slow down applications significantly. This overhead becomes apparent when browsing large nested markdown folders.
**Action:** Replace `Array.prototype.forEach` with `for...of` statements in code paths related to file/folder traversal or recursive processes (like `FolderBrowserService.js` `getAllFiles` and `countFiles`). This maintains readability while avoiding the closure creation overhead, and should be the preferred pattern when processing unbounded or deep file trees.
**Action:** Replace `Array.prototype.forEach` with `for...of` statements in code paths related to file/folder traversal or recursive processes (like `FolderBrowserService.js` `getAllFiles` and `countFiles`). This maintains readability while avoiding the closure creation overhead, and should be the preferred pattern when processing unbounded or deep file trees.

## 2024-05-25 - [Optimize recursive DOM rendering with DocumentFragment and for...of]

**Learning:** In recursive UI rendering processes (e.g., `renderFileTree` or `populateLocationDropdown`), using `.forEach` leads to excessive closure allocations. Appending elements directly to the live DOM inside recursive loops causes severe layout thrashing. The combination of both leads to unacceptable UI lockup during deep folder expansion.
**Action:** Always combine `for...of` loops with `document.createDocumentFragment()` to batch DOM insertions before appending them to the container in recursive Vanilla JS DOM manipulation. This prevents both closure allocation bottlenecks and synchronous layout thrashing.
82 changes: 61 additions & 21 deletions script.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,16 @@ function generateSlug(text, seen = headingSlugMap) {
.replace(/^-+|-+$/g, '');

// Fallback for empty result
if (!slug) slug = 'section';
if (!slug) {
slug = 'section';
}

// Handle duplicates (header, header-1, header-2)
const baseSlug = slug;
const count = seen.get(baseSlug) || 0;
if (count > 0) slug = `${baseSlug}-${count}`;
if (count > 0) {
slug = `${baseSlug}-${count}`;
}
seen.set(baseSlug, count + 1);

return slug;
Expand Down Expand Up @@ -122,10 +126,14 @@ const AnchorNavigation = {
*/
handleClick(event) {
const link = event.target.closest('a[href^="#"]');
if (!link) return;
if (!link) {
return;
}

// Ignore external links
if (link.hostname && link.hostname !== window.location.hostname) return;
if (link.hostname && link.hostname !== window.location.hostname) {
return;
}

event.preventDefault();

Expand Down Expand Up @@ -166,7 +174,9 @@ const AnchorNavigation = {
}

const target = this.resolveTarget(hash);
if (!target) return;
if (!target) {
return;
}

target.scrollIntoView({
behavior: smooth ? 'smooth' : 'instant',
Expand All @@ -192,7 +202,9 @@ const AnchorNavigation = {
* @returns {Element|null} Target element or null
*/
resolveTarget(hash) {
if (!hash) return null;
if (!hash) {
return null;
}

// Decode URI components safely
let decodedHash;
Expand All @@ -204,21 +216,29 @@ const AnchorNavigation = {

// Priority 1: Exact ID (SCOPED to container)
let target = this.container.querySelector(`#${CSS.escape(decodedHash)}`);
if (target) return target;
if (target) {
return target;
}

// Priority 2: Normalized ID with our custom replacements (C++ β†’ cpp)
const normalized = this.normalizeHash(decodedHash);
target = this.container.querySelector(`#${CSS.escape(normalized)}`);
if (target) return target;
if (target) {
return target;
}

// Priority 3: GitHub-style normalization (C++ β†’ c, keeps double hyphens)
const githubStyle = this.normalizeHashGitHub(decodedHash);
target = this.container.querySelector(`#${CSS.escape(githubStyle)}`);
if (target) return target;
if (target) {
return target;
}

// Priority 4: Try all headings for fuzzy match (last resort)
const fuzzyTarget = this.fuzzyMatchHeading(decodedHash);
if (fuzzyTarget) return fuzzyTarget;
if (fuzzyTarget) {
return fuzzyTarget;
}

console.warn(`[AnchorNav] Target not found: "${hash}"`);
return null;
Expand Down Expand Up @@ -507,7 +527,9 @@ window.toggleSupportRegion = toggleSupportRegion;
*/
function renderSupportWidget(region) {
const container = document.getElementById('support-widget');
if (!container) return;
if (!container) {
return;
}

const config =
region === 'india'
Expand Down Expand Up @@ -854,7 +876,7 @@ function configureMarkedExtensions() {
const text = token.tokens ? this.parser.parseInline(token.tokens) : token.text;

// Defensive: only catch truly broken cases (null/undefined)
if (href == null) {
if (href === null || href === undefined) {
return `<a>${text}</a>`;
}

Expand Down Expand Up @@ -2317,19 +2339,30 @@ Wrap up your thoughts and include a call to action.

// Recursively add directories
function addDirectories(items, path = '', indent = 0) {
items.forEach(item => {
// ⚑ Bolt Optimization: Use DocumentFragment to batch DOM insertions
const fragment = document.createDocumentFragment();

// ⚑ Bolt Optimization: Use for...of instead of .forEach to avoid closure allocation
for (const item of items) {
if (item.type === 'directory') {
const fullPath = path ? `${path}/${item.name}` : item.name;
const option = document.createElement('option');
option.value = fullPath;
option.textContent = `${' '.repeat(indent)}πŸ“ ${item.name}`;
newFileLocationSelect.appendChild(option);
fragment.appendChild(option);

if (item.children) {
// Append children directly to the select element for simplicity,
// or we could return a fragment. Given the recursive nature,
// appending the current fragment then calling recursively is easiest.
newFileLocationSelect.appendChild(fragment);
addDirectories(item.children, fullPath, indent + 1);
}
}
});
}

// Append any remaining items in the fragment
newFileLocationSelect.appendChild(fragment);
Comment on lines 2341 to +2365
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Potential issue: DocumentFragment is appended mid-loop before all items are added.

The current implementation appends the fragment to newFileLocationSelect on line 2358 before recursing, then appends it again at line 2365. This pattern has issues:

  1. After appendChild(fragment), the fragment becomes empty (its children move to the DOM)
  2. The recursive call then creates new options that won't be in the original fragment
  3. The final appendChild(fragment) on line 2365 only appends items added after the mid-loop append

This may cause directories without children to appear after their sibling directories that have children, breaking the expected hierarchical order.

πŸ› Proposed fix: Append fragment only once at the end of each recursive call
     function addDirectories(items, path = '', indent = 0) {
       // ⚑ Bolt Optimization: Use DocumentFragment to batch DOM insertions
       const fragment = document.createDocumentFragment();

       // ⚑ Bolt Optimization: Use for...of instead of .forEach to avoid closure allocation
       for (const item of items) {
         if (item.type === 'directory') {
           const fullPath = path ? `${path}/${item.name}` : item.name;
           const option = document.createElement('option');
           option.value = fullPath;
           option.textContent = `${'  '.repeat(indent)}πŸ“ ${item.name}`;
           fragment.appendChild(option);
-
-          if (item.children) {
-            // Append children directly to the select element for simplicity,
-            // or we could return a fragment. Given the recursive nature,
-            // appending the current fragment then calling recursively is easiest.
-            newFileLocationSelect.appendChild(fragment);
-            addDirectories(item.children, fullPath, indent + 1);
-          }
         }
       }
-
-      // Append any remaining items in the fragment
       newFileLocationSelect.appendChild(fragment);
+
+      // Recurse into children after appending current level
+      for (const item of items) {
+        if (item.type === 'directory' && item.children) {
+          const fullPath = path ? `${path}/${item.name}` : item.name;
+          addDirectories(item.children, fullPath, indent + 1);
+        }
+      }
     }
πŸ€– Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@script.js` around lines 2341 - 2365, The fragment is being appended mid-loop
which empties it before recursion; in addDirectories, remove the in-loop
newFileLocationSelect.appendChild(fragment) so you only append the fragment once
after the for...of loop, and let recursive calls manage their own fragments
(i.e., keep addDirectories recursive but ensure each invocation appends its own
fragment to newFileLocationSelect at the end); this preserves the intended
hierarchical order of options created by addDirectories, option, and fragment
without double-appending or losing nodes.

}

addDirectories(folderFiles);
Expand Down Expand Up @@ -2441,22 +2474,29 @@ Wrap up your thoughts and include a call to action.
return;
}

items.forEach(item => {
// ⚑ Bolt Optimization: Use DocumentFragment to batch DOM insertions
const fragment = document.createDocumentFragment();

// ⚑ Bolt Optimization: Use for...of instead of .forEach to avoid closure allocation
for (const item of items) {
if (item.type === 'directory') {
const folderDiv = createFolderElement(item, indent);
container.appendChild(folderDiv);
fragment.appendChild(folderDiv);

if (item.expanded && item.children) {
const childContainer = document.createElement('div');
childContainer.className = 'tree-children';
renderFileTree(item.children, childContainer, indent + 1);
container.appendChild(childContainer);
fragment.appendChild(childContainer);
}
} else if (item.type === 'file') {
const fileDiv = createFileElement(item, indent);
container.appendChild(fileDiv);
fragment.appendChild(fileDiv);
}
});
}

// Append the batched items to the container
container.appendChild(fragment);
}

// Create folder element
Expand Down Expand Up @@ -2592,7 +2632,7 @@ Wrap up your thoughts and include a call to action.
}

console.log(
`[App] Loaded from link: ${fileData.path}${fileData.anchor ? '#' + fileData.anchor : ''}`
`[App] Loaded from link: ${fileData.path}${fileData.anchor ? `#${fileData.anchor}` : ''}`
);
});

Expand Down
28 changes: 20 additions & 8 deletions src/js/services/LinkNavigationService.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ export class LinkNavigationService {
* @param {string} basePath - Base path for recursion
*/
async buildFileCache(dirHandle, basePath = '') {
if (!dirHandle) return;
if (!dirHandle) {
return;
}

this.currentFolderHandle = dirHandle;
this.fileHandleCache.clear();
Expand Down Expand Up @@ -121,10 +123,14 @@ export class LinkNavigationService {
*/
async handleLinkClick(event) {
const link = event.target.closest('a');
if (!link) return;
if (!link) {
return;
}

const href = link.getAttribute('href');
if (!href) return;
if (!href) {
return;
}

// Anchor links - let default handler scroll
if (isAnchorLink(href)) {
Expand All @@ -150,7 +156,9 @@ export class LinkNavigationService {
* @returns {boolean} True if markdown file with anchor
*/
isMarkdownFileWithAnchor(href) {
if (!href) return false;
if (!href) {
return false;
}
// Match patterns like: file.md#heading, ./path/file.md#section
return /\.md#/i.test(href);
}
Expand All @@ -174,7 +182,7 @@ export class LinkNavigationService {
// Resolve absolute path
const targetPath = this.resolveTargetPath(cleanPath);
console.log(
`[LinkNav] Navigating: ${this.currentFilePath} β†’ ${targetPath}${anchor ? '#' + anchor : ''}`
`[LinkNav] Navigating: ${this.currentFilePath} β†’ ${targetPath}${anchor ? `#${anchor}` : ''}`
);

// Check cache
Expand All @@ -194,7 +202,9 @@ export class LinkNavigationService {
* @returns {{filePath: string, anchor: string|null}} Parsed path and anchor
*/
parsePathWithAnchor(href) {
if (!href) return { filePath: '', anchor: null };
if (!href) {
return { filePath: '', anchor: null };
}

const hashIndex = href.indexOf('#');
if (hashIndex === -1) {
Expand Down Expand Up @@ -247,7 +257,7 @@ export class LinkNavigationService {
});
}

console.log(`[LinkNav] βœ… Loaded: ${filePath}${anchor ? '#' + anchor : ''}`);
console.log(`[LinkNav] βœ… Loaded: ${filePath}${anchor ? `#${anchor}` : ''}`);
} catch (error) {
console.error('[LinkNav] File load error:', error);
this.showWarning('Failed to load file', `Could not read file: ${error.message}`);
Expand Down Expand Up @@ -312,7 +322,9 @@ export class LinkNavigationService {
closeModalBtn?.addEventListener('click', closeHandler);

modal.addEventListener('click', e => {
if (e.target === modal) closeHandler();
if (e.target === modal) {
closeHandler();
}
});

return modal;
Expand Down
12 changes: 7 additions & 5 deletions tests/unit/services/LinkNavigationService.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe('LinkNavigationService', () => {
it('should clear cache before building', async () => {
service.fileHandleCache.set('old-file.md', {});
const mockDirHandle = {
values: async function* () {},
async *values() {},
};

await service.buildFileCache(mockDirHandle);
Expand All @@ -68,7 +68,7 @@ describe('LinkNavigationService', () => {

it('should cache markdown files only', async () => {
const mockDirHandle = {
values: async function* () {
async *values() {
yield { kind: 'file', name: 'test.md' };
yield { kind: 'file', name: 'test.txt' };
yield { kind: 'file', name: 'README.MD' };
Expand All @@ -84,13 +84,13 @@ describe('LinkNavigationService', () => {

it('should recursively cache files in subdirectories', async () => {
const mockSubDir = {
values: async function* () {
async *values() {
yield { kind: 'file', name: 'nested.md' };
},
};

const mockDirHandle = {
values: async function* () {
async *values() {
yield { kind: 'file', name: 'root.md' };
yield { kind: 'directory', name: 'subfolder', ...mockSubDir };
},
Expand Down Expand Up @@ -224,6 +224,7 @@ describe('LinkNavigationService', () => {
name: 'test.md',
path: 'docs/test.md',
handle: mockFileHandle,
anchor: null,
});
});

Expand Down Expand Up @@ -253,6 +254,7 @@ describe('LinkNavigationService', () => {
name: 'test.md',
path: 'docs/test.md',
handle: mockFileHandle,
anchor: null,
});
});

Expand Down Expand Up @@ -338,7 +340,7 @@ describe('LinkNavigationService', () => {
});

it('should handle very long file paths', async () => {
const longPath = 'a/'.repeat(100) + 'file.md';
const longPath = `${'a/'.repeat(100)}file.md`;
service.setCurrentFile(longPath);
expect(service.currentFilePath).toBe(longPath);
});
Expand Down