-
-
Notifications
You must be signed in to change notification settings - Fork 849
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.
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
feat: migrate script to typescript (#3423) #3761
Conversation
* fix eslint error * rename * rename * fix eslint error * add error type * add new types * add types * refactor and fix eslint errors * use google apis and add types * remove console log * add specific rules for the script folder * refactor dir variables * add types and refactor it * refactor: update eslint rules * reformat document * update include paths * use dynamic import * reformat eslintrc * update rules * change tabs to spaces * update ts node config * migrate tools scripts * refactor __dirname variable * refactor function in build-rss * add new package type * Added error handling for undefined types * add types * refactor adopter/index.ts * use tsx instead of ts-node * add types for build RSS * revert name * fix name * fix name * fix name * Original name * Rename next-i18next.config.js to next-i18next.config.cjs * migrate build-post-list.ts * use old package-lock file * No code changes made * migrate check-markdown.ts * feat: add TypeScript types for tools and categories * update type * feat: enhance type safety in combine-tools script with TypeScript types * chore: disable eslint rule for no-await-in-loop in combine-tools script * feat: improve type safety and refactor dashboard script functions * feat: add TypeScript types for dashboard script and enhance type safety * No code changes detected. * update package json * refactor: remove console log from buildPostList function * fix: update workflow to conditionally install dependencies based on package.json existence * chore: remove unused PowerShell script and TypeScript type definitions * refactor: add type annotations to capitalizeJsxTags function parameters * refactor: migrate build-docs * refactor: remove unused URL validation function from check-markdown.ts * chore: update Jest configuration and dependencies for TypeScript support * chore: enable ES module support in Jest configuration * chore: update Jest configuration for TypeScript file handling and coverage * chore: update Jest configuration to set test environment to Node * chore: enable ES module support in ts-jest transformer * refactor: update case studies import to default export and improve type imports * refactor: update Jest configuration for improved TypeScript support * refactor: change imports to default exports for consistency * refactor: update test imports to remove file extensions for consistency * refactor: simplify Jest transform configuration for TypeScript files * refactor: update dynamic import for manual tools and mock axios in tests * refactor: update path resolution to use fileURLToPath for better compatibility * refactor: update path resolution to use fileURLToPath for improved compatibility * refactor: improve optional chaining usage and enhance tool validation logging * refactor: enable ESM support in ts-jest transform configuration * refactor: update ts-jest configuration to use ESNext module in tsconfig * refactor: add support for ESM by updating Jest configuration and TypeScript target * refactor: enhance Jest configuration for ESM support and improve module resolution * refactor: update Jest configuration to use ES2022 module and target * refactor: simplify JSON import in build-rss and remove debug log in combine-tools * Revert "refactor: improve optional chaining usage and enhance tool validation logging" This reverts commit d6b0e2a. * refactor: update type definitions for discussions and improve null handling * refactor: improve optional chaining for language filter in getFinalTool function * refactor: clean up code formatting and improve consistency across utility functions * refactor: change getData to an exported function and update TypeScript target version * refactor: remove unused slugify function declaration from markdown-toc module * refactor: update guid type in RSSItemType to include isPermaLink property * refactor: replace manual path joining with path.join for better compatibility * refactor: change parameter type of convertToJson function to unknown for better type safety * refactor: add return type Promise<void> to buildFinanceInfoList function for better clarity * refactor: use optional chaining for safer access to video properties in buildNewsroomVideos function * refactor: simplify interfaces by introducing BasicIssueOrPR for shared properties * refactor: enhance MarkdownToken interface with optional properties for better flexibility * refactor: update tool handling by replacing dynamic import with JSON parsing and enhancing type definitions for better clarity and safety * test: update mock path for readAndWriteJson to include file extension * refactor: simplify Jest configuration by removing unnecessary options and streamlining transform settings * refactor: update imports to use named exports for consistency and clarity * refactor: simplify Jest configuration by utilizing ts-jest preset and removing unnecessary transform settings * refactor: enhance type safety by casting imported JSON as 'any' in getAllPosts function * refactor: enhance Jest configuration by integrating ts-jest preset with ESM support for JSON transformation * refactor: enhance Jest configuration by enabling JSON module resolution in ts-jest preset * refactor: simplify Jest configuration by removing JSON transform settings and adjusting transformIgnorePatterns * refactor: update Jest configuration to specify tsconfig for ts-jest preset * refactor: clean up test files by removing test directory after execution and updating fs module import * refactor: update import statement for pathExists from fs-extra to improve compatibility * refactor: remove assert statements and export utility functions for better error handling and reusability * feat: add TypeScript declaration for JSON module to improve type safety * feat: add Babel configuration and TypeScript presets for improved compatibility * refactor: remove assert statements and improve error handling in build scripts * feat: replace Google APIs client with node-fetch for YouTube data retrieval and improve error handling * feat: update Jest and Babel configuration for TypeScript support and improve test script transformation * fix: correct filename for automated tools configuration in build script * chore: remove ts-jest from package.json and package-lock.json * refactor: replace assert statements with error handling in scripts * ci: replace npm install with npm ci for dependency installation in workflow * feat: add JSDoc comments for improved documentation across various scripts * chore: remove unnecessary roots configuration from Jest config * chore: clean up tsconfig.json by removing unused ts-node configuration * test: remove redundant axios.get mock initialization in tools-object tests * test: fix indentation in cleanup comment for build-pages tests * fix: correct typo in comment for additional check in build-docs script * fix: update type annotation for validateFunction parameter in checkMarkdownFiles * fix: add error handling for missing CALENDAR_SERVICE_ACCOUNT environment variable * fix: add error handling for missing YOUTUBE_TOKEN and validate video data structure in buildNewsroomVideos function * fix: remove redundant error handling for missing CALENDAR_SERVICE_ACCOUNT environment variable in buildMeetings function * fix: add file existence checks for Expenses.yml and ExpensesLink.yml in buildFinanceInfoList function * fix: await rssFeed call and add error handling in start function * fix: add error handling for missing GITHUB_TOKEN environment variable in getData function * fix: enhance error handling for GitHub API responses in getData function * refactor: remove file existence checks and simplify start function error handling * fix: remove GITHUB_TOKEN check from getData function * fix: add validation for missing frontmatter in validateBlogs function * feat: implement pagination for GitHub API data retrieval in getData function * fix: format mockData structure in getData tests for consistency * refactor: move pause function to utils and remove redundant definition from build-dashboard * feat: add processManualTool function to validate and process manual tools in combine-tools script * refactor: remove max-depth eslint rule from combine-tools script * refactor: remove duplicate Scala color configuration from tags-color script * feat(logging): integrate Winston logger for improved error handling and logging * feat(logging): replace console log with Winston logger for GitHub rate limit warnings * test(logging): update tests to use Winston logger for warnings and errors * fix: use optional chaining for comments pageInfo in processHotDiscussions function * refactor: improve type definitions for tools in combine-tools.ts * refactor(jest): remove 'node' from moduleFileExtensions in jest.config.js * refactor(logging): replace console.log with logger in scripts * fix(docs): simplify error handling in buildNavTree function * fix(errors): simplify error handling in buildMeetings and convertTools functions * fix(errors): simplify error handling in buildFinanceInfoList function * fix(validation): return errors when frontmatter is missing in validateBlogs function * test(validation): update frontmatter validation test for consistency * test(meetings): add error handling tests for invalid data structure and missing start.dateTime * fix(videos): improve error handling for missing video properties and add test for YOUTUBE_TOKEN * test(videos): remove unnecessary whitespace in buildNewsroomVideos test * test(github): add pagination test for getData function and mock logger * test(tools): add test for createToolObject with single parameter * fix(tools): add istanbul ignore comments to enhance test coverage * fix(tools): simplify language and technology filter checks in getFinalTool function * feat(tools): export getFinalTool function and enhance test for handling tools with missing data * test(tools): remove unnecessary blank line in combineTools test suite * fixed lint errors * test(markdown): replace console.log with logger.warn in checkMarkdown tests * test(markdown): refactor checkMarkdown tests for improved readability and consistency * test: improve test readability and consistency by refactoring whitespace and formatting * fixed variable naming issue * fix: update import paths to include .ts extension for build-docs and build-pages tests * fix: update import paths in utils tests to include .ts extension * test(markdown): improve test clarity by removing commented-out code * fix: add istanbul ignore comments to exclude specific lines from coverage * fix: add istanbul ignore comment to exclude directory check from coverage * fix: add istanbul ignore comments to improve test coverage exclusions * test(dashboard): remove commented-out expectation for error logging * fix: add istanbul ignore comment to exclude title validation from coverage * refactor: migrate check-edit-links script to TypeScript and update related tests * fix: add istanbul ignore comments for code coverage in check-edit-links script * chore: update workflow to trigger on push instead of pull request * Revert "chore: update workflow to trigger on push instead of pull request" This reverts commit cc194a6. * fix(docs): handle undefined document object in convertDocPosts function * fix: disable eslint rule for no-await-in-loop in combine-tools.ts * fix: require GITHUB_TOKEN environment variable in getData function * fix: update getData function to return specific ToolsData type * test: change link checker workflow trigger to run on push * fix: remove GITHUB_TOKEN requirement in getData function * chore: update link checker workflow to run weekly and allow manual triggers * fix: update i18next config import to use .cjs extension * Update scripts/adopters/index.ts Co-authored-by: Akshat Nema <[email protected]> * fix: require GITHUB_TOKEN environment variable in extract-tools-github script * fix: validate YOUTUBE_TOKEN and GITHUB_TOKEN environment variables in scripts * refactor: remove redundant YOUTUBE_TOKEN error test in build-newsroom-videos * fix: improve type safety in build-rss script by updating post types * refactor: simplify category extraction logic in getUniqueCategories function * fix: handle errors while walking directories in build-post-list script * fix: add error handling and logging in build-tools script * fix: improve error handling in build-tools script * refactor: remove unnecessary comment from ensureDirectoryExists function * Add ts types in return type of functions * Moved ts to a stable version --------- Co-authored-by: asyncapi-bot <[email protected]> Co-authored-by: akshatnema <[email protected]> Co-authored-by: Akshat Nema <[email protected]>
WalkthroughThis set of changes refactors a large portion of the codebase to convert from JavaScript (CommonJS) to TypeScript (ES Modules). Import statements for the internationalization configuration have been updated to use a “.cjs” extension, and the deprecated configuration file has been removed. Many scripts, tests, and utility modules have been migrated to TypeScript with enhanced type safety, updated logging mechanisms, reformatted configuration files, and newly added type declarations. These changes also update package scripts and testing configurations accordingly. Changes
Sequence Diagram(s)(No sequence diagrams are generated as the changes primarily involve type conversions and refactoring without modifying control flow.) Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
✅ Deploy Preview for asyncapi-website ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🔭 Outside diff range comments (3)
scripts/build-post-list.ts (1)
253-289
:⚠️ Potential issueClose the loop on ephemeral data.
Since
finalResult
is globally defined, repeated calls tobuildPostList
could merge data. If your use case always calls this function once per run, that may be fine. Otherwise, initialize a fresh result object inside the function or pass a context object around.tests/dashboard/build-dashboard.test.js (1)
142-142
: 💡 Verification agent
⚠️ Potential issue🧩 Analysis chain
Fix the failing test for monthsSince calculation
The pipeline is reporting a test failure in the "should calculate months since date" test, indicating that the function is returning 1 month instead of the expected 2 months.
🏁 Script executed:
#!/bin/bash # Check the implementation of monthsSince function in the TypeScript file rg -A 10 "function monthsSince" scripts/dashboard/build-dashboard.tsLength of output: 468
Action Needed: Revise
monthsSince
Calculation to Match Expected Test OutputThe failing "should calculate months since date" test reveals that the current implementation—which calculates the number of months by dividing the elapsed seconds by 2,592,000 (i.e., 30 days worth of seconds) and then applying
Math.floor
—is yielding 1 instead of the expected 2 months. This suggests that the rounding method is not handling partial month values as intended by the test.Suggested next steps:
- Review and Update Rounding Logic: Consider switching from
Math.floor
toMath.ceil
(or another appropriate adjustment) so that the function correctly accounts for partial months and returns 2 when expected.- Verify Impact: Ensure that changing the rounding behavior aligns with other parts of the code that depend on
monthsSince
and update associated tests if necessary.Affected areas:
- File:
scripts/dashboard/build-dashboard.ts
- Function:
monthsSince(date: string)
🧰 Tools
🪛 GitHub Actions: PR testing - if Node project
[error] 142-142: Test failed: Expected: 2, Received: 1 in 'should calculate months since date'.
scripts/compose.ts (1)
29-113
: 🛠️ Refactor suggestionEnsure robust validation or sanitization of user inputs
ThegenFrontMatter
function splits user-supplied comma-separated tags, which might introduce edge cases (e.g., empty tags). Consider trimming and validating strings or preventing unexpected whitespace. Also confirm that any user input is safe to embed in Markdown front matter, especially if user input might contain special characters.const tagArray = answers.tags.split(','); tagArray.forEach((tag: string, index: number) => { tagArray[index] = tag.trim(); + if (!tagArray[index]) { + tagArray.splice(index, 1); + } });
🧹 Nitpick comments (55)
next-i18next.config.cjs (1)
11-11
: Consider removing the "this line" comment.The comment "this line" at the end of line 11 doesn't provide any useful information and should be removed for cleaner code.
- react: { useSuspense: false } // this line + react: { useSuspense: false }utils/getUniqueCategories.ts (1)
13-13
: Consider adding a more specific type for the expense entry.Rather than using an inline type annotation, consider defining a proper interface for the expense entry structure to improve code maintainability and type safety.
+ interface ExpenseEntry { + Category: string; + // Add other properties that exist in the expense entries + } Object.keys(Expenses).forEach((month) => { - Expenses[month as keyof typeof Expenses].forEach((entry: { Category: string }) => { + Expenses[month as keyof typeof Expenses].forEach((entry: ExpenseEntry) => {scripts/tools/extract-tools-github.ts (1)
1-1
: Document the ESLint disable directiveAdd a comment explaining why the no-await-in-loop rule is disabled to improve maintainability.
-/* eslint-disable no-await-in-loop */ +/* eslint-disable no-await-in-loop */ +// This rule is disabled because we need to process API pages sequentially to handle pagination correctlyscripts/utils.ts (2)
11-34
: Consider using a more specific return type instead of 'any'Using
any
type loses TypeScript's type safety benefits. Consider using a more specific type or at leastunknown
to enforce type checking at usage sites.-function convertToJson(contentYAMLorJSON: unknown): any { +function convertToJson<T = Record<string, unknown>>(contentYAMLorJSON: unknown): T {
41-45
: Use a more descriptive name for the Promise resolverUsing
res
as a variable name could be confused with a response object. A more descriptive name would improve readability.async function pause(ms: number): Promise<void> { - return new Promise((res) => { + return new Promise((resolve) => { - setTimeout(res, ms); + setTimeout(resolve, ms); }); }scripts/casestudies/index.ts (2)
6-11
: JSDoc comments are well written but could use TypeScript parameter typesThe JSDoc comments provide good documentation for the function, but the parameter types in the comments still use the JavaScript-style
{string}
notation instead of TypeScript syntax.Consider updating the JSDoc parameter types to match TypeScript syntax:
/** * Builds a list of case studies from files in a directory and writes it to a specified file. - * @param {string} dirWithCaseStudy - The directory containing case study files. - * @param {string} writeFilePath - The path to write the case studies list to. + * @param dirWithCaseStudy - The directory containing case study files. + * @param writeFilePath - The path to write the case studies list to. * @returns {Promise<object[]>} - The list of case studies. */
12-33
: Function implementation looks good but could benefit from file filteringThe implementation properly uses async/await with Promise.all for parallel processing, which is efficient. The error handling is also well done.
Consider adding filtering to only process files with specific extensions (like .yml or .yaml):
async function buildCaseStudiesList(dirWithCaseStudy: string, writeFilePath: string): Promise<object[]> { try { const files = await readdir(dirWithCaseStudy); + const validFiles = files.filter(file => file.endsWith('.yml') || file.endsWith('.yaml')); // Process all files in parallel using Promise.all const caseStudiesList = await Promise.all( - files.map(async (file) => { + validFiles.map(async (file) => { const caseStudyFileName = join(dirWithCaseStudy, file); const caseStudyContent = await readFile(caseStudyFileName, 'utf-8'); return convertToJson(caseStudyContent); }) );scripts/index.ts (1)
61-63
: Consider separating export and immediate executionThe function is both exported and immediately executed, which might cause unexpected behavior when imported by other modules.
Consider conditionally executing the function only when the file is run directly:
export { start }; -start(); +// Only run when this file is executed directly +if (import.meta.url === fileURLToPath(process.argv[1])) { + start(); +}tests/casestudies/index.test.js (1)
3-4
:❓ Verification inconclusive
Consider migrating test file to TypeScript for consistency
While importing from the TypeScript file works, it would be more consistent to migrate the test file to TypeScript as well.
Also, requiring a TypeScript file directly might cause issues depending on your project configuration. Ensure your transpilation setup supports this pattern or consider using a compiled JavaScript file instead.
🏁 Script executed:
#!/bin/bash # Check if ts-node or a similar package is installed to support direct TS imports grep -E "ts-node|ts-jest" package.jsonLength of output: 40
Action Required: Migrate Test File to TypeScript & Verify Transpilation Setup
- The current test file (
tests/casestudies/index.test.js
) imports a TypeScript module directly using the.ts
extension. While this adheres to the project's policy (requiring explicit.ts
extensions per our configuration), importing TS files from a JavaScript test file can lead to subtle issues if the transpilation setup isn’t fully aligned.- Our preliminary check of
package.json
did not reveal any dependencies such asts-node
orts-jest
, which raises concerns about how directly imported TS files are handled during test execution.- Recommendation:
- Migrate the test file to TypeScript to maintain consistency with the rest of the codebase.
- Alternatively, ensure that your current transpilation configuration (notably the
"moduleResolution": "bundler"
setting intsconfig.json
) fully supports direct imports of TypeScript files in JavaScript tests. If not, consider either adding the appropriate runtime support (e.g.,ts-node
orts-jest
) or using a precompiled JavaScript version of the module.Please verify your transpilation setup manually to confirm that it correctly handles direct TS imports or update the setup as needed.
scripts/utils/readAndWriteJson.ts (1)
1-36
: Well-structured TypeScript utility with good error handling.This new utility function effectively handles reading, converting, and writing JSON files with proper TypeScript types and comprehensive error handling at each stage. The JSDoc comments clearly document the function's purpose and parameters.
A few minor suggestions to enhance the implementation:
- Consider adding a return type to the function (e.g.,
Promise<void>
) to make it fully type-safe- The error messages could include the file paths to make debugging easier
- export async function writeJSON(readPath: string, writePath: string) { + export async function writeJSON(readPath: string, writePath: string): Promise<void> { let readContent; let jsonContent; // Attempt to read the file try { readContent = await readFile(readPath, 'utf-8'); } catch (err) { - throw new Error(`Error while reading file\nError: ${err}`); + throw new Error(`Error while reading file ${readPath}\nError: ${err}`); } // Attempt to convert content to JSON try { jsonContent = convertToJson(readContent); } catch (err) { - throw new Error(`Error while conversion\nError: ${err}`); + throw new Error(`Error while converting content from ${readPath}\nError: ${err}`); } // Attempt to write the JSON content to file try { await writeFile(writePath, JSON.stringify(jsonContent)); } catch (err) { - throw new Error(`Error while writing file\nError: ${err}`); + throw new Error(`Error while writing file ${writePath}\nError: ${err}`); } }scripts/finance/index.ts (1)
1-51
: Well-implemented TypeScript conversion with proper interface and error handling.This implementation properly defines a TypeScript interface for the function parameters and includes clear JSDoc documentation. The code has good error handling and follows modern JavaScript/TypeScript patterns by using async/await and fs/promises.
A couple of minor suggestions for improvement:
- Consider adding more specific error handling for the different operations (mkdir and writeJSON) to provide more granular error messages
- The try/catch block encompasses the entire function - you could make the error messages more specific by using separate try/catch blocks for different operations
export async function buildFinanceInfoList({ currentDir, configDir, financeDir, year, jsonDataDir }: BuildFinanceInfoListProps): Promise<void> { try { const expensesPath = resolve(currentDir, configDir, financeDir, year, 'Expenses.yml'); const expensesLinkPath = resolve(currentDir, configDir, financeDir, year, 'ExpensesLink.yml'); // Ensure the directory exists before writing the files const jsonDirectory = resolve(currentDir, configDir, financeDir, jsonDataDir); + try { await mkdir(jsonDirectory, { recursive: true }); + } catch (err) { + throw new Error(`Failed to create directory ${jsonDirectory}: ${err}`); + } // Write Expenses and ExpensesLink to JSON files const expensesJsonPath = resolve(jsonDirectory, 'Expenses.json'); + try { await writeJSON(expensesPath, expensesJsonPath); + } catch (err) { + throw new Error(`Failed to write Expenses.json: ${err}`); + } const expensesLinkJsonPath = resolve(jsonDirectory, 'ExpensesLink.json'); + try { await writeJSON(expensesLinkPath, expensesLinkJsonPath); + } catch (err) { + throw new Error(`Failed to write ExpensesLink.json: ${err}`); + } } catch (err) { throw new Error(`Error in buildFinanceInfoList: ${err}`); } }tests/build-rss.test.js (1)
4-5
: Consider removing the file extension from imports.While the current import works with Jest's transformer, it's generally better practice to import without the file extension for better maintainability.
-const { rssFeed } = require('../scripts/build-rss.ts'); +const { rssFeed } = require('../scripts/build-rss');scripts/build-tools.ts (1)
13-21
: JSDoc comments contain redundant type information.The JSDoc comments include type annotations that are redundant with TypeScript's type annotations. Consider removing the type information from JSDoc comments and letting TypeScript handle it.
/** * Builds the tools by combining automated and manual tools data, and writes the result to the specified paths. * - * @param {string} automatedToolsPath - The path to write the automated tools data. - * @param {string} manualToolsPath - The path to read the manual tools data. - * @param {string} toolsPath - The path to write the combined tools data. - * @param {string} tagsPath - The path to write the tags data. + * @param automatedToolsPath - The path to write the automated tools data. + * @param manualToolsPath - The path to read the manual tools data. + * @param toolsPath - The path to write the combined tools data. + * @param tagsPath - The path to write the tags data. * @throws {Error} - Throws an error if there is an issue during the build process. */tests/build-post-list.test.js (1)
3-4
: Consider removing the file extension from imports.Similar to other tests, it's better practice to import without the file extension.
-const { buildPostList, slugifyToC, addItem } = require('../scripts/build-post-list.ts'); +const { buildPostList, slugifyToC, addItem } = require('../scripts/build-post-list');types/scripts/build-posts-list.ts (2)
3-8
: Consider using more descriptive property names to improve clarity.The interface
TableOfContentsItem
uses concise property names likelvl
andi
. While they are compact, consider expanding them (e.g.,level
,index
) to enhance readability for future maintainers and new contributors.
13-35
: Review the broad[key: string]: any;
index signature.Allowing any additional properties ensures flexibility but could undermine type safety. Consider introducing narrower types (e.g.,
[key: string]: unknown;
) or more structured definitions to prevent accidental misuse of arbitrary keys.tests/tools/combine-tools.test.js (3)
3-3
: Align imports and modules consistently.You're requiring a TypeScript file (
combine-tools.ts
) from a.js
test file. This can be valid, but consider switching to ES modules or renaming the test file to.ts
for consistency and better type checking across the codebase.Also applies to: 20-21, 23-27
83-83
: Use logger for consistency if desired.Currently mocking
console.error
is fine, but if all logging is handled vialogger.error
, consider removing direct calls toconsole.error
for consistency.
119-119
: Safeguard JSON parsing of logger output.Parsing
logger.error.mock.calls[0][0]
as JSON works only if the logger consistently logs valid JSON strings. Consider adding guards or a try/catch if the logger might log other formats in the future.scripts/build-post-list.ts (5)
1-2
: Review the need for disablingno-await-in-loop
andmax-depth
.If performance is not critical, disabling these ESLint rules may be acceptable. Otherwise, consider refactoring loops to run in parallel (e.g.,
Promise.all
) or simplifying nested logic to comply with best practices.
29-47
: Allow additional transformations after extracting heading IDs.The
slugifyToC
function currently returns an empty string if no ID is found. If there's a desire to auto-generate fallback slugs from the heading text, consider adding that enhancement.
88-103
: Consider advanced version parsing.
getVersionDetails
currently relies on the file’s base name and a manual split logic. If future specs or release naming diverge, consider a more robust approach (e.g., regex-based version extraction) to avoid breakage.
105-124
: Support additional spec tags.
handleSpecificationVersion
handles certain keywords likenext-spec
orexplorer
, appending text to the title. Consider centralizing spec tags in a config or enum to handle expansions better later.
136-251
: Explore possible concurrency improvements.
walkDirectories
usesfor ... of ... await
. This approach is straightforward but sequential. For large directories, consider parallel processing withPromise.all
, while being mindful of potential I/O concurrency limits.scripts/tools/combine-tools.ts (3)
42-48
: Consider synchronizing language/technology list definitions
The code duplicates similar logic for language and technology lists, each with its ownFuse
instance. If these remain structurally similar in the future, consider extracting the common logic into a reusable function or class, ensuring consistency and reducing duplication.
90-110
: Assess performance of using await in loop
The loops retrieve matching color items usingawait languageFuse.search(...)
andawait technologyFuse.search(...)
inside iteration. While it works fine for normal use, if the number of tools grows considerably, it might be more efficient to process them in batches.
183-205
: Ensure thread safety or concurrency control when appending results
The function writes to shared objects (finalTools
and the file system) in a sequential loop. Although synchronous writes might suffice here, consider concurrency patterns or safe merges if you anticipate parallel usage or concurrency in the future.scripts/build-rss.ts (2)
24-35
: Sanitization approach
Replacing HTML entities with string operations is simple and effective, but if additional transformations or complex HTML are encountered, consider using a more robust HTML sanitization library or utility to avoid missing entities or potential injection issues.
90-93
: Validate critical fields earlier
While you filter out invalid posts after sorting, consider validating them before potentially processing or sorting them. This can short-circuit and prevent unnecessary data manipulation on incomplete entries.scripts/build-newsroom-videos.ts (1)
13-19
: JSDoc comments don't match TypeScript parameter typesThe JSDoc parameter type uses
{string}
notation while you're now using TypeScript's native type annotations. This creates redundancy and potential for inconsistency.For TypeScript files, you can simplify your JSDoc by removing the type annotations since TypeScript's native annotations will handle type checking:
/** * Fetches the latest YouTube videos from the AsyncAPI channel and writes the data to a specified path. * - * @param {string} writePath - The path to write the video data. + * @param writePath - The path to write the video data. * @returns {Promise<string>} - A promise that resolves to the video data in JSON format. * @throws {Error} - Throws an error if there is an issue during the fetch or write process. */scripts/build-pages.ts (3)
10-14
: JSDoc doesn't match TypeScript type annotationSimilar to other files, the JSDoc parameter type annotation is redundant when using TypeScript's native type annotations.
/** * Ensures that the specified directory exists. If it doesn't, creates it. - * @param {PathLike} directory - The directory path to check or create. + * @param directory - The directory path to check or create. */
21-27
: JSDoc return type is redundant with TypeScript return typeThe JSDoc return type annotation is redundant when the function already has a TypeScript return type.
/** * Capitalizes JSX tags in the provided content string. * - * @param {string} content - The content string to process. - * @returns {string} - The content string with capitalized JSX tags. + * @param content - The content string to process. + * @returns The content string with capitalized JSX tags. */
37-43
: JSDoc doesn't match TypeScript type annotationsOnce again, the JSDoc parameter type annotations are redundant when using TypeScript's native type annotations.
/** * Copies and renames files from the source directory to the target directory. * - * @param {string} srcDir - The source directory. - * @param {string} targetDir - The target directory. + * @param srcDir - The source directory. + * @param targetDir - The target directory. */tests/build-newsroom-videos.test.js (1)
104-104
: Consider using undefined assignment instead of delete operatorUsing the
delete
operator can impact performance.- delete process.env.YOUTUBE_TOKEN; + process.env.YOUTUBE_TOKEN = undefined;🧰 Tools
🪛 Biome (1.9.4)
[error] 104-104: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
tests/build-meetings.test.js (1)
74-74
: Consider using undefined assignment instead of delete operatorUsing the
delete
operator can impact performance.- delete process.env.CALENDAR_SERVICE_ACCOUNT; + process.env.CALENDAR_SERVICE_ACCOUNT = undefined;🧰 Tools
🪛 Biome (1.9.4)
[error] 74-74: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
tests/tools/extract-tools-github.test.js (1)
90-90
: Consider using undefined assignment instead of delete operatorUsing the
delete
operator can impact performance.- delete process.env.GITHUB_TOKEN; + process.env.GITHUB_TOKEN = undefined;🧰 Tools
🪛 Biome (1.9.4)
[error] 90-90: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
tests/markdown/check-edit-links.test.js (1)
14-16
: Mocking the logger utility supports better test isolation
This mocking approach ensures that the tests capture only the calls tologger.info
without noisy console output. Make sure other log levels (likeerror
,warn
) are also covered if necessary.scripts/tools/tools-object.ts (2)
42-63
: createToolObject method ensures flexible repository data
Generating a structured tool object from partial or absent fields (e.g.,repoUrl
fallback) is a sensible design. As a further enhancement, consider stronger runtime validation to guard against unexpected undefined fields intoolFile
.
77-151
: convertTools function handles concurrency and mapping well
UsingPromise.all
effectively processes the array of tools in parallel. A few suggestions:
- Implement a concurrency limit if you anticipate a large number of tools to avoid hitting rate limits or exhausting network resources.
- Ensure partial failures don’t hamper the entire process. Currently, a single error re-throws and interrupts everything—if you need partial results, consider capturing errors individually.
tests/tools/tools-object.test.js (1)
55-73
: Typo in Test Name
The test name currently reads "should create a tool object one parameters." A small grammar adjustment is recommended:- it('should create a tool object one parameters', async () => { + it('should create a tool object with one parameter', async () => {scripts/tools/tools-schema.json (1)
2-210
: Consider Adding$id
and Other Best Practices
Your schema is well-structured with strict validation rules. For completeness, you might want to add a$id
property and consider referencing$defs
(if desired) to ensure consistent reusability. Everything else looks correct for draft-07 usage.scripts/build-meetings.ts (2)
21-31
: Authentication Block
The try/catch pattern here is valid. As a minor enhancement, you could warn the user ifprocess.env.CALENDAR_SERVICE_ACCOUNT
is undefined and skip the attempt, preventing a silent fallback toundefined
.try { + if (!process.env.CALENDAR_SERVICE_ACCOUNT) { + logger.warn('CALENDAR_SERVICE_ACCOUNT is not set. Check your environment variables.'); + } auth = new google.auth.GoogleAuth({ scopes: ['https://www.googleapis.com/auth/calendar'], credentials: process.env.CALENDAR_SERVICE_ACCOUNT ? JSON.parse(process.env.CALENDAR_SERVICE_ACCOUNT) : undefined });
51-66
: Use Optional Chaining
The static analysis tool suggests using optional chaining for banner and URL references. This reduces redundancy and makes the code more concise:- url: - e.extendedProperties?.private && - `https://github.com/asyncapi/community/issues/${e.extendedProperties.private.ISSUE_ID}`, - banner: e.extendedProperties?.private && e.extendedProperties.private.BANNER, + url: `https://github.com/asyncapi/community/issues/${e.extendedProperties?.private?.ISSUE_ID}`, + banner: e.extendedProperties?.private?.BANNER,🧰 Tools
🪛 Biome (1.9.4)
[error] 62-62: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
scripts/build-docs.ts (2)
1-7
: Consider replacing lodash’s sortBy with native array sort for smaller data sets.
Although lodash’ssortBy
is perfectly fine, modern JavaScript array methods can often accomplish similar tasks with minimal overhead and no extra dependency. If performance or code size is a concern, you might evaluate whether the built-inArray.prototype.sort
meets your needs here.
161-248
: Clarify and simplify next/previous navigation logic.
TheaddDocButtons
function merges multiple arrays and manipulates indices to build next/previous links. For improved readability, consider extracting the logic for constructing navigation links into a helper function. This can reduce the nesting depth and isolate the index checks for next/prev pages, leading to more maintainable code.function addDocButtons(docPosts: Details[], treePosts: NavTree): Details[] { ... - // next/prev logic inline + // next/prev logic in a dedicated utilitytests/markdown/check-markdown.test.js (2)
141-157
: Remove duplicate test for isValidURL.
The test suite duplicates the “should return true or false for URLs” block in lines 141-147 and again in 150-156. Merging them can reduce maintenance overhead.- it('should return true or false for URLs', () => { - expect(isValidURL('http://example.com')).toBe(true); - ... - }); - - it('should return true or false for URLs', () => { - expect(isValidURL('http://example.com')).toBe(true); - ... - }); + it('should validate URLs properly', () => { + expect(isValidURL('http://example.com')).toBe(true); + ... + });
159-166
: Align missing frontmatter tests under one descriptive “invalid frontmatter” scenario.
Multiple tests verify behavior when frontmatter is undefined or incomplete. Consider grouping them for clarity, to make it obvious which frontmatter validations are being tested collectively.scripts/markdown/check-markdown.ts (1)
131-174
: Optimize directory traversal strategies.
checkMarkdownFiles
recursively traverses potentially large directories. If performance or concurrency become concerns, consider parallelizing file reads in smaller batches or short-circuiting on severe errors. Right now, each read is launched in parallel, which could be large in memory usage for huge directories.types/scripts/tools.ts (1)
7-25
: Consider using an enum for Category.While string unions offer flexibility, using an enum might enable advanced tooling features (like reverse lookups) and reduce the risk of typos. However, if these benefits aren’t required, this approach is perfectly fine.
scripts/markdown/check-edit-links.ts (2)
22-53
: Good asynchronous error handling inprocessBatch
.The function tries to fetch HEAD requests for edit links and handles timeouts. Ensure renegotiation of concurrency is handled by the batch approach higher up.
123-164
: Recursive file scanning ingeneratePaths
is well-structured.The logic excludes
_section.md
files and uses a relative path approach. Pay attention to any potential performance issues in large docs directories, but for most docs projects, this is fine.scripts/dashboard/build-dashboard.ts (2)
24-36
: Time-based calculation inmonthsSince
is straightforward.Ensure your assumption for 30 days per month is acceptable or consider a more precise approach (e.g., difference in month and year or using date libraries) for certain edge cases.
242-263
:start
function orchestrates everything well.The function gracefully handles data fetching from multiple queries. Any expansions to concurrency or data validation can be integrated here if performance or data consistency become concerns.
types/scripts/dashboard.ts (2)
1-54
: Well-structured basic entity interfacesThe basic entity interfaces are well designed with clear property definitions that model GitHub API entities effectively. The structure follows TypeScript best practices.
Consider adding JSDoc comments to document the purpose of each interface, which would improve maintainability.
+/** + * Interface representing GitHub API rate limit information + */ interface RateLimit { limit: number; cost: number; remaining: number; resetAt: string; }
87-101
: Specialized entity interfacesThe interfaces extending
BasicIssueOrPR
are appropriately designed for their specific use cases. However, there's potential duplication betweenHotDiscussionsIssuesNode
andHotDiscussionsPullRequestsNode
that could be refactored.Consider creating a common interface for the shared properties:
+interface HotDiscussionsBase extends BasicIssueOrPR { + timelineItems: TimelineItems; + reactions: Reactions; + comments: Comments; + reviews: Reviews; +} + -export interface HotDiscussionsIssuesNode extends BasicIssueOrPR { - timelineItems: TimelineItems; - reactions: Reactions; - comments: Comments; - reviews: Reviews; +export interface HotDiscussionsIssuesNode extends HotDiscussionsBase { } -export interface HotDiscussionsPullRequestsNode extends BasicIssueOrPR { - timelineItems: TimelineItems; - reactions: Reactions; - reviews: Reviews; - comments: Comments; +export interface HotDiscussionsPullRequestsNode extends HotDiscussionsBase { }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (90)
components/languageSelector/LanguageSelect.tsx
(1 hunks)components/navigation/MobileNavMenu.tsx
(1 hunks)jest.config.js
(1 hunks)next-i18next.config.cjs
(1 hunks)next-i18next.config.js
(0 hunks)package.json
(5 hunks)pages/_document.tsx
(1 hunks)postcss.config.cjs
(1 hunks)scripts/adopters/index.js
(0 hunks)scripts/adopters/index.ts
(1 hunks)scripts/build-docs.js
(0 hunks)scripts/build-docs.ts
(1 hunks)scripts/build-meetings.js
(0 hunks)scripts/build-meetings.ts
(1 hunks)scripts/build-newsroom-videos.js
(0 hunks)scripts/build-newsroom-videos.ts
(1 hunks)scripts/build-pages.ts
(1 hunks)scripts/build-post-list.js
(0 hunks)scripts/build-post-list.ts
(1 hunks)scripts/build-rss.js
(0 hunks)scripts/build-rss.ts
(1 hunks)scripts/build-tools.js
(0 hunks)scripts/build-tools.ts
(1 hunks)scripts/casestudies/index.js
(0 hunks)scripts/casestudies/index.ts
(1 hunks)scripts/compose.ts
(2 hunks)scripts/dashboard/build-dashboard.js
(0 hunks)scripts/dashboard/build-dashboard.ts
(1 hunks)scripts/dashboard/issue-queries.ts
(2 hunks)scripts/finance/index.js
(0 hunks)scripts/finance/index.ts
(1 hunks)scripts/index.ts
(2 hunks)scripts/markdown/check-edit-links.ts
(5 hunks)scripts/markdown/check-markdown.js
(0 hunks)scripts/markdown/check-markdown.ts
(1 hunks)scripts/tools/categorylist.js
(0 hunks)scripts/tools/categorylist.ts
(1 hunks)scripts/tools/combine-tools.js
(0 hunks)scripts/tools/combine-tools.ts
(1 hunks)scripts/tools/extract-tools-github.js
(0 hunks)scripts/tools/extract-tools-github.ts
(1 hunks)scripts/tools/tags-color.js
(0 hunks)scripts/tools/tags-color.ts
(1 hunks)scripts/tools/tools-object.js
(0 hunks)scripts/tools/tools-object.ts
(1 hunks)scripts/tools/tools-schema.json
(1 hunks)scripts/utils.js
(0 hunks)scripts/utils.ts
(1 hunks)scripts/utils/logger.ts
(1 hunks)scripts/utils/readAndWriteJson.js
(0 hunks)scripts/utils/readAndWriteJson.ts
(1 hunks)tests/adopters/index.test.js
(1 hunks)tests/babel.test.config.cjs
(1 hunks)tests/build-docs/addDocButtons.test.js
(7 hunks)tests/build-docs/buildNavTree.test.js
(4 hunks)tests/build-docs/convertDocPosts.test.js
(1 hunks)tests/build-meetings.test.js
(1 hunks)tests/build-newsroom-videos.test.js
(1 hunks)tests/build-pages.test.js
(2 hunks)tests/build-post-list.test.js
(10 hunks)tests/build-rss.test.js
(7 hunks)tests/build-tools.test.js
(1 hunks)tests/casestudies/index.test.js
(3 hunks)tests/dashboard/build-dashboard.test.js
(6 hunks)tests/finance/index.test.js
(2 hunks)tests/fixtures/combineToolsData.js
(4 hunks)tests/fixtures/dashboardData.js
(1 hunks)tests/index.test.js
(1 hunks)tests/markdown/check-edit-links.test.js
(2 hunks)tests/markdown/check-markdown.test.js
(1 hunks)tests/readAndWriteJson.test.js
(1 hunks)tests/tools/combine-tools.test.js
(6 hunks)tests/tools/extract-tools-github.test.js
(1 hunks)tests/tools/tools-object.test.js
(5 hunks)tests/utils.test.js
(2 hunks)tsconfig.json
(1 hunks)types/packages/jgexml__json2xml.d.ts
(1 hunks)types/packages/json.d.ts
(1 hunks)types/packages/markdown-toc.d.ts
(1 hunks)types/packages/node-fetch-2.d.ts
(1 hunks)types/scripts/build-docs.ts
(1 hunks)types/scripts/build-posts-list.ts
(1 hunks)types/scripts/build-rss.ts
(1 hunks)types/scripts/dashboard.ts
(1 hunks)types/scripts/tools.ts
(1 hunks)utils/getStatic.ts
(1 hunks)utils/getUniqueCategories.ts
(1 hunks)utils/languageDetector.ts
(1 hunks)utils/redirect.ts
(2 hunks)utils/staticHelpers.ts
(1 hunks)
💤 Files with no reviewable changes (19)
- scripts/tools/extract-tools-github.js
- scripts/tools/categorylist.js
- scripts/utils.js
- next-i18next.config.js
- scripts/adopters/index.js
- scripts/finance/index.js
- scripts/build-meetings.js
- scripts/utils/readAndWriteJson.js
- scripts/build-rss.js
- scripts/tools/tags-color.js
- scripts/tools/tools-object.js
- scripts/build-docs.js
- scripts/casestudies/index.js
- scripts/tools/combine-tools.js
- scripts/build-post-list.js
- scripts/build-newsroom-videos.js
- scripts/markdown/check-markdown.js
- scripts/build-tools.js
- scripts/dashboard/build-dashboard.js
✅ Files skipped from review due to trivial changes (20)
- pages/_document.tsx
- components/languageSelector/LanguageSelect.tsx
- utils/staticHelpers.ts
- postcss.config.cjs
- tests/utils.test.js
- utils/getStatic.ts
- components/navigation/MobileNavMenu.tsx
- types/packages/json.d.ts
- tests/babel.test.config.cjs
- scripts/tools/categorylist.ts
- utils/redirect.ts
- tests/build-docs/convertDocPosts.test.js
- tests/finance/index.test.js
- tests/build-docs/addDocButtons.test.js
- tests/adopters/index.test.js
- tests/fixtures/dashboardData.js
- scripts/tools/tags-color.ts
- tests/fixtures/combineToolsData.js
- tests/index.test.js
- utils/languageDetector.ts
🧰 Additional context used
🪛 Biome (1.9.4)
scripts/tools/extract-tools-github.ts
[error] 60-60: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
tests/build-meetings.test.js
[error] 74-74: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
tests/build-newsroom-videos.test.js
[error] 104-104: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
scripts/build-meetings.ts
[error] 62-62: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
tests/tools/extract-tools-github.test.js
[error] 90-90: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🪛 GitHub Actions: PR testing - if Node project
tests/dashboard/build-dashboard.test.js
[error] 142-142: Test failed: Expected: 2, Received: 1 in 'should calculate months since date'.
🔇 Additional comments (169)
tsconfig.json (5)
3-7
: Improved Readability forlib
Array.
Splitting thelib
array into multiple lines enhances readability and makes it easier to manage in the future without affecting functionality.
14-14
: Addition of Target ES2022.
Introducing"target": "ES2022"
ensures that the TypeScript compiler uses modern ECMAScript features, improving compatibility and performance.
21-23
: Enhanced Formatting forpaths
.
The multi-line formatting of thepaths
configuration improves clarity while preserving its intended module resolution logic.
26-31
: Refinedinclude
Array Formatting.
Reformatting theinclude
array into a multi-line structure increases readability and makes it straightforward to add or modify file patterns in the future.
32-35
: Refinedexclude
Array Formatting.
The multi-line structure for theexclude
array enhances maintainability and clarity, ensuring that directory exclusions are easily understood.next-i18next.config.cjs (2)
1-3
: Good use of comments explaining the file naming requirements.The comments clearly explain why the file uses the .cjs extension, including a GitHub issue reference for future maintainers. Setting the environment variable is also essential for proper configuration loading.
5-17
: Configuration structure looks good.The i18n configuration correctly defines locales, namespaces, and React integration options. The langMap object provides a clean way to reference human-readable language names.
utils/getUniqueCategories.ts (1)
9-21
: Good TypeScript migration with improved iteration method.The function has been properly converted to TypeScript with appropriate type annotations. Switching from a for...in loop to Object.keys().forEach() is a positive change that improves code readability and maintainability.
types/packages/node-fetch-2.d.ts (1)
1-3
: Clean and effective type declaration.This type declaration file properly defines the module for 'node-fetch-2', exporting a fetch function with the correct typing, which will enhance type safety in TypeScript files that use this package. Great addition as part of the migration effort.
scripts/dashboard/issue-queries.ts (1)
1-1
: Proper migration from CommonJS to ES Module syntax.The export statement has been successfully updated from CommonJS (
exports.Queries
) to ES Module syntax (export const Queries
), which is appropriate for TypeScript files. This change aligns with the TypeScript migration objective of the PR.scripts/adopters/index.ts (1)
1-8
: Good setup of ES Module imports and file path resolution.The code properly imports necessary modules and sets up path resolution using the recommended pattern with
fileURLToPath
andimport.meta.url
, which is the correct approach for ES Modules.scripts/utils/logger.ts (2)
1-16
: Well-configured logger with good formatting options.The Winston logger is well-configured with appropriate formatting options including colorization, timestamps, and alignment. Using an environment variable for the log level is a good practice for flexibility across different environments.
18-18
: Consider named export instead of default export.The named export is appropriate here as it allows for potential additional exports from this module in the future. This follows good TypeScript practices.
types/scripts/build-docs.ts (1)
3-12
: Well-structured type definitions for navigation treeThe type definitions are clear and follow TypeScript best practices. The recursive structure allows for properly modeling hierarchical navigation data, and the use of the ESLint disable comment is appropriate since
RecursiveChildren
is referenced before it's defined.types/packages/jgexml__json2xml.d.ts (1)
1-9
: Clean declaration file for the json2xml moduleThe type declaration follows proper conventions for third-party library definitions. Parameter types are well-defined, providing good type safety for consumers of this module.
scripts/casestudies/index.ts (1)
1-5
: Good job on properly importing modules!The imports are well organized, separating Node.js built-in modules from local utility functions.
scripts/index.ts (2)
1-13
: Well-structured ES module imports and path handlingGood job migrating from CommonJS to ES modules with proper imports. The
fileURLToPath
anddirname
usage to replace__dirname
is the correct approach for ES modules.
26-30
: Good update of path resolution using the new ES module path variablesThe path resolution has been correctly updated to use
currentDirPath
instead of__dirname
.types/scripts/build-rss.ts (1)
1-40
: Well-structured type definitions for RSS feedThe TypeScript type definitions are clear, comprehensive, and accurately represent RSS feed structures. Good job on the nested type definitions and representing the XML structure in TypeScript.
However, I notice that the implementation of the
rssFeed
function mentioned in the AI summary is not present in this file. Is this intentional, or should the implementation be included here?tests/casestudies/index.test.js (2)
24-24
: Consistent arrow function formattingGood job on maintaining consistent formatting in the updated code.
74-74
: Consistent quote styleThe switch to single quotes maintains consistency with the codebase style.
tests/build-pages.test.js (1)
55-56
: Good test cleanup practice.Adding cleanup code to remove the test directory after the test is a good practice that prevents test pollution and ensures a clean environment for future test runs.
tests/build-docs/buildNavTree.test.js (2)
1-1
: Update import path to handle TypeScript migration.The import statement has been updated to reference the TypeScript version of the build-docs script, which is consistent with the migration goals. Similar to the other test file, importing a TypeScript file directly in a JavaScript test requires proper Jest configuration.
Make sure your Jest configuration is properly set up to handle TypeScript imports in JavaScript test files, typically using ts-jest or a similar setup.
20-21
: Consistent change from bracket to dot notation.The code has been updated to use dot notation (e.g.,
result.welcome
) instead of bracket notation (e.g.,result['welcome']
). This change is consistent with TypeScript's ability to provide better type safety and autocompletion when properties are properly typed.The change suggests that the migrated TypeScript code now has proper type definitions for the return values, making dot notation safer to use. This is a good side effect of the TypeScript migration.
Also applies to: 37-37, 44-44, 51-52, 58-58, 65-66, 113-113, 118-119
jest.config.js (4)
1-4
: Configuration updated for TypeScript support.The Jest configuration has been properly updated to transform TypeScript files using babel-jest. This is essential for the migration to TypeScript.
9-10
: Coverage patterns updated for TypeScript files.The collectCoverageFrom and coveragePathIgnorePatterns have been correctly updated to target TypeScript files instead of JavaScript files.
11-12
: Added TypeScript file extensions to module extensions list.The moduleFileExtensions property has been appropriately configured to include both TypeScript and JavaScript file extensions.
14-15
: Updated export syntax to match ES modules.Export syntax has been updated from CommonJS
module.exports
to ES modulesexport default
. This aligns with the TypeScript module system.tests/build-rss.test.js (4)
24-24
: Added parentheses in arrow function for consistency.The code now consistently uses parentheses in arrow functions, which improves readability.
55-56
: Added semicolons for consistency.Semicolons have been added at the end of statements for consistent code style.
110-116
: Improved readability with consistent indentation.The mock configuration has been reformatted with proper indentation, making it more readable.
142-144
: Improved error message format for clarity.The error message now includes the post title, which makes debugging easier when a post is missing date information.
scripts/build-tools.ts (4)
1-9
: Good organization of imports.The imports are well-organized, separating Node.js built-ins from local imports with a blank line.
22-35
: Proper error handling with TypeScript type assertion.The function includes proper error handling with a try-catch block and correctly casts the error to Error type when accessing the message property.
37-48
: Clean implementation of command-line execution logic.The script properly checks if it's being executed directly and provides clear error handling with process exit. The istanbul ignore comment ensures that this code is excluded from test coverage.
50-50
: Clean export syntax.The function is exported using named export syntax, which is the preferred approach in TypeScript.
tests/build-post-list.test.js (5)
46-48
: Improved object formatting for better readability.The test object assertions have been reformatted for better readability and consistency.
Also applies to: 52-53, 56-58
70-72
: Consistent formatting of test expectations.The formatting of expectations has been improved for better readability and consistency.
Also applies to: 80-82
87-88
: Improved variable naming for better clarity.The variable name now more clearly describes what it's finding in the test data.
133-135
: More specific error message expectations.The test now expects more specific error messages, which makes the tests more accurate and easier to maintain.
Also applies to: 180-182, 190-192, 196-198
222-223
: Consistent spacing in test organization.Added consistent spacing between test cases for better readability.
Also applies to: 228-229
types/scripts/build-posts-list.ts (2)
9-12
: Validate external usage of optionalhref
property.
NavigationPage
types ahref
property as optional. Ensure any consumers ofNavigationPage
handle cases whenhref
is undefined, avoiding runtime errors.
37-42
: Confirm that all sections of the code properly initialize and handle thedocsTree
.
Result
includesdocsTree
as aNavTree
. Verify that any code populating it is aligned with the structure used inbuild-docs.ts
to maintain consistency.tests/tools/combine-tools.test.js (5)
31-31
: Consider verifying your AJV rule coverage.Here, the mocked compile function only checks if
data.title !== 'Invalid Tool'
. Ensure that other validation rules (e.g., schema consistency and required fields) are tested elsewhere to avoid missing coverage on complex validation scenarios.
98-98
: Testing deeper properties withtoEqual
is effective.Your usage of
expect(tagsData).toEqual(expectedDataT1)
is appropriate for ensuring structural equivalence. This is a solid approach to confirm the entire object matches expectations.
112-112
: Check for empty or missing tool titles.When mapping over
combinedTools.category1.toolsList
, ensure no unexpectedtitle
is missing or null. Consider adding a fallback title or a validation check.
214-214
: Ensure coverage of invalid inputs and edge cases.These tests covering invalid paths, invalid categories, invalid URLs, and circular references are comprehensive. Good job ensuring robust error handling.
Also applies to: 218-220, 224-226, 231-234
235-242
: Validate filtering or default values ingetFinalTool
.This test ensures missing data and filters are properly handled. Confirm that downstream usage of
finalToolWithMissingData
does not assume properties that might be absent.scripts/build-post-list.ts (3)
49-60
: Check for locale or special characters incapitalize
.Your
capitalize
approach splits on whitespace and hyphens. Ensure it properly handles words with Unicode or apostrophes (e.g., names with diacritics).
62-86
: Validate sections carefully before adding tofinalResult
.Within
addItem
, ifdetails.slug
is missing or invalid, the function throws an error. This is good for data integrity. Ensure that the calling code is prepared to handle these exceptions, possibly with try/catch or pre-validation.
126-134
: Ensure robust error handling forstat
calls.
isDirectory
usesawait stat(dir)
. Ifstat
fails (e.g., missing permissions), it will throw. Validate the usage ensures we catch those errors upstream as well.scripts/tools/combine-tools.ts (2)
20-23
: Initialize Ajv properly with necessary plugins
Ajv is correctly instantiated, andaddFormats
is used to enable URI format validation. This ensures that the schema validations for tool objects can handle various string formats, reducing potential issues from invalid data.
56-65
: Use consistent data type for filters
The function handles both strings and arrays fortoolObject.filters.language
, which is a flexible approach. However, be sure that downstream code or other functions can also handle the same variability. Otherwise, consider enforcing a single consistent data type to reduce potential complexity.tests/build-tools.test.js (2)
6-7
: Confirm TypeScript imports are recognized by the test runner
Switching from a JavaScript path to a TypeScript file (build-tools.ts
) may require updates or configurations in the testing framework (e.g., Jest’stransform
rules or ts-node usage) to ensure the tests can correctly load and run the TypeScript code.
44-45
: Environment variables usage
Settingprocess.env.GITHUB_TOKEN
in thebeforeAll
hook is helpful for mocking. Double-check if other tests or scripts rely on this environment variable, as they might get affected by the globally set token.types/packages/markdown-toc.d.ts (1)
1-46
: Well-structured type declaration file for markdown-tocThis is a well-organized TypeScript declaration file that properly defines the types for the
markdown-toc
module. The interfaces are comprehensive and cover all the necessary properties with appropriate types.scripts/build-newsroom-videos.ts (5)
1-9
: Good use of ES module imports with explicit typesYour imports follow best practices for TypeScript modules - importing specific types with the
type
keyword and using explicit named imports.
22-24
: Good environment variable validationThe function properly validates the presence of the required environment variable before proceeding with the API call.
47-54
: Well-typed data transformation using TypeScript genericsThe function correctly uses TypeScript's type system to ensure type safety when transforming the YouTube API response.
63-65
: Proper error handling with type assertionThe error handling correctly uses type assertion to safely access the error message.
68-71
: Direct execution support with import.meta.urlGood use of ES modules'
import.meta.url
withfileURLToPath
for determining the current file path when running the script directly.tests/dashboard/build-dashboard.test.js (5)
14-14
: Update file extension in import pathThe import path now includes
.ts
extension to reference the TypeScript file, which is good for clarity.
23-27
: Good practice mocking the loggerYou've properly imported and mocked the logger module for testing.
84-86
: Replaced console.log with logger.warnThe test has been updated to verify logger.warn calls instead of console.log, which is consistent with the TypeScript migration.
117-117
: Replaced console.error with logger.errorThe test has been updated to verify logger.error calls instead of console.error, which is consistent with the TypeScript migration.
188-188
: Replaced console.error with logger.errorAnother instance of updating the test to verify logger.error calls, which is consistent with the TypeScript migration.
scripts/build-pages.ts (3)
1-3
: Good module imports with type separationThe imports correctly use ES Module syntax with separate imports for types. This follows TypeScript best practices.
28-28
: Good type annotations for callback parametersThe callback function parameters are properly typed, which enhances type safety.
51-52
: Ensure test coverage with Istanbul directiveGood use of the Istanbul ignore directive to maintain test coverage metrics.
tests/build-newsroom-videos.test.js (5)
5-6
: Updated imports to reference TypeScript filesThe import paths have been correctly updated to reference the TypeScript files with the
.ts
extension, which aligns with the migration objective of this PR.
14-18
: Well-structured test setupGood addition of the
beforeAll
hook to set up the test environment including the temporary directory, test file, and environment variables.
24-26
: Improved test isolation with beforeEachThe addition of
beforeEach
to clear mock calls before each test is a good practice to ensure test isolation and prevent cross-test interference.
28-49
: Enhanced test structure for clarityThe test case structure has been improved with clearer assertions and better organization of the expected URL construction.
51-109
: Comprehensive error handling test casesExcellent addition of test cases covering various error scenarios:
- Fetch errors
- Invalid API responses
- HTTP status codes
- File write errors
- Missing environment variable
This significantly improves test coverage and ensures the function is robust against failure modes.
🧰 Tools
🪛 Biome (1.9.4)
[error] 104-104: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
tests/readAndWriteJson.test.js (3)
1-3
: Updated imports for TypeScript migrationThe import statements have been properly updated to:
- Use the modern
fs/promises
import directly- Reference TypeScript files with
.ts
extension- Use destructuring for the
writeJSON
importThese changes align well with the TypeScript migration objective.
6-9
: Simplified mocking approach for fs/promisesThe mock implementation has been correctly updated to mock
fs/promises
directly, which matches the updated import statement on line 1.
12-12
: Consistent mocking patternThe mock for
convertToJson
maintains consistency with the new import structure, using the destructured format.tests/build-meetings.test.js (4)
2-5
: Updated imports for TypeScript migrationThe imports have been correctly updated to reference TypeScript files with the
.ts
extension, and the path module is now imported separately at the top.
7-23
: Improved mock structure for clarityThe mock implementation for googleapis has been restructured into a more readable format with the mockGoogle variable. This enhances code maintainability.
26-39
: Better test setup and teardownThe test setup has been improved with:
- Clearer test directory path construction using path.join
- Clear mock resetting in beforeEach
- Proper environment variable setup
- Comprehensive cleanup in afterEach
This ensures tests run in isolation and clean up after themselves.
63-146
: Comprehensive error handling test casesExcellent addition of test cases that cover:
- Google API failures
- Missing environment variables
- Authentication failures
- File write errors
- Invalid data structures
- Missing required fields in events
These tests ensure the function is robust against various failure modes.
🧰 Tools
🪛 Biome (1.9.4)
[error] 74-74: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
tests/tools/extract-tools-github.test.js (6)
2-3
: Updated imports for TypeScript migrationThe import statements have been properly updated to reference TypeScript files with the
.ts
extension.
5-7
: Added logger mockGood addition of a mock for the logger, which allows testing and verification of logging behavior.
12-14
: Environment setup in beforeAllGood practice to set up environment variables in the beforeAll hook to ensure consistent test environment.
18-25
: Updated mock response formatThe mock data structure has been updated to better reflect the actual GitHub API response format with items array and total_count.
42-81
: Added pagination test caseExcellent addition of a test case for paginated responses, which:
- Verifies handling of multiple pages
- Checks logging of page numbers
- Validates correct API calls for each page
- Ensures all items are properly combined
This significantly improves test coverage for real-world API interaction.
89-95
: Added environment variable validation testGood addition of a test to verify the function properly checks for the required GITHUB_TOKEN environment variable.
🧰 Tools
🪛 Biome (1.9.4)
[error] 90-90: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
tests/markdown/check-edit-links.test.js (4)
10-10
: Switching to TypeScript module import is consistent with the PR goal
This line updates the import source to reference the new TypeScript file. This change aligns with the ongoing migration to TypeScript, ensuring consistency across the codebase.
12-12
: Adopting a dedicated logger is a great enhancement
Replacing direct console outputs with a logger utility centralizes logging, making it easier to manage log levels and formatting.
150-150
: Verifying success message logging
Verifying thatlogger.info
is called with "All URLs are valid." is consistent with the updated logging approach.
158-158
: Verifying 404 message logging
This test ensures the logger captures invalid URLs. The string check is flexible enough to detect partial matches, preventing brittle test failures on slight message changes.scripts/compose.ts (6)
5-10
: Great transition to TypeScript imports
Switching from CommonJSrequire
to ES moduleimport
statements is consistent with TypeScript best practices. Ensure all consumed packages properly expose default exports for these imports.
12-21
: Clear type definition for ComposePromptType
Defining an explicit type for prompt answers improves maintainability and static type checking, preventing accidental omission of required fields (e.g.,title
,excerpt
).
121-143
: Appropriately specifying prompt question types
Usingtype: 'input'
and'list'
clarifies how the user interacts with each question. Consider adding default values or validation if necessary (e.g., ensuringtitle
isn't empty).
145-153
: Typed handling of prompt answers
Assigning(answers: ComposePromptType)
ensures thatgenFrontMatter
receives strongly typed input. The approach for sanitizing the file name is also helpful to avoid invalid paths.
157-159
: Clarify error handling during file writing
Throwing the error and logging the success message is fine, but catching and handling partial write failures might be helpful. If needed, verify that partial files do not remain after a failure.
161-168
: Improved error messaging for different prompt failures
Logging separate messages for TTY errors vs. generic errors helps diagnose issues more quickly. Ensure sensitive info is not being logged.scripts/tools/tools-object.ts (2)
1-4
: Informative import structure for external libraries
The usage ofAjv
,addFormats
,axios
, andFuse
is neatly organized. Ensure these dependencies are listed in the project's dependencies and remain compatible with the TypeScript setup.
6-11
: Type imports centralize domain data definitions
By importing and usingAsyncAPITool
,ToolsData
, andToolsListObject
from a dedicated@/types/scripts/tools
file, you enhance clarity regarding function inputs and outputs.tests/tools/tools-object.test.js (13)
2-2
: Use.ts
Extension for TypeScript Migration
Great job updating the import path to point to the.ts
file.
10-10
: Logger Import
Importing the logger from a TypeScript file is consistent with the migration efforts.
12-14
: Mocking Logger
The logger mock is properly set up.
20-20
: Added 'Others' Category
Declaring the "Others" category is a good approach to group uncategorized tools.
51-51
: VerifiedcreateToolObject
Parameter List
Passes four parameters (toolFile
,repoUrl
, description, and boolean), which aligns with the updated TypeScript function signature.
145-145
: Describe Error Handling Logic
This test name clearly indicates the intent to throw ifaxios.get
fails. Good coverage.
146-151
: Mock Data Definition
UsingcreateMockData
with a single repository record is consistent and covers the error scenario effectively.
166-166
: Empty Description Case
Providing an empty description string helps verify the fallback logic for descriptions.
168-168
: Blank Line
No relevant change.
170-170
: Mock Data for Missing Description
Creates a realistic scenario for testing fallback behavior.
171-176
: Repository Description Fallback
Storing the computed repository description increateMockData
ensures coverage for missing tool descriptions.
177-177
:axios.get.mockResolvedValue
Correctly simulates the response for the tested tool file.
179-179
: Converting Tools
Verifies the final result fromconvertTools
to ensure the fallback logic is applied.scripts/build-meetings.ts (13)
1-10
: Imports and Directory Resolution
The imports (fs
,googleapis
,path
,url
, andlogger
) are standard and well organized.
11-16
: JSDoc Annotations
Clear and concise JSDoc forbuildMeetings
. Nicely done.
17-20
: Function Declaration
DefiningbuildMeetings
with a typed parameter (writePath: string
) follows TypeScript best practices.
32-33
: Variable Declaration
StoringeventsItems
for later assignment is straightforward and readable.
34-45
: Wide Time Range
You’re fetching events from 100 days in the past to 30 days in the future. Confirm that this wide range is intended.
46-49
: Validating Calendar Response
CheckingeventsList.data.items
ensures robust error handling against malformed or empty responses.
67-68
: Stringify for Human-Readable Output
Serializing with an indentation of two spaces is a good choice for readability.
69-69
: Informative Logging
Usinglogger.info
to display fetched events is helpful for diagnosing issues or confirming successful runs.
71-71
: Writing JSON to File
writeFileSync
is appropriate for small to medium-sized data. No concerns here.
73-74
: Error Handling
Throwing a new error with details about the cause is clear and helps diagnose issues quickly.
75-76
: Function End
No additional logic here.
77-81
: Conditional Script Execution
InvokingbuildMeetings
when run directly is a neat pattern for CLI-like usage.
82-82
: Export Statement
ExportingbuildMeetings
is consistent with the new TypeScript setup, making it reusable elsewhere.scripts/build-docs.ts (2)
43-48
: Validate parent section availability more gracefully.
Throwing an error is correct when a specified parent doesn’t exist. However, you might wish to expand the error message or handle it gracefully depending on user input or partial tree data.
69-111
: Ensure sub-sections are always expected to have children.
The logic to sort and handle children is well-structured. However, confirm that empty or dynamic sub-sections don’t cause unhandled cases or unexpected ordering. It may be beneficial to consider edge cases where no children exist.tests/markdown/check-markdown.test.js (1)
13-15
: Mock logger usage looks correct.
Usingjest.mock
for the logger is a clean approach that confirms calls tologger.warn
andlogger.error
without polluting test output.scripts/markdown/check-markdown.ts (2)
45-99
: Gracefully handle frontmatter for partial or future expansions.
While the validations are robust, if new properties are added to frontmatter (cover tags, authors, etc.), the rigid checks might break. Consider logging warnings rather than errors for unknown fields, based on project criteria.
182-197
: Robust error handling in main function.
Catching errors and exiting with an error code is a straightforward approach. Ensure that any resources or state are cleaned up if new functionalities or parallel processes are ever introduced.types/scripts/tools.ts (10)
1-5
: Interface structure looks good.The
Links
interface is straightforward and captures optional links in a clean manner. No issues found.
27-39
: LGTM for base item definitions.The
CategoryListItem
andLanguageColorItem
types are well-defined and straightforward. No issues noted.
40-46
: Well-structured filter types.The
Filters
interface is concise. Consider ensuring that arrays forlanguage
andtechnology
are always defined in your usage, or handle undefined checks.
48-54
: Separation of FinalFilters is reasonable.It’s clear how the final interface differs by making certain properties mandatory. This approach appears consistent and intentional.
56-61
: Base tool type is clean.The
BaseAsyncAPITool
type effectively captures core properties. No issues found.
63-70
: Extended tool interfaces are coherent.The
AsyncAPITool
andFinalAsyncAPITool
definitions, including the transition from optional to required fields, align well with the intended usage.
72-81
: Repository type is well-defined.Consistent naming and adhered structure. No concerns.
82-88
: Clear separation of tool item data.
ToolItem
ties together repository info with a path and name. Looks good.
90-92
: ToolsData type is straightforward.No additional considerations here.
94-104
: Tools list types are well-designed.Your mapping from string keys to an object containing both
description
andtoolsList
is logical. Confirmation that it integrates seamlessly across other modules would be advisable, but at face value, everything appears consistent.scripts/markdown/check-edit-links.ts (8)
1-5
: Imports and ES module usage look fine.Noting
node-fetch-2
: ensure it's properly installed and recognized if you upgraded the version from a prior dependency.
6-14
: Check for robust coverage of ignored files.You declare
ignoreFiles
for specific Markdown files. Make sure these are intentionally excluded from link checking logic.
[approve]
16-20
: Strong typed structure for path objects.
PathObject
organizes data well for link checks. No issues found.
55-86
: Efficient chunk-based URL checking incheckUrls
.Your batch approach (default size = 5) helps manage concurrency. Logging statements are instructive. Nicely done.
95-113
: Conditional logic indetermineEditLink
is coherent.Implementation gracefully handles the fallback case for empty
value
. Just confirm coverage for unusual path edge-cases or deeply nested docs.
166-192
: Main function orchestrates link checks thoroughly.Graceful error handling with logs. Confirm that environment variables for
DOCS_LINK_CHECK_TIMEOUT
andDOCS_LINK_CHECK_BATCH_SIZE
are set or default to safe values in production.
195-197
: ES moduleif
check is consistent.Running
main()
if invoked directly is standard practice.
199-199
: Named exports are well-managed.Clear and explicit export of all needed functions.
scripts/dashboard/build-dashboard.ts (13)
1-5
: Dependency imports are correct.You appropriately leverage
@octokit/graphql
andfs/promises
. Confirm all typed dependencies are installed inpackage.json
.
6-15
: Type imports enhance clarity.Using explicit types from
@/types/scripts/dashboard
helps maintain a strongly typed codebase.
17-19
: Utility imports clock in well.Reusing
pause
andlogger
is consistent. No concerns here.
21-23
: Directory resolution technique looks solid.Using
fileURLToPath
anddirname
ensures stable ES modules behavior.
38-47
:getLabel
function is concise.Clean approach to extracting label. No issues, other than verifying that the label’s structure is always as expected (i.e.,
'something/something'
).
49-94
: GraphQL data fetch ingetDiscussions
is well-handled.The recursion for pagination (line 88) is a neat solution to handle multiple pages. Good job pausing to respect rate limits.
96-117
:getDiscussionByID
is logically similar.Handling PR vs Issue fetch with a single function is an efficient approach. Rate-limit handling or concurrency appears reliant on the calling code, which is acceptable.
119-168
:processHotDiscussions
elegantly aggregates interactions.Time-based weighting in the calculation is imaginative. Keep an eye on performance if the data set grows significantly, but your approach is likely fine for typical usage.
170-193
: Nicely chunked approach ingetHotDiscussions
.Processing five items at a time is a safe concurrency strategy. Sorting and slicing to top 12 ensures a manageable result.
195-219
:writeToFile
function thoroughly logs errors.This helps debugging. The structure for rethrowing is also good. No issues found.
221-240
: Mapping functionmapGoodFirstIssues
is straightforward.It extracts relevant fields from raw issues to a standard shape. Validation is minimal but presumably sufficient.
265-268
: Command-line invocation block is correct.Your approach is consistent with best practices for ES modules.
270-281
: Exports are well curated.All relevant pieces are exposed. This fosters maintainability in other modules.
package.json (4)
11-14
: Consistent migration to TypeScript executionThe scripts have been properly updated to use
tsx
for executing TypeScript files instead of usingnode
for JavaScript files.
22-28
: Complete script migration to TypeScriptAll script execution commands have been consistently updated to use
tsx
to run TypeScript files.
119-120
: New runtime dependencies for TypeScript supportAdded dependencies:
tsx
: Enables direct execution of TypeScript files without separate compilation stepwinston
: A structured logging library, likely used for improved logging in TypeScript codeThese additions support the TypeScript migration effectively.
124-148
:✅ Verification successful
Added TypeScript development dependencies
The added TypeScript-related dependencies are appropriate for the migration:
- Babel TypeScript preset for transpilation
- Type definitions for various libraries
- Transform plugin for handling import.meta
Consider adding a dedicated tsconfig.json file if not already present to configure TypeScript compilation options.
🏁 Script executed:
#!/bin/bash # Check if tsconfig.json exists and examine its contents if [ -f "tsconfig.json" ]; then echo "tsconfig.json exists:" cat tsconfig.json else echo "No tsconfig.json found. Consider adding one for proper TypeScript configuration." fiLength of output: 707
TypeScript Dependencies and Configuration Verified
The additions in the package.json—such as the Babel TypeScript preset, necessary type definitions, and the transform plugin—are appropriate for the migration. Our check confirms that a dedicated
tsconfig.json
already exists and is well-configured, so no further action is needed regarding TypeScript settings.types/scripts/dashboard.ts (4)
56-68
: Base interface for Issues and PRsThe
BasicIssueOrPR
interface provides a good foundation for both issues and pull requests, extracting common properties appropriately.Note that
__typename
follows snake_case while other properties use camelCase. This is likely intentional to match the GraphQL API response format.
69-85
: Composition pattern for complex entitiesGood use of TypeScript's intersection types (
&
) to combine the base interface with additional properties specific to pull requests and issues. This promotes code reuse and maintainability.
110-131
: Well-defined result interfacesThe
ProcessedDiscussion
andMappedIssue
interfaces provide clear structures for processed data, making the code more maintainable and type-safe.
38-54
:❓ Verification inconclusive
Inconsistent optional fields in collection interfaces
In the
Reviews
interface, thenodes
field is marked as optional with?
, while inComments
it's required. Consider whether this is intentional based on API behavior.
🏁 Script executed:
#!/bin/bash # Search for the usage of Reviews and Comments to verify consistent handling of optional nodes rg --no-heading "reviews\.(nodes|totalCount)" --type=ts echo "---" rg --no-heading "comments\.(nodes|totalCount)" --type=tsLength of output: 688
Inconsistent Optional Fields – Please Verify API Intent
The current definitions show that while
Comments.nodes
is required,Reviews.nodes
is marked optional. However, inscripts/dashboard/build-dashboard.ts
the code forcefully assertsdiscussion.reviews.nodes
(using the!
operator), suggesting that nodes are expected to be present for reviews. Please verify if:
- The API always provides review nodes—in which case, consider making
nodes
required forReviews
to avoid unnecessary non-null assertions.- Or if there’s a valid scenario where review nodes might be omitted—then update the calling code to safely handle a potentially undefined value.
Locations to review:
- Interface Definitions:
types/scripts/dashboard.ts
(lines 38–54)- Usage in Code:
scripts/dashboard/build-dashboard.ts
wherediscussion.reviews.nodes!
is used versusdiscussion.comments.nodes
(which is accessed directly).
export async function buildAdoptersList() { | ||
writeJSON('config/adopters.yml', resolve(currentDirPath, '../../config', 'adopters.json')); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Missing await for asynchronous operation.
The function is marked as async but doesn't await the result of the writeJSON
call, which could lead to the function returning before the write operation completes.
export async function buildAdoptersList() {
- writeJSON('config/adopters.yml', resolve(currentDirPath, '../../config', 'adopters.json'));
+ await writeJSON('config/adopters.yml', resolve(currentDirPath, '../../config', 'adopters.json'));
}
📝 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.
export async function buildAdoptersList() { | |
writeJSON('config/adopters.yml', resolve(currentDirPath, '../../config', 'adopters.json')); | |
} | |
export async function buildAdoptersList() { | |
await writeJSON('config/adopters.yml', resolve(currentDirPath, '../../config', 'adopters.json')); | |
} |
export async function getData(): Promise<ToolsData> { | ||
// eslint-disable-next-line no-useless-catch | ||
try { | ||
if (!process.env.GITHUB_TOKEN) { | ||
throw new Error('GITHUB_TOKEN environment variable is required'); | ||
} | ||
const allItems = []; | ||
let page = 1; | ||
|
||
const maxPerPage = 50; | ||
const getReqUrl = (PerPage: number, pageNo: number) => | ||
`https://api.github.com/search/code?q=filename:.asyncapi-tool&per_page=${PerPage}&page=${pageNo}`; | ||
const headers = { | ||
accept: 'application/vnd.github.text-match+json', | ||
authorization: `token ${process.env.GITHUB_TOKEN}` | ||
}; | ||
const result = await axios.get(getReqUrl(maxPerPage, page), { | ||
headers | ||
}); | ||
const totalResults = result.data.total_count; | ||
|
||
allItems.push(...result.data.items); | ||
|
||
while (allItems.length < totalResults) { | ||
page++; | ||
|
||
logger.info(`Fetching page: ${page}`); | ||
// pause for 1 second to avoid rate limiting | ||
await pause(1000); | ||
const nextPageData = await axios.get(getReqUrl(maxPerPage, page), { | ||
headers | ||
}); | ||
|
||
const { data } = nextPageData; | ||
|
||
allItems.push(...data.items); | ||
} | ||
|
||
result.data.items.push(...allItems); | ||
|
||
return result.data; | ||
} catch (err) { | ||
throw err; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix double-counting of results and remove unnecessary try-catch
There's a logical error where items from the first page are being counted twice, and the try-catch block is redundant since it just rethrows the error.
Apply these changes:
- // eslint-disable-next-line no-useless-catch
- try {
if (!process.env.GITHUB_TOKEN) {
throw new Error('GITHUB_TOKEN environment variable is required');
}
const allItems = [];
let page = 1;
const maxPerPage = 50;
const getReqUrl = (PerPage: number, pageNo: number) =>
`https://api.github.com/search/code?q=filename:.asyncapi-tool&per_page=${PerPage}&page=${pageNo}`;
const headers = {
accept: 'application/vnd.github.text-match+json',
authorization: `token ${process.env.GITHUB_TOKEN}`
};
const result = await axios.get(getReqUrl(maxPerPage, page), {
headers
});
const totalResults = result.data.total_count;
allItems.push(...result.data.items);
while (allItems.length < totalResults) {
page++;
logger.info(`Fetching page: ${page}`);
// pause for 1 second to avoid rate limiting
await pause(1000);
const nextPageData = await axios.get(getReqUrl(maxPerPage, page), {
headers
});
const { data } = nextPageData;
allItems.push(...data.items);
}
- result.data.items.push(...allItems);
+ // Items from the first page are already in allItems, no need to append them again
return result.data;
- } catch (err) {
- throw err;
- }
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 60-60: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add this suggestion as task inside #3764
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
@akshatnema I've created a new issue with the suggestion to fix the double-counting of results and remove the unnecessary try-catch block in the getData
function. The issue has been assigned to you and references both this PR (#3761) and issue #3764 as requested.
🏁 Script executed:
#!/bin/bash
# Create an issue with the suggestion from the comment
gh issue create \
--title "Fix double-counting of results and remove unnecessary try-catch in getData function" \
--body "## Issue
In the `scripts/tools/extract-tools-github.ts` file, there are two issues that need to be fixed:
1. There's a logical error where items from the first page are being counted twice
2. There's an unnecessary try-catch block that just rethrows the error
## Suggested fix
\`\`\`diff
- // eslint-disable-next-line no-useless-catch
- try {
if (!process.env.GITHUB_TOKEN) {
throw new Error('GITHUB_TOKEN environment variable is required');
}
const allItems = [];
let page = 1;
const maxPerPage = 50;
const getReqUrl = (PerPage: number, pageNo: number) =>
\`https://api.github.com/search/code?q=filename:.asyncapi-tool&per_page=\${PerPage}&page=\${pageNo}\`;
const headers = {
accept: 'application/vnd.github.text-match+json',
authorization: \`token \${process.env.GITHUB_TOKEN}\`
};
const result = await axios.get(getReqUrl(maxPerPage, page), {
headers
});
const totalResults = result.data.total_count;
allItems.push(...result.data.items);
while (allItems.length < totalResults) {
page++;
logger.info(\`Fetching page: \${page}\`);
// pause for 1 second to avoid rate limiting
await pause(1000);
const nextPageData = await axios.get(getReqUrl(maxPerPage, page), {
headers
});
const { data } = nextPageData;
allItems.push(...data.items);
}
- result.data.items.push(...allItems);
+ // Items from the first page are already in allItems, no need to append them again
return result.data;
- } catch (err) {
- throw err;
- }
\`\`\`
This issue is related to the TypeScript migration in PR #3761.
Originally identified in: https://github.com/asyncapi/website/pull/3761#discussion_r1976376063
Adding this as a task for issue #3764 as requested by @akshatnema." \
--assignee "akshatnema"
Length of output: 2142
@akshatnema I've created issue #3765 with the suggestions to fix the double-counting of results and remove the unnecessary try-catch block. The issue has been assigned to you and references both this PR (#3761) and issue #3764 as requested.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of creating a new issue, can't you edit existing issue that mentions this comment, in issue #3764 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@akshatnema You're right, and I apologize for the misunderstanding. I don't have the capability to edit existing issues directly - I can only create new ones or comment on existing threads.
For this situation, you have a few options:
- You could manually add the task to issue Refactor scripts code for better code quality #3764 by copying the relevant information
- You could close issue Fix double-counting of results and remove unnecessary try-catch in getData function #3765 with a comment referencing Refactor scripts code for better code quality #3764
- You could keep both issues but link them together
Sorry for not following your instructions correctly. In the future, I'll clarify first if you want me to add something to an existing issue rather than creating a new one.
(•‿•)
@@ -1,6 +1,6 @@ | |||
const fs = require('fs'); | |||
const path = require('path'); | |||
const { capitalizeJsxTags, copyAndRenameFiles, ensureDirectoryExists } = require('../scripts/build-pages'); | |||
const { capitalizeJsxTags, copyAndRenameFiles, ensureDirectoryExists } = require('../scripts/build-pages.ts'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Update import path to handle TypeScript migration.
The import statement has been updated to reference the TypeScript version of the build-pages script, which is consistent with the migration goals. However, importing a TypeScript file directly in a JavaScript test is unusual and may cause issues depending on your test configuration.
Make sure your Jest configuration is properly set up to handle TypeScript imports in JavaScript test files. Usually, this is handled by ts-jest or a similar transpilation setup in your Jest configuration.
🏁 Script executed:
#!/bin/bash
# Check if Jest is configured to handle TypeScript imports in JavaScript tests
grep -r "ts-jest" package.json jest.config.js 2>/dev/null || echo "No ts-jest configuration found"
Length of output: 116
Action Required: Update Jest Configuration for TypeScript Imports
The updated import path now references the TypeScript version of the build-pages script as intended for migration. However, our verification revealed that there's no configuration for ts-jest (or an equivalent TypeScript transpiler) in your Jest setup. Importing a TypeScript file directly from a JavaScript test may lead to runtime issues.
- Jest Configuration Issue:
- No
ts-jest
configuration was found in yourpackage.json
orjest.config.js
. - Ensure your Jest configuration is updated to handle TypeScript imports—either by configuring ts-jest or an alternative transpilation solution.
- No
Please update your Jest configuration accordingly to avoid potential test failures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of ts-jest We are using babel to transform the typescript files. Here is the config file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
@JeelRajodiya I see that you're using Babel to transform TypeScript files instead of ts-jest. Upon checking your configuration:
- The
jest.config.js
file has been updated with atransform
property that usesbabel-jest
for both.ts
and.js
files. - You've added
tests/babel.test.config.cjs
with the necessary Babel presets, specifically@babel/preset-env
and@babel/preset-typescript
. - The appropriate dependencies have been added to
package.json
to support this setup.
This configuration is valid and should properly handle the TypeScript imports in your JavaScript test files. Your approach of using Babel instead of ts-jest is a common alternative and should work well for your TypeScript migration.
🏁 Script executed:
#!/bin/bash
# Check the Jest configuration file for transform settings
if [ -f "jest.config.js" ]; then
echo "=== jest.config.js content ==="
cat jest.config.js
fi
# Check the Babel test config file
if [ -f "tests/babel.test.config.cjs" ]; then
echo -e "\n\n=== tests/babel.test.config.cjs content ==="
cat tests/babel.test.config.cjs
fi
# Check if any Jest-related transform is configured in package.json
if [ -f "package.json" ]; then
echo -e "\n\n=== Jest and Babel related entries in package.json ==="
grep -A 10 "\"jest\":" package.json 2>/dev/null || echo "No jest configuration in package.json"
grep -A 3 "\"@babel/preset-typescript\"" package.json 2>/dev/null
fi
Length of output: 1737
@JeelRajodiya Thank you for the clarification. I can see that your project is indeed using Babel to transform TypeScript files in tests rather than ts-jest. Your configuration looks correct with:
- A transform pattern in jest.config.js that uses babel-jest for both TS and JS files
- A dedicated babel.test.config.cjs with @babel/preset-typescript
- The necessary dependencies in package.json
This approach is perfectly valid and should handle TypeScript imports in your JavaScript test files without issues. The import change to require('../scripts/build-pages.ts')
is aligned with your migration strategy.
let specWeight = 100; | ||
const finalResult: Result = { | ||
docs: [], | ||
blog: [], | ||
about: [], | ||
docsTree: {} | ||
}; | ||
const releaseNotes: (string | undefined)[] = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid relying on file-level state for reusable code.
Storing specWeight
, finalResult
, and releaseNotes
at the top-level means multiple subsequent calls to buildPostList
can accumulate or override data. Consider initializing these within the function scope to avoid shared mutable state.
- let specWeight = 100;
- const finalResult: Result = { ... };
- const releaseNotes: (string | undefined)[] = [];
+ function createContext() {
+ let specWeight = 100;
+ const finalResult: Result = { ... };
+ const releaseNotes: (string | undefined)[] = [];
+ return { specWeight, finalResult, releaseNotes };
+ }
📝 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.
let specWeight = 100; | |
const finalResult: Result = { | |
docs: [], | |
blog: [], | |
about: [], | |
docsTree: {} | |
}; | |
const releaseNotes: (string | undefined)[] = []; | |
function createContext() { | |
let specWeight = 100; | |
const finalResult: Result = { | |
docs: [], | |
blog: [], | |
about: [], | |
docsTree: {} | |
}; | |
const releaseNotes: (string | undefined)[] = []; | |
return { specWeight, finalResult, releaseNotes }; | |
} |
for (const post of posts) { | ||
const link = `${base}${post.slug}${tracking}`; | ||
const { title, excerpt, date } = post; | ||
const pubDate = new Date(date).toUTCString(); | ||
const description = clean(excerpt!); | ||
const guid = { '@isPermaLink': true, '': link }; | ||
const item: RSSItemType = { | ||
title, | ||
description, | ||
link, | ||
category: type, | ||
guid, | ||
pubDate | ||
} as RSSItemType; | ||
|
||
if (post.cover) { | ||
const fileExtension = post.cover.substring(post.cover.lastIndexOf('.')).toLowerCase(); | ||
/* istanbul ignore next */ | ||
const mimeType = mimeTypes[fileExtension] || 'image/jpeg'; | ||
|
||
item.enclosure = { | ||
'@url': base + post.cover, | ||
'@length': 15026, // dummy value, anything works | ||
'@type': mimeType | ||
}; | ||
} | ||
rss.channel.item.push(item); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve enclosure handling
When setting the @length
for an enclosure, consider computing the actual file size if feasible. This can provide end-users with accurate metadata, especially if the feed readers rely on this value.
interface FrontMatter { | ||
title: string; | ||
date: string; | ||
type: string; | ||
tags: string[]; | ||
cover: string; | ||
weight?: number; | ||
authors: { name: string; link: string; photo: string }[]; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Extend FrontMatter interface for doc-specific fields.
Right now, both blogs and docs share the same interface. If there are doc-specific fields (e.g., weight
is optional for blogs?), consider distinguishing them or using more precise typed fields, such as optional keys for doc-only or blog-only features.
@@ -3,28 +3,29 @@ | |||
"version": "0.1.0", | |||
"description": "AsyncAPI website", | |||
"private": true, | |||
"type": "module", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
ESM migration impact: Setting "type": "module"
Adding "type": "module"
to the package.json indicates migration to ECMAScript modules, which is a good step that aligns with modern JavaScript standards. This change will affect how imports work across the codebase.
To ensure this change doesn't break existing functionality, run the following script to check for any CommonJS-style requires that might need updating:
🏁 Script executed:
#!/bin/bash
# Check for potential CommonJS require statements that might need updating
rg "^\s*(?:const|let|var).*require\(" --type=js
Length of output: 7553
ESM Migration & Test File Compatibility
Adding "type": "module"
in package.json
is a positive step toward modernizing the codebase. However, our scan detected multiple CommonJS require
calls in test files (e.g., in tests/adopters/index.test.js
, tests/build-meetings.test.js
, and others). This could lead to runtime issues under ESM if not handled appropriately.
- Action Required:
- Verify whether these test files should be updated to use ESM
import
statements or be adapted (e.g., by usingcreateRequire
) to work correctly in an ESM context. - Ensure that the test runner configuration supports either mixed modules or has a plan for handling CommonJS within an ESM project.
- Verify whether these test files should be updated to use ESM
"Co-authored-by: JeelRajodiya <[email protected]>"
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3761 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 20 21 +1
Lines 732 667 -65
Branches 0 113 +113
==========================================
- Hits 732 667 -65 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (7)
scripts/dashboard/build-dashboard.ts (7)
146-146
: Consider adding null check for reviews.nodes.The non-null assertion operator (!) is used here without first verifying that
reviews.nodes
isn't null or undefined. This could lead to runtime errors if the GraphQL API response changes.- discussion.reviews.nodes!.reduce((acc, curr) => acc + curr.comments.totalCount, 0) + (discussion.reviews.nodes || []).reduce((acc, curr) => acc + curr.comments.totalCount, 0)
160-160
: Document the score calculation formula.The score calculation uses a specific formula with an exponent of 1.8, but there's no explanation of why this value was chosen or what it represents. Adding a comment would improve maintainability.
- score: finalInteractionsCount / (monthsSince(discussion.timelineItems.updatedAt) + 2) ** 1.8 + // Score decreases as discussion ages; exponent 1.8 provides balanced decay rate + score: finalInteractionsCount / (monthsSince(discussion.timelineItems.updatedAt) + 2) ** 1.8
90-93
: Consider more specific error handling.The current error handling logs the error and rejects the promise with the same error. It might be helpful to provide more context about what operation failed, especially since this function is used in multiple places.
- logger.error(e); + logger.error(`Failed to fetch discussions with query params: pageSize=${pageSize}, endCursor=${endCursor}`, e);
128-129
: Avoid using type assertion with __typename.The code relies on the
__typename
field and disables eslint rules to access it. Consider using a type guard function for cleaner code.- // eslint-disable-next-line no-underscore-dangle - const isPR = discussion.__typename === 'PullRequest'; + const isPR = isPullRequest(discussion); // Add this helper function at the top level of the file: // function isPullRequest(node: HotDiscussionsIssuesNode): node is HotDiscussionsPullRequestsNode { // return node.__typename === 'PullRequest'; // }
134-135
: Consider alternative to reassigning parameters.The code reassigns the
discussion
parameter, which requires disabling an ESLint rule. Consider using a different variable name instead.- // eslint-disable-next-line no-param-reassign - discussion = fetchedDiscussion.node; + const updatedDiscussion = fetchedDiscussion.node; + // Use updatedDiscussion for the remainder of the function
179-186
: Consider using Promise.all for parallel batch processing.Processing batches one at a time with
await
in a loop (requiring eslint-disable) could be slower. You might consider a more parallel approach for improved performance, while still respecting rate limits.- for (let i = 0; i < discussions.length; i += batchSize) { - const batch = discussions.slice(i, i + batchSize); - // eslint-disable-next-line no-await-in-loop - const batchResults = await processHotDiscussions(batch); - - // eslint-disable-next-line no-await-in-loop - await pause(1000); - result.push(...batchResults); - } + const batches = []; + for (let i = 0; i < discussions.length; i += batchSize) { + batches.push(discussions.slice(i, i + batchSize)); + } + + for (const batch of batches) { + // Still process sequentially to respect rate limits, but with cleaner code + const batchResults = await processHotDiscussions(batch); + await pause(1000); + result.push(...batchResults); + }
249-252
: Use consistent naming convention for variables.The variable naming is inconsistent -
issues
andrawGoodFirstIssues
are camelCase, whilePRs
is PascalCase. Consider using camelCase for all variables.- const PRs = (await getDiscussions(Queries.hotDiscussionsPullRequests, 20)) as HotDiscussionsPullRequestsNode[]; + const prs = (await getDiscussions(Queries.hotDiscussionsPullRequests, 20)) as HotDiscussionsPullRequestsNode[];
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/dashboard/build-dashboard.ts
(1 hunks)tests/dashboard/build-dashboard.test.js
(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/dashboard/build-dashboard.test.js
⏰ Context from checks skipped due to timeout of 180000ms (1)
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (5)
scripts/dashboard/build-dashboard.ts (5)
1-23
: LGTM! Good TypeScript import setup with proper module paths.The import organization is clean and well-structured, with proper separation between external dependencies and internal modules. The URL and path handling for ES modules is implemented correctly.
24-47
: LGTM! Well-documented helper functions with proper TypeScript type annotations.The helper functions are clearly documented with JSDoc comments that explain parameters and return types, which align well with the TypeScript type annotations.
247-263
: Well-structured orchestration function with proper error handling.The
start
function effectively orchestrates the entire process with appropriate error handling and logging.
265-268
: LGTM! Good entry point detection for both direct execution and module import.The code correctly detects when it's being run directly vs being imported as a module using the ES modules pattern.
70-78
: Good rate limit monitoring.The code monitors GitHub's rate limit and logs a warning when approaching the limit, which is a good practice for preventing API throttling issues.
authorization: `token ${process.env.GITHUB_TOKEN}` | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Check for missing GitHub token.
The code doesn't validate if the GITHUB_TOKEN environment variable is set before using it, which could lead to authentication errors.
+ if (!process.env.GITHUB_TOKEN) {
+ throw new Error('GITHUB_TOKEN environment variable is not set');
+ }
headers: {
authorization: `token ${process.env.GITHUB_TOKEN}`
}
📝 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.
authorization: `token ${process.env.GITHUB_TOKEN}` | |
} | |
if (!process.env.GITHUB_TOKEN) { | |
throw new Error('GITHUB_TOKEN environment variable is not set'); | |
} | |
headers: { | |
authorization: `token ${process.env.GITHUB_TOKEN}` | |
} |
async function mapGoodFirstIssues(issues: GoodFirstIssues[]): Promise<MappedIssue[]> { | ||
/* istanbul ignore next */ | ||
return issues.map((issue) => ({ | ||
id: issue.id, | ||
title: issue.title, | ||
isAssigned: !!issue.assignees.totalCount, | ||
resourcePath: issue.resourcePath, | ||
repo: `asyncapi/${issue.repository.name}`, | ||
author: issue.author.login, | ||
area: getLabel(issue, 'area/') || 'Unknown', | ||
labels: issue.labels.nodes.filter( | ||
(label) => !label.name.startsWith('area/') && !label.name.startsWith('good first issue') | ||
) | ||
})); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Add unit test coverage for mapGoodFirstIssues.
The function has an istanbul ignore next
comment, indicating no test coverage. Consider adding unit tests to ensure this critical mapping function works correctly.
🏁 Script executed:
#!/bin/bash
# Check if test file exists for dashboard functionality
fd "build-dashboard.test" --type file
Length of output: 77
Action Required: Add Explicit Unit Tests for mapGoodFirstIssues
While we found a dashboard test file at tests/dashboard/build-dashboard.test.js
, there’s no clear evidence that this critical mapping function is directly covered by tests. The istanbul ignore next
comment in the function indicates that it’s being excluded from code coverage, which could hide potential issues.
- Ensure explicit tests for
mapGoodFirstIssues
are added.- Verify that various input scenarios (e.g., issues with no assignees, invalid or missing labels) return the expected mapped values.
- Check that labels starting with
"area/"
and"good first issue"
are correctly filtered. - Consider removing or revising the
istanbul ignore next
comment once proper test coverage is established.
Docstrings generation was requested by @akshatnema. * #3761 (comment) The following files were modified: * `scripts/adopters/index.ts` * `scripts/build-docs.ts` * `scripts/build-meetings.ts` * `scripts/build-newsroom-videos.ts` * `scripts/build-pages.ts` * `scripts/build-post-list.ts` * `scripts/build-rss.ts` * `scripts/build-tools.ts` * `scripts/casestudies/index.ts` * `scripts/compose.ts` * `scripts/dashboard/build-dashboard.ts` * `scripts/finance/index.ts` * `scripts/index.ts` * `scripts/markdown/check-edit-links.ts` * `scripts/markdown/check-markdown.ts` * `scripts/tools/combine-tools.ts` * `scripts/tools/extract-tools-github.ts` * `scripts/tools/tools-object.ts` * `scripts/utils.ts` * `scripts/utils/readAndWriteJson.ts` * `utils/redirect.ts`
Note Generated docstrings for this pull request at #3770 |
Description
This PR targets to migrate all scripts to typescript.
Related issue(s)
Summary by CodeRabbit