Skip to content

⚡ Bolt: [performance improvement] Optimize node-html-parser node list iteration #172

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bartholomej wants to merge 1 commit into
base: master
Choose a base branch
from
Open

⚡ Bolt: [performance improvement] Optimize node-html-parser node list iteration #172

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 13 additions & 4 deletions src/helpers/search.helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,21 @@ export const parseSearchPeople = (
if (type === 'directors') who = 'Režie:';
if (type === 'actors') who = 'Hrají:';

const peopleNode = Array.from(el && el.querySelectorAll('.article-content p')).find((el) =>
el.textContent.includes(who)
);
const pNodes = el && el.querySelectorAll('.article-content p');
let peopleNode: HTMLElement | null = null;
if (pNodes) {
for (const p of pNodes) {
if (p.textContent.includes(who)) {
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 14, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add defensive optional chaining in the text match condition.

Use optional chaining for scraper resilience when CSFD markup changes unexpectedly.

Suggested patch 
-      if (p.textContent.includes(who)) {
+      if (p?.textContent?.includes(who)) {

As per coding guidelines, "Never assume an element exists. CSFD changes layouts. Use optional chaining ?. or try/catch inside helpers for robust scraping."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (p.textContent.includes(who)) {
if (p?.textContent?.includes(who)) {
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/helpers/search.helper.ts` at line 57, The text match condition in
src/helpers/search.helper.ts uses a direct property access
(p.textContent.includes(who)) which can throw if the element or its text is
missing; update the condition in the helper (where p is evaluated) to use
optional chaining on the element and its text (e.g.,
p?.textContent?.includes(who)) or otherwise guard with a null check so the
scraper remains resilient to CSFD markup changes.

peopleNode = p;
break;
}
}
}

if (peopleNode) {
const people = Array.from(peopleNode.querySelectorAll('a')) as unknown as HTMLElement[];
// Optimization: Avoid Array.from allocation and use direct mapping if possible,
// though NodeList might require mapping. In our environment querySelectorAll returns an array.
const people = peopleNode.querySelectorAll('a');

return people.map((person) => {
return {
Expand Down
Loading