Migrate codebase to TypeScript with modern dependencies#45
Migrate codebase to TypeScript with modern dependencies#45
Conversation
Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
…fety Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
… methods Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
… caching Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
…add tests Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
…-rule Address review comments and modernize to async/await
…rnals-api Address PR review comments and enhance error handling + API ergonomics
…on-improvements Complete post-migration maintainability infrastructure and type safety improvements
Co-authored-by: LucasLeandro1204 <13268691+LucasLeandro1204@users.noreply.github.com>
Done! ✅ Changes made in commit a717f5a:
All 36 tests pass without any network calls to external sites. The test suite only validates API behavior and error conditions. |
There was a problem hiding this comment.
Pull request overview
This PR modernizes the Pornsearch library by migrating from JavaScript/Babel 6 to TypeScript 5, updating critical dependencies, and establishing a modern development infrastructure.
Purpose: The PR aims to eliminate 54 security vulnerabilities, add strict type safety, and modernize the build toolchain while maintaining full backward compatibility with existing consumers.
Changes:
- Complete TypeScript migration with strict type checking and comprehensive type definitions
- Major dependency updates (axios, cheerio) and removal of deprecated Babel tooling
- New development infrastructure including Jest testing, ESLint with TypeScript support, GitHub Actions CI/CD, and extensive documentation
Reviewed changes
Copilot reviewed 42 out of 45 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updated dependencies and scripts for TypeScript build system - CRITICAL ISSUES with dependency versions |
| tsconfig.json | TypeScript compiler configuration with strict mode and CommonJS output |
| jest.config.js | Jest testing framework configuration with ts-jest preset |
| eslint.config.mjs | Modern ESLint configuration for TypeScript |
| .prettierrc | Code formatting configuration |
| src/types.ts | Core type definitions for Video, Gif, and module interfaces |
| src/core/AbstractModule.ts | Base class with mixin support for module composition |
| src/core/VideoMixin.ts | Mixin adding video search capability |
| src/core/GifMixin.ts | Mixin adding GIF search capability |
| src/core/OverwriteError.ts | Custom error class with corrected message |
| src/core/Modules.ts | Module registry mapping names to constructors |
| src/modules/*.ts | Six content modules migrated to TypeScript - multiple bugs with undefined href handling |
| src/Pornsearch.ts | Main class with improved error handling and type safety |
| tests/*.test.ts | Comprehensive unit tests (26 tests) using Jest |
| example/example.ts | New TypeScript usage example |
| example/example.js | Updated CommonJS example - incorrect require path |
| README.md | Updated documentation with TypeScript examples - markdown syntax error |
| .github/workflows/ci.yml | CI/CD pipeline with multi-version testing and security scanning |
| Multiple .md files | Extensive new documentation (MAINTAINER_GUIDE, ARCHITECTURE, CONTRIBUTING, etc.) - minor date issues |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| return { | ||
| title: data.find('a').text().trim(), | ||
| url: `http://pornhub.com${data.find('a').eq(0).attr('href')}`, |
There was a problem hiding this comment.
Potential undefined value in URL construction. The data.find('a').eq(0).attr('href') can return undefined if the attribute doesn't exist, which would result in a URL like "http://pornhub.comundefined". Consider adding a null check or filtering out items where href is undefined, similar to how it's done in the gifParser method (lines 54-58).
There was a problem hiding this comment.
Fixed in 3f805e6. Added null check for href before URL construction to prevent "undefined" in URLs.
|
|
||
| return { | ||
| title, | ||
| url: `http://www.sex.com${link.attr('href')}`, |
There was a problem hiding this comment.
Potential undefined value in URL construction. The link.attr('href') on line 40 can return undefined if the attribute doesn't exist, which would result in a URL like "http://www.sex.comundefined". Consider adding a check for href similar to the title and duration checks on line 34, or add || '' as a fallback.
There was a problem hiding this comment.
Fixed in 3f805e6. Added href to the validation check along with title and duration.
| Pornsearch.gifs() | ||
| .then(gifs => console.log(gifs)); | ||
| ``` | ||
| ``` |
There was a problem hiding this comment.
Extra closing backticks - this creates a syntax error in the markdown. The code block starting at line 36 already ends at line 41, so this additional closing backticks on line 42 should be removed.
| ``` |
There was a problem hiding this comment.
Fixed in 3f805e6. Removed extra closing backticks.
|
|
||
| return { | ||
| title: title.text(), | ||
| url: `https://xvideos.com${title.attr('href')}`, |
There was a problem hiding this comment.
Potential undefined value in URL construction. The title.attr('href') can return undefined if the attribute doesn't exist, which would result in a URL like "https://xvideos.comundefined". Consider adding a null check or using the nullish coalescing operator: url: \https://xvideos.com${title.attr('href') || ''}``.
There was a problem hiding this comment.
Fixed in 3f805e6. Added null check for href and added filter to remove undefined items.
|
|
||
| return { | ||
| title: data.find('.video-box-title').text().trim(), | ||
| url: `http://youporn.com${data.find('a').eq(0).attr('href')}`, |
There was a problem hiding this comment.
Potential undefined value in URL construction. The data.find('a').eq(0).attr('href') can return undefined if the attribute doesn't exist, which would result in a URL like "http://youporn.comundefined". Consider adding a null check or using the OR operator for a fallback value.
There was a problem hiding this comment.
Fixed in 3f805e6. Added null check for href before URL construction.
| if (!data.length) { | ||
| return undefined; | ||
| } | ||
|
|
There was a problem hiding this comment.
Potential undefined value in URL. The data.find('a').eq(0).attr('href') can return undefined if the attribute doesn't exist, which would result in a URL of "undefined". Unlike other modules that prepend a domain, this module appears to expect an absolute or relative URL. Consider adding a null check or using || '' as a fallback.
There was a problem hiding this comment.
Fixed in 3f805e6. Added null checks for href in both videoParser and gifParser methods.
| 'use strict'; | ||
|
|
||
| const Pornsearch = require('../'); | ||
| const Pornsearch = require('../').default || require('../'); |
There was a problem hiding this comment.
The require path is incorrect. require('../') will try to load from the package root, but the main entry point in package.json is "dist/Pornsearch". This should be const Pornsearch = require('../dist/Pornsearch').default || require('../dist/Pornsearch'); to properly load the compiled module.
| const Pornsearch = require('../').default || require('../'); | |
| const Pornsearch = require('../dist/Pornsearch').default || require('../dist/Pornsearch'); |
There was a problem hiding this comment.
Fixed in 3f805e6. Updated require path to use '../dist/Pornsearch' as suggested.
|
@copilot apply changes based on the comments in this thread |
Done in 3f805e6! Applied all changes from the review: Fixed Issues:
Code Review:
|
Modernizing Pornsearch to TypeScript and Modern Standards ✅
🎉 All Tasks Completed Successfully!
Recent Updates (PR Review Fixes):
Security & Code Quality Improvements:
Previous Achievements:
Module Support:
GIF Support: pornhub, sex, motherless
Video Support: pornhub, sex, redtube, xvideos, youporn, motherless
Type Safety:
Videointerface: title, url, duration, thumbGifinterface: title, url, webm?, thumb?ModuleInterface: Contract for all modulesTest Results:
Migration Status: COMPLETE 🚀
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.