Skip to content

⚑ Bolt: [performance improvement] Optimize search creators parsing #166

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 search creators parsing #166

Show file tree
Hide file tree
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
65 changes: 42 additions & 23 deletions src/helpers/search.helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,29 +42,48 @@ export const getSearchOrigins = (el: HTMLElement): string[] => {
return originsAll?.split('/').map((country) => country.trim());
};

export const parseSearchPeople = (
el: HTMLElement,
type: 'directors' | 'actors'
): CSFDMovieCreator[] => {
let who: Creator;
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)
);
export const parseSearchCreators = (
el: HTMLElement
): { directors: CSFDMovieCreator[]; actors: CSFDMovieCreator[] } => {
const creators: { directors: CSFDMovieCreator[]; actors: CSFDMovieCreator[] } = {
directors: [],
actors: []
};

if (!el) return creators;

const pNodes = el.querySelectorAll('.article-content p');

if (peopleNode) {
const people = Array.from(peopleNode.querySelectorAll('a')) as unknown as HTMLElement[];

return people.map((person) => {
return {
id: parseIdFromUrl(person.attributes.href),
name: person.innerText.trim(),
url: `https://www.csfd.cz${person.attributes.href}`
};
});
} else {
return [];
let directorsNode: HTMLElement | null = null;
let actorsNode: HTMLElement | null = null;

// Single pass to find both directors and actors nodes
for (const node of pNodes) {
const text = node.textContent;
if (text.includes('ReΕΎie:')) {
directorsNode = node;
} else if (text.includes('HrajΓ­:')) {
actorsNode = node;
}
}
Comment on lines +61 to 68
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 8, 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 null safety for textContent access.

node.textContent can return null, which would cause text.includes(...) to throw a TypeError. Per coding guidelines, helpers should use optional chaining for robust scraping since CSFD changes layouts.

πŸ›‘οΈ Proposed fix 
   for (const node of pNodes) {
-    const text = node.textContent;
-    if (text.includes('ReΕΎie:')) {
+    const text = node.textContent ?? '';
+    if (text.includes('ReΕΎie:')) {
       directorsNode = node;
     } else if (text.includes('HrajΓ­:')) {
       actorsNode = node;
     }
   }

Based on learnings: "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
for (const node of pNodes) {
const text = node.textContent;
if (text.includes('ReΕΎie:')) {
directorsNode = node;
} else if (text.includes('HrajΓ­:')) {
actorsNode = node;
}
}
for (const node of pNodes) {
const text = node.textContent ?? '';
if (text.includes('ReΕΎie:')) {
directorsNode = node;
} else if (text.includes('HrajΓ­:')) {
actorsNode = node;
}
}
πŸ€– Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/helpers/search.helper.ts` around lines 61 - 68, The loop over pNodes uses
node.textContent directly which may be null and cause text.includes(...) to
throw; update the loop in search.helper.ts (the pNodes iteration) to guard
against null by either using optional chaining (e.g.
node.textContent?.includes('ReΕΎie:')) or normalizing the value first (const text
= node.textContent ?? '') before calling includes, then set
directorsNode/actorsNode as before; ensure both checks use the null-safe access
so scraping remains robust.


if (directorsNode) {
const people = directorsNode.querySelectorAll('a');
creators.directors = people.map((person) => ({
id: parseIdFromUrl(person.attributes.href),
name: person.innerText.trim(),
url: `https://www.csfd.cz${person.attributes.href}`
}));
}

if (actorsNode) {
const people = actorsNode.querySelectorAll('a');
creators.actors = people.map((person) => ({
id: parseIdFromUrl(person.attributes.href),
name: person.innerText.trim(),
url: `https://www.csfd.cz${person.attributes.href}`
}));
}

return creators;
};
7 changes: 2 additions & 5 deletions src/services/search.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
getSearchType,
getSearchUrl,
getSearchYear,
parseSearchPeople
parseSearchCreators
} from '../helpers/search.helper';
import { CSFDLanguage, CSFDOptions } from '../types';
import { getUrlByLanguage, searchUrl } from '../vars';
Expand Down Expand Up @@ -56,10 +56,7 @@ export class SearchScraper {
colorRating: getSearchColorRating(m),
poster: getSearchPoster(m),
origins: getSearchOrigins(m),
creators: {
directors: parseSearchPeople(m, 'directors'),
actors: parseSearchPeople(m, 'actors')
}
creators: parseSearchCreators(m)
};
};

Expand Down
49 changes: 24 additions & 25 deletions tests/search.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
getSearchType,
getSearchUrl,
getSearchYear,
parseSearchPeople
parseSearchCreators
} from '../src/helpers/search.helper';
import { searchMock } from './mocks/search.html';

Expand Down Expand Up @@ -147,8 +147,8 @@ describe('Get Movie origins', () => {

describe('Get Movie creators', () => {
test('First movie directors', () => {
const movie = parseSearchPeople(moviesNode[0], 'directors');
expect(movie).toEqual<CSFDMovieCreator[]>([
const creators = parseSearchCreators(moviesNode[0]);
expect(creators.directors).toEqual<CSFDMovieCreator[]>([
{
id: 3112,
name: 'Lilly Wachowski',
Expand All @@ -162,8 +162,8 @@ describe('Get Movie creators', () => {
]);
});
test('Last movie actors', () => {
const movie = parseSearchPeople(moviesNode[moviesNode.length - 1], 'actors');
expect(movie).toEqual<CSFDMovieCreator[]>([
const creators = parseSearchCreators(moviesNode[moviesNode.length - 1]);
expect(creators.actors).toEqual<CSFDMovieCreator[]>([
{
id: 101,
name: 'Carrie-Anne Moss',
Expand All @@ -177,8 +177,8 @@ describe('Get Movie creators', () => {
]);
});
// test('Empty actors', () => {
// const movie = parseSearchPeople(moviesNode[5], 'actors');
// expect(movie).toEqual<CSFDCreator[]>([]);
// const movie = parseSearchCreators(moviesNode[5]);
// expect(movie.actors).toEqual<CSFDCreator[]>([]);
// });
});

Expand Down Expand Up @@ -295,30 +295,29 @@ describe('Get TV series origins', () => {

describe('Get TV series creators', () => {
test('First TV series directors', () => {
const movie = parseSearchPeople(tvSeriesNode[0], 'directors');
expect(movie.length).toBeGreaterThan(0);
expect(movie[0].id).toBeGreaterThan(0);
expect(movie[0].name.length).toBeGreaterThan(0);
expect(movie[0].url).toContain(String(movie[0].id));
expect(movie[0].url).toContain('https://www.csfd.cz/tvurce/');
const creators = parseSearchCreators(tvSeriesNode[0]);
expect(creators.directors.length).toBeGreaterThan(0);
expect(creators.directors[0].id).toBeGreaterThan(0);
expect(creators.directors[0].name.length).toBeGreaterThan(0);
expect(creators.directors[0].url).toContain(String(creators.directors[0].id));
expect(creators.directors[0].url).toContain('https://www.csfd.cz/tvurce/');
});
test('Last TV series actors', () => {
const movie = parseSearchPeople(tvSeriesNode[tvSeriesNode.length - 1], 'actors');
expect(movie.length).toBeGreaterThan(0);
expect(movie[0].id).toBeGreaterThan(0);
expect(movie[0].name.length).toBeGreaterThan(0);
expect(movie[0].url).toContain(String(movie[0].id));
expect(movie[0].url).toContain('https://www.csfd.cz/tvurce/');
const creators = parseSearchCreators(tvSeriesNode[tvSeriesNode.length - 1]);
expect(creators.actors.length).toBeGreaterThan(0);
expect(creators.actors[0].id).toBeGreaterThan(0);
expect(creators.actors[0].name.length).toBeGreaterThan(0);
expect(creators.actors[0].url).toContain(String(creators.actors[0].id));
expect(creators.actors[0].url).toContain('https://www.csfd.cz/tvurce/');
});
test('Empty directors check', () => {
const movie = parseSearchPeople(parse('<div></div>') as any, 'directors');
expect(movie).toEqual<CSFDMovieCreator[]>([]);
const creators = parseSearchCreators(parse('<div></div>') as any);
expect(creators.directors).toEqual<CSFDMovieCreator[]>([]);
});
// test('Empty directors + some actors', () => {
// const movie = parseSearchPeople(tvSeriesNode[3], 'actors');
// const movieDirectors = parseSearchPeople(tvSeriesNode[3], 'directors');
// expect(movie.length).toBeGreaterThan(0);
// expect(movieDirectors.length).toBeGreaterThan(0); // The mock changed, it now has directors
// const movie = parseSearchCreators(tvSeriesNode[3]);
// expect(movie.actors.length).toBeGreaterThan(0);
// expect(movie.directors.length).toBeGreaterThan(0); // The mock changed, it now has directors
// });
});

Expand Down
Loading