refactor: migrate build script to typescript #3423
Conversation
This reverts commit df75c79.
|
Warning Rate limit exceeded@JeelRajodiya has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 46 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces modifications to various configuration and script files, including updates to ESLint settings, GitHub Actions workflows, and the addition of TypeScript configurations. Key changes include the introduction of a new Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Repo as Repository
participant CI as CI/CD Pipeline
participant Build as Build Process
Dev->>Repo: Commits changes
Repo->>CI: Triggers workflow
CI->>Build: Runs TypeScript compilation
Build->>Build: Validates scripts
Build->>Build: Converts to ES modules
Build-->>CI: Returns build status
CI-->>Dev: Provides feedback
Poem
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
|
| "import/no-extraneous-dependencies": "off", | ||
| "no-console": "off", | ||
| "require-jsdoc":"off" | ||
| } |
There was a problem hiding this comment.
Why you added these three rules for scripts folder?
There was a problem hiding this comment.
import/no-extraneous-dependencies: The Scripts are using some of the dependencies which are listed in DevDependencies in package.json file. Since the scripts are ran in the server, it does not matter where the dependencies are listed in package.json file.
This error was occuring in the scripts/compose.ts file
no-console: We have several console.log statements in the scripts, which are necessary for build scripts. So I have removed the eslint warning since it is evident for the build scripts to have console statements in them (view your old comment)
require-jsdoc: We are migrating the files to typescript, I have removed the jsdoc rule because it would be irrelevant to keep jsdoc in typescript files.
|
|
||
| import { writeJSON } from '../utils/readAndWriteJson'; | ||
|
|
||
| const currentFilePath = fileURLToPath(import.meta.url); |
There was a problem hiding this comment.
What is import.meta.url here?
There was a problem hiding this comment.
It is an alternative way of getting the current currentFilePath and currentDirPath. Apprently __dirname and and __filename are not in available in es modules.
Value of import.meta.url in /scripts/index.ts
Reference: Alternative for __dirname in Node.js when using ES6 modules
| async function pause(ms) { | ||
| import { graphql } from '@octokit/graphql'; | ||
| import assert from 'assert'; | ||
| import { writeFile } from 'fs-extra'; |
There was a problem hiding this comment.
Add the types of this library as devDependency in package.json
There was a problem hiding this comment.
I have searched online but couldn't find types for @octokit/graphql library. So I have declared necessary types manually in the /types directory.
Other libraries such as assert, fs-extra takes type definitions from @types/node which We have already installed.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
JeelRajodiya
requested review from
derberg,
magicmatatjahu,
anshgoyalevil,
Mayaleeeee and
asyncapi-bot-eve
as code owners
…ackage.json existence
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 25
🔭 Outside diff range comments (2)
scripts/dashboard/build-dashboard.ts (1)
Line range hint
96-137: Guard against nullish “discussion.reactions” or “discussion.comments”
Although the code uses non-null assertions, there could be edge cases where GitHub returns unexpected structures (for example, missing “comments” or “reactions”). It may be safer to use optional chaining or explicit checks before reducing or accessing nested properties.Below is a sample diff to handle these edge cases more gracefully:
- const interactionsCount = - discussion.reactions.totalCount + - discussion.comments.totalCount + - discussion.comments.nodes.reduce((acc, curr) => acc + curr.reactions.totalCount, 0); + const totalReactions = discussion.reactions?.totalCount ?? 0; + const totalComments = discussion.comments?.totalCount ?? 0; + const nestedReactions = discussion.comments?.nodes?.reduce((acc, curr) => acc + (curr.reactions?.totalCount ?? 0), 0) ?? 0; + const interactionsCount = totalReactions + totalComments + nestedReactions;scripts/index.ts (1)
Line range hint
14-47: Add return type and improve error handling for async operationsThe start function needs better error handling and type annotations.
-async function start() { +async function start(): Promise<void> { + try { await buildPostList(); rssFeed('blog', 'AsyncAPI Initiative Blog RSS Feed', 'AsyncAPI Initiative Blog', 'rss.xml'); await buildCaseStudiesList('config/casestudies', resolve(currentDirPath, '../config', 'case-studies.json')); await buildAdoptersList(); // ... rest of the function ... + } catch (error: unknown) { + const errorMessage = error instanceof Error ? error.message : String(error); + console.error(`Build process failed: ${errorMessage}`); + process.exit(1); + } }
🧹 Nitpick comments (46)
scripts/build-rss.ts (4)
7-8: Avoid direct dynamic import of large data if alternatives exist.
If the JSON file is large, consider an approach that can lazily load or partially load data to improve performance and memory usage.
26-28: Avoid using "as any[]" for typed collections.
Explicitly define the type of the posts array (e.g., an interface) to ensure type safety.
46-48: Consider making base URL configurable.
It might be preferable to load the domain from a config file or environment variable for different deployment environments.
93-105: Use the actual image size for enclosure length.
The code currently uses a dummy value for@length. If accuracy is important, consider retrieving the actual content length of the image.scripts/tools/tools-object.ts (3)
7-12: Consider caching repeated fetches.
Repeatedly fetching large files over the network can degrade performance. If feasible, cache results for multiple usages in a single run.
33-54: Return early if required fields are missing.
IncreateToolObject, you rely on optional chaining for fields likedescriptionorlinks.repoUrl. If these are truly required, consider more direct validation or fallback defaults for better clarity.
75-90: Enforce stricter concurrency or parallelism boundaries.
Promises are run in parallel, which is good for performance, but if there's a large number of tool files, it may stress resources. Use concurrency controls if needed.scripts/markdown/check-markdown.ts (4)
31-74: Consider supporting custom link checking or advanced metadata validation.
While validating format is good, you may want to scan for broken links or missing alt text for images to further enhance content quality.
76-77: Add optional attribute checks for some frontmatter fields.
Though the blog postauthorsfield is checked, consider a deep check to ensure consistent data structure (like an author object must always have non-empty strings for keys).
113-116: Explicitly log a skip message for “docs/reference/specification”.
When skipping a directory, consider adding a log statement so users or maintainers can quickly see what was skipped and why.
146-155: Wrap bulk operations with additional error details.
When multiple markdown files fail, it may be helpful to summarize the total errors at the end for a single glance rather than scanning many lines in logs.scripts/tools/combine-tools.ts (1)
96-114: Evaluate memory usage with repeated updates to Fuse instances.
Re-instantiating Fuse after adding new items is convenient but can be expensive if large. Consider a single pass to add items, then one final initialization, if scaling is expected.scripts/dashboard/build-dashboard.ts (2)
Line range hint
39-78: Clarify the return type and pagination logic
The current logic assumes a result shape that includes “search.nodes” and “search.pageInfo”. Consider creating a dedicated type to annotate the GraphQL response, improving clarity around the structure, especially since the function recurses for pagination. This will make it easier for contributors to understand and maintain the code.
217-227: Exported entities confirmation
This file exports many functions. Ensure that only the functions needed by external modules are part of the public API, to keep the code surface minimal and maintainable.scripts/build-post-list.ts (2)
37-52: Consider simplifying slug creation
The function slugifyToC uses regex matching for inline anchor tags and fallback slug generation. You could unify both conditions in a single regex or apply a more streamlined approach for better readability.
178-188: Handle potential I/O errors
The script writes the final JSON to disk in production mode. It’s worth adding a confirmation log or a try/catch to handle unexpected file system errors, ensuring any failure is reported properly and does not silently halt the build process.types/packages/jgexml__json2xml.d.ts (1)
2-3: Provide more usage context or doc comments
Although this declaration provides the essential contract for getXml, consider adding a short JSDoc comment describing “feed” and other parameters, guiding users on what data structure “feed” expects.scripts/adopters/index.ts (1)
9-11: Verify async operation success
The writeJSON function is called but not awaited. Although it might happen nearly instantaneously, consider awaiting it or handling potential errors to ensure the file write finishes before proceeding.next-i18next.config.cjs (1)
11-11: Improve documentation for useSuspense settingThe comment "this line" doesn't explain why
useSuspenseis disabled.- react: { useSuspense: false } // this line + react: { useSuspense: false } // Disable React suspense to prevent hydration issues with SSRscripts/tools/extract-tools-github.ts (1)
9-14: Handle GitHub API rate limits and paginationThe GitHub Search API has rate limits and returns paginated results. Add proper handling:
+const GITHUB_API_URL = 'https://api.github.com/search/code'; +const SEARCH_QUERY = 'filename:.asyncapi-tool'; + +async function checkRateLimit() { + const response = await axios.get('https://api.github.com/rate_limit', { + headers: { + authorization: `token ${process.env.GITHUB_TOKEN}` + } + }); + const { remaining, reset } = response.data.rate.search; + if (remaining === 0) { + const resetDate = new Date(reset * 1000); + throw new Error(`GitHub API rate limit exceeded. Resets at ${resetDate.toISOString()}`); + } +} + const getData = async (): Promise<GitHubSearchResponse> => { + await checkRateLimit(); + const result = await axios.get( - 'https://api.github.com/search/code?q=filename:.asyncapi-tool', + `${GITHUB_API_URL}?q=${SEARCH_QUERY}&per_page=100`, { headers: { accept: 'application/vnd.github.text-match+json',tsconfig.json (1)
21-24: Consider performance implications of transpileOnlyThe ts-node configuration with
transpileOnly: truewill improve development speed but skips type checking. Consider adding a pre-commit hook or CI check to ensure type checking is performed.types/scripts/build-posts-list.ts (1)
26-26: Consider replacing index signature with specific propertiesUsing
[key: string]: anyreduces type safety. If possible, explicitly define all expected properties.scripts/utils/readAndWriteJson.ts (1)
5-29: Consider enhancing error handling and type safetyWhile the error handling structure is good, consider these improvements:
- Use more specific error types
- Add input validation
- Type the JSON content
Here's a suggested implementation:
-export async function writeJSON(readPath: string, writePath: string) { +export async function writeJSON(readPath: string, writePath: string): Promise<void> { + if (!readPath || !writePath) { + throw new Error('Both readPath and writePath must be provided'); + } + let readContent: string; - let jsonContent; + let jsonContent: Record<string, unknown>; // 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(`Failed to read file ${readPath}: ${err instanceof Error ? err.message : String(err)}`); } // Attempt to convert content to JSON try { jsonContent = convertToJson(readContent); } catch (err) { - throw new Error(`Error while conversion\nError: ${err}`); + throw new Error(`Failed to parse JSON from ${readPath}: ${err instanceof Error ? err.message : String(err)}`); } // Attempt to write the JSON content to file try { - await writeFile(writePath, JSON.stringify(jsonContent)); + await writeFile(writePath, JSON.stringify(jsonContent, null, 2)); } catch (err) { - throw new Error(`Error while writing file\nError: ${err}`); + throw new Error(`Failed to write to ${writePath}: ${err instanceof Error ? err.message : String(err)}`); } }scripts/casestudies/index.ts (1)
5-26: Enhance type safety and error handlingConsider adding return type annotation and more specific error handling.
Here's a suggested implementation:
-export async function buildCaseStudiesList(dirWithCaseStudy: string, writeFilePath: string) { +type CaseStudy = Record<string, unknown>; +export async function buildCaseStudiesList( + dirWithCaseStudy: string, + writeFilePath: string +): Promise<CaseStudy[]> { try { const files = await readdir(dirWithCaseStudy); // Process all files in parallel using Promise.all const caseStudiesList = await Promise.all( files.map(async (file) => { const caseStudyFileName = join(dirWithCaseStudy, file); const caseStudyContent = await readFile(caseStudyFileName, 'utf-8'); return convertToJson(caseStudyContent) as CaseStudy; }) ); // Write the complete list once after all files are processed await writeFile(writeFilePath, JSON.stringify(caseStudiesList, null, 2)); return caseStudiesList; } catch (err) { - throw new Error(err instanceof Error ? err.message : String(err)); + throw new Error( + `Failed to build case studies list: ${err instanceof Error ? err.message : String(err)}` + ); } }scripts/utils.ts (1)
15-25: Enhance error handling with typed errorsThe error handling could be improved by using typed errors and providing more context.
- } catch (jsonError) { + } catch (jsonError: unknown) { try { const yamlContent = yaml.parse(contentYAMLorJSON); return yamlContent; - } catch (yamlError) { + } catch (yamlError: unknown) { + const jsonErr = jsonError instanceof Error ? jsonError.message : String(jsonError); + const yamlErr = yamlError instanceof Error ? yamlError.message : String(yamlError); throw new Error( - `Invalid content format:\nJSON Parse Error: ${jsonError}\nYAML Parse Error: ${yamlError}` + `Invalid content format:\nJSON Parse Error: ${jsonErr}\nYAML Parse Error: ${yamlErr}` ); } }scripts/finance/index.ts (2)
23-34: Add file existence checks before operationsAdd checks to verify if the source YAML files exist before attempting to process them.
+import { access } from 'fs/promises'; + try { const expensesPath = resolve(currentDir, configDir, financeDir, year, 'Expenses.yml'); const expensesLinkPath = resolve(currentDir, configDir, financeDir, year, 'ExpensesLink.yml'); + // Check if source files exist + try { + await access(expensesPath); + await access(expensesLinkPath); + } catch { + throw new Error(`Required files not found: ${expensesPath} or ${expensesLinkPath}`); + } // Ensure the directory exists before writing the files
39-42: Improve error handling with specific error typesThe error handling could be more specific and provide better context.
- } catch (err) { - assert(err instanceof Error); - throw new Error(err.message); + } catch (err: unknown) { + const errorMessage = err instanceof Error ? err.message : String(err); + throw new Error(`Failed to build finance info list: ${errorMessage}`); }scripts/index.ts (2)
11-12: Consider using a configuration file for pathsThe current directory resolution could be moved to a configuration file for better maintainability.
Consider creating a
config.tsfile to store all path configurations.
Line range hint
48-51: Consider removing immediate invocationThe immediate invocation of
start()at the module level could cause issues with testing and module imports.Consider moving the
start()invocation to a separate file or adding a condition:export { start }; -start(); +if (require.main === module) { + start().catch((error) => { + console.error('Failed to start build process:', error); + process.exit(1); + }); +}scripts/build-tools.ts (1)
13-26: Consider adding input validation and return type annotationThe
buildToolsfunction should validate input parameters and specify its return type for better type safety.-const buildTools = async (automatedToolsPath: string, manualToolsPath: string, toolsPath: string, tagsPath: string) => { +const buildTools = async (automatedToolsPath: string, manualToolsPath: string, toolsPath: string, tagsPath: string): Promise<void> => { + // Validate input paths + if (!automatedToolsPath || !manualToolsPath || !toolsPath || !tagsPath) { + throw new Error('All path parameters are required'); + } try {scripts/build-pages.ts (2)
1-2: Consider using fs-extra for better error handlingReplace
fswithfs-extrafor Promise-based APIs and better error handling.-import fs from 'fs'; +import fs from 'fs-extra';
24-24: Add return type annotation and error handlingThe function should specify its return type and include proper error handling.
-export function copyAndRenameFiles(srcDir: string, targetDir: string) { +export function copyAndRenameFiles(srcDir: string, targetDir: string): void { + if (!fs.existsSync(srcDir)) { + throw new Error(`Source directory ${srcDir} does not exist`); + }types/githubGraphQL.ts (1)
91-95: Add JSDoc documentation and fix EOF newlineAdd documentation for the generic type parameter and fix the missing newline at EOF.
+/** + * Represents a GraphQL query response + * @template T The type of the node in the response + */ export interface QueryResponse<T> { node?: T; rateLimit: RateLimit; search?: SearchResult; -} \ No newline at end of file +} +🧰 Tools
🪛 eslint
[error] 95-95: Newline required at end of file but not found.
(eol-last)
[error] 95-95: Insert
⏎(prettier/prettier)
types/scripts/tools.ts (2)
39-44: Consider making categories property more type-safeThe
categoriesarray allows bothCategoryandstringtypes, which could lead to runtime errors if arbitrary strings are used. Consider restricting it to just theCategorytype.interface Filters { language?: Array<LanguageColorItem | string>; technology?: Array<LanguageColorItem | string>; - categories: Array<Category | string>; + categories: Array<Category>; hasCommercial?: boolean; }
54-69: Consider using Pick/Omit utility types for repository structureThe nested structure in
ToolsDatacould benefit from TypeScript's utility types to make it more maintainable.+ type Repository = { + full_name: string; + html_url: string; + owner: { + login: string; + }; + description: string; + }; export type ToolsData = { items: { name: string; url: string; path: string; html_url: string; - repository: { - full_name: string; - html_url: string; - owner: { - login: string; - }; - description: string; - }; + repository: Repository; }[]; };scripts/build-newsroom-videos.ts (1)
19-26: Consider extracting API parameters to configurationThe YouTube API parameters are hardcoded. Consider moving them to a configuration file for better maintainability.
// config/youtube.ts export const YOUTUBE_CONFIG = { channelId: 'UCIz9zGwDLbrYQcDKVXdOstQ', maxResults: 5, // ... other parameters } as const;scripts/build-meetings.ts (2)
27-28: Extract date calculations to named constantsThe date calculations are hard to understand at first glance. Consider extracting them to named constants.
- const timeMin = new Date(Date.parse(currentTime) - 100 * 24 * 60 * 60 * 1000).toISOString(); - const timeMax = new Date(Date.parse(currentTime) + 30 * 24 * 60 * 60 * 1000).toISOString(); + const DAYS_TO_LOOK_BACK = 100; + const DAYS_TO_LOOK_AHEAD = 30; + const MS_PER_DAY = 24 * 60 * 60 * 1000; + const timeMin = new Date(Date.parse(currentTime) - DAYS_TO_LOOK_BACK * MS_PER_DAY).toISOString(); + const timeMax = new Date(Date.parse(currentTime) + DAYS_TO_LOOK_AHEAD * MS_PER_DAY).toISOString();
48-53: Use optional chaining as suggested by static analysisThe static analysis tool correctly identifies a potential improvement using optional chaining.
- e.extendedProperties?.private && - `https://github.com/asyncapi/community/issues/${e.extendedProperties.private.ISSUE_ID}`, - banner: e.extendedProperties?.private && e.extendedProperties.private.BANNER, + url: e.extendedProperties?.private?.ISSUE_ID && + `https://github.com/asyncapi/community/issues/${e.extendedProperties.private.ISSUE_ID}`, + banner: e.extendedProperties?.private?.BANNER,🧰 Tools
🪛 Biome (1.9.4)
[error] 52-52: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
types/scripts/dashboard.ts (1)
22-28: Consider extracting common node propertiesThe
PullRequestById.nodeandIssueById.nodeshare many properties with other interfaces. Consider creating a base interface for common node properties.+interface BaseNode { + __typename: string; + assignees: Assignees; + timelineItems: TimelineItems; + author: Author; + id: string; + title: string; + resourcePath: string; + repository: Repository; + reactions: Reactions; + comments: Comments; + labels: { + nodes: Label[]; + }; +} -export interface PullRequestById { - node: { - // ... current properties - }; -} +export interface PullRequestById { + node: BaseNode & { + reviews: Reviews; + }; +} -export interface IssueById { - node: { - // ... current properties - }; +export interface IssueById { + node: BaseNode; +}Also applies to: 56-73, 75-92
scripts/tools/tags-color.ts (1)
149-177: Consider unique color schemes for technologiesMultiple technologies (Kubernetes-native, Jenkins, Flask) share the same color scheme (#D7C7F2/#A387D2). Consider assigning unique colors to each technology for better visual distinction.
{ name: 'Jenkins', - color: 'bg-[#D7C7F2]', - borderColor: 'border-[#A387D2]' + color: 'bg-[#335061]', + borderColor: 'border-[#224759]' }, { name: 'Flask', - color: 'bg-[#D7C7F2]', - borderColor: 'border-[#A387D2]' + color: 'bg-[#000000]', + borderColor: 'border-[#424242]' },scripts/dashboard/issue-queries.ts (1)
Line range hint
1-277: Consider adding TypeScript types for GraphQL queriesSince we're migrating to TypeScript, consider adding type definitions for the GraphQL queries' responses to improve type safety and developer experience.
type PullRequestResponse = { node: { __typename: string; assignees: { totalCount: number; }; // ... other fields }; rateLimit: { limit: number; cost: number; remaining: number; resetAt: string; }; }; export const Queries = Object.freeze({ pullRequestById: `...` as const, // ... other queries });scripts/compose.ts (2)
144-150: Consider using async/await for file operationsThe callback-style file operations could be modernized using async/await for better error handling and readability.
const writePost = async (filePath: string, frontMatter: string) => { try { await fs.promises.writeFile(filePath, frontMatter, { flag: 'wx' }); console.log(`Blog post generated successfully at ${filePath}`); } catch (err) { if (err.code === 'EEXIST') { throw new Error(`File ${filePath} already exists`); } throw err; } };
153-159: Enhance error messages with more contextThe error messages could be more descriptive to help users understand and resolve issues.
.catch((error) => { console.error('Failed to generate blog post:', error); if (error.isTtyError) { console.log("The prompt couldn't be rendered in your current terminal environment"); } else { console.log('An unexpected error occurred while generating the blog post. Please check your inputs and try again.'); } });scripts/build-docs.ts (1)
138-138: Use array destructuring for clarityReplace array filtering with destructuring for better readability.
-structuredPosts[0] = docPosts.filter((p) => p.slug === '/docs')[0]; +const [welcomePost] = docPosts.filter((p) => p.slug === '/docs'); +structuredPosts[0] = welcomePost;🧰 Tools
🪛 eslint
[error] 138-138: Use array destructuring.
(prefer-destructuring)
scripts/tools/tools-schema.json (1)
13-16: Fix grammatical errors in descriptionThe description property contains grammatical errors that should be corrected for clarity.
- "description": "By default scripts read description of repository there project is stored. You can override this behaviour by providing custom description." + "description": "By default, scripts read the description from the repository where the project is stored. You can override this behavior by providing a custom description."utils/getStatic.ts (1)
31-37: Consider improving TypeScript typesThe
anytype forctxparameter could be replaced with a more specific type to improve type safety during the TypeScript migration.Consider defining an interface for the context parameter:
interface I18nContext { params?: { lang?: string; }; }Then update the function signature:
- export async function getI18nProps(ctx: any, ns = ['common']) { + export async function getI18nProps(ctx: I18nContext, ns = ['common']) {🧰 Tools
🪛 eslint
[error] 34-34: Delete
,(prettier/prettier)
[error] 34-34: Unexpected trailing comma.
(comma-dangle)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (55)
.eslintrc(1 hunks).github/workflows/if-nodejs-pr-testing.yml(1 hunks)next-i18next.config.cjs(1 hunks)next-i18next.config.js(0 hunks)package.json(3 hunks)pages/_document.tsx(1 hunks)rename.ps1(1 hunks)scripts/adopters/index.js(0 hunks)scripts/adopters/index.ts(1 hunks)scripts/build-docs.ts(7 hunks)scripts/build-meetings.ts(2 hunks)scripts/build-newsroom-videos.js(0 hunks)scripts/build-newsroom-videos.ts(1 hunks)scripts/build-pages.ts(2 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.ts(9 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-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/readAndWriteJson.js(0 hunks)scripts/utils/readAndWriteJson.ts(1 hunks)tsconfig.json(1 hunks)types/githubGraphQL.ts(1 hunks)types/packages/jgexml__json2xml.d.ts(1 hunks)types/packages/markdown-toc.d.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(5 hunks)utils/languageDetector.ts(1 hunks)
💤 Files with no reviewable changes (16)
- scripts/tools/extract-tools-github.js
- scripts/tools/categorylist.js
- next-i18next.config.js
- scripts/utils.js
- scripts/utils/readAndWriteJson.js
- scripts/adopters/index.js
- scripts/finance/index.js
- scripts/tools/tools-object.js
- scripts/casestudies/index.js
- scripts/build-tools.js
- scripts/build-rss.js
- scripts/tools/tags-color.js
- scripts/tools/combine-tools.js
- scripts/build-post-list.js
- scripts/build-newsroom-videos.js
- scripts/markdown/check-markdown.js
✅ Files skipped from review due to trivial changes (1)
- pages/_document.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
scripts/tools/extract-tools-github.ts
[error] 18-18: 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)
scripts/build-meetings.ts
[error] 52-52: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
types/packages/markdown-toc.d.ts
[error] 29-29: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
🪛 eslint
scripts/build-docs.ts
[error] 138-138: Use array destructuring.
(prefer-destructuring)
[error] 146-146: Expected '===' and instead saw '=='.
(eqeqeq)
[error] 147-147: Expected '===' and instead saw '=='.
(eqeqeq)
types/githubGraphQL.ts
[error] 76-76: Replace "Issue"·|·"PullRequest" with 'Issue'·|·'PullRequest'
(prettier/prettier)
[error] 76-76: Strings must use singlequote.
(quotes)
[error] 76-76: Strings must use singlequote.
(quotes)
[error] 95-95: Newline required at end of file but not found.
(eol-last)
[error] 95-95: Insert ⏎
(prettier/prettier)
utils/getStatic.ts
[error] 12-12: Delete ,
(prettier/prettier)
[error] 12-12: Unexpected trailing comma.
(comma-dangle)
[error] 13-13: Delete ,
(prettier/prettier)
[error] 13-13: Unexpected trailing comma.
(comma-dangle)
[error] 22-22: Delete ,
(prettier/prettier)
[error] 22-22: Unexpected trailing comma.
(comma-dangle)
[error] 34-34: Delete ,
(prettier/prettier)
[error] 34-34: Unexpected trailing comma.
(comma-dangle)
[error] 48-48: Delete ,
(prettier/prettier)
[error] 48-48: Unexpected trailing comma.
(comma-dangle)
utils/languageDetector.ts
[error] 7-7: Delete ,
(prettier/prettier)
[error] 7-7: Unexpected trailing comma.
(comma-dangle)
🔇 Additional comments (22)
scripts/build-rss.ts (2)
75-89: Double-check date conversions and time zones.
Ensure no discrepancies in date/time formatting or time zone issues if your environment is sensitive to time zone drift. Using library-based date formatting may be safer.
36-40: Validate the featured property existence before usage.
Currently, the code sorts based on the featured field. If featured is undefined for any post, it may lead to unintended sorting behaviors. Ensure fallback or default values.
scripts/tools/tools-object.ts (1)
60-74: Ensure error-logging doesn't mask critical issues.
The code gracefully logs problems when encountering invalid tool files, but consider whether continuing execution can lead to partial or incorrect final data.
scripts/tools/combine-tools.ts (2)
29-35: Consider a consistent ID or index approach for mapping.
Fuse is used repeatedly for searching color tags. Suppose multiple new tags appear; ensure all are assigned correct, unique identifiers.
139-158: Validate or safely fallback for tool.title.
Sorting by tool.title might lead to runtime errors if tool.title is missing. Ensure a fallback or a validation step prior to sorting.
scripts/dashboard/build-dashboard.ts (1)
33-38: Check for potential undefined “filter” usage or missing “/”
The function splits the label name on the “/” character. Consider adding a safety check or fallback default for cases where no “/” is present in the label string, to avoid possible runtime errors.
Run the following script to search for all usage patterns of this function to see if there’s ever a case where “filter” may be empty or not contain “/”:
scripts/build-post-list.ts (1)
61-68: Validate existing slugs before pushing
If there is any possibility of a repeated or malformed slug, it could cause collisions in finalResult arrays. Consider adding a uniqueness check to ensure no duplicates.
tsconfig.json (1)
19-20: Verify type definitions inclusion
The removal of "types/**/*.d.ts" from the include array might affect custom type definitions. Ensure all type definition files are still properly included through the existing patterns.
✅ Verification successful
The previous script showed that .d.ts files exist in the types directory, but we need to verify their content and usage to make a proper assessment. The new script will help us understand if these type definitions are actually being used in the codebase and their importance.
Type definitions are properly included in the configuration
The **/*.ts pattern in the include array already covers all TypeScript files, including .d.ts files, in any subdirectory. This is confirmed by:
- All
.d.tsfiles are located under thetypesdirectory, which is covered by the**/*.tspattern - The type definitions are actively used in the codebase (e.g., SponsorType is used in multiple sponsor-related components)
The removal of the explicit "types/**/*.d.ts" pattern is redundant and doesn't affect type coverage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if any .d.ts files exist outside the included patterns
fd -e d.ts -E "node_modules" -E "netlify" | while read -r file; do
if ! [[ "$file" =~ ^(next-env\.d\.ts|.*\.ts|.*\.tsx|.*\.json)$ ]]; then
echo "Warning: Type definition file may be excluded: $file"
fi
done
Length of output: 461
Script:
#!/bin/bash
# Let's check the actual content of the types directory to understand the structure
fd . "types" -t f -x echo "=== {} ===" \; cat "{}"
# Also check if these type definitions are actually used in the codebase
rg -l "SponsorType|jgexml__json2xml|markdown-toc" --type ts
Length of output: 1600
types/packages/markdown-toc.d.ts (1)
4-9: LGTM! Well-structured interface definition
The TocItem interface clearly defines all necessary properties with appropriate types.
types/scripts/build-rss.ts (1)
2-7: Review recursive type definition in Enclosure
The Enclosure type has a recursive definition that could lead to infinite nesting. Consider if this is intentional or if it should be limited.
Consider using a non-recursive structure or limiting the depth:
export type Enclosure = {
'@url': string;
'@length': number;
'@type': string;
- enclosure: Enclosure;
+ enclosure?: Enclosure; // Make it optional
};scripts/casestudies/index.ts (1)
10-17: Good use of Promise.all for parallel processing
Excellent implementation of concurrent file processing using Promise.all. This effectively addresses the ESLint no-await-in-loop rule while improving performance.
scripts/dashboard/issue-queries.ts (1)
1-1: LGTM! Clean migration to ES module syntax.
The transition from CommonJS exports.Queries to ES modules export const Queries is correct and aligns with the TypeScript migration objective.
scripts/compose.ts (1)
10-16: LGTM! Well-structured type definition.
The ComposePromptType interface clearly defines the expected structure of the prompt answers.
.github/workflows/if-nodejs-pr-testing.yml (1)
Line range hint 85-108: LGTM! Well-structured markdown validation workflow
The markdown validation implementation with error capture and PR commenting is well-designed and provides good feedback to contributors.
package.json (2)
11-27: LGTM: Consistent migration to tsx for script execution
The script commands have been properly updated to use tsx instead of node, which aligns with the TypeScript migration strategy discussed in the PR comments.
118-118: LGTM: Appropriate TypeScript dependencies added
The addition of tsx and type definitions for fs-extra and inquirer properly supports the TypeScript migration.
Also applies to: 134-135
scripts/build-docs.ts (1)
93-116: LGTM: Well-structured recursive function
The convertDocPosts function is well-documented and properly handles the recursive traversal of the document tree.
scripts/tools/tools-schema.json (2)
43-110: LGTM: Comprehensive language support
The schema properly handles both single string and array values for languages, with well-documented explanations and examples.
145-200: LGTM: Well-structured categories definition
The categories property is well-defined with clear documentation, validation rules, and comprehensive examples.
.eslintrc (1)
317-322: Review the necessity of disabling JSDoc requirements
While disabling import/no-extraneous-dependencies and no-console is justified for build scripts (CLI tools), consider keeping require-jsdoc enabled to maintain documentation quality during the TypeScript migration.
Let's check the current JSDoc coverage in scripts:
Consider keeping JSDoc requirements for better maintainability, especially during the TypeScript migration phase where type documentation becomes more crucial.
utils/languageDetector.ts (1)
3-3: LGTM: CommonJS extension properly specified
The .cjs extension correctly identifies this as a CommonJS module, which is appropriate for configuration files in a TypeScript project.
utils/getStatic.ts (1)
3-3: LGTM: Consistent use of CommonJS extension
The .cjs extension usage is consistent with other files in the codebase.
| function clean(s: string) { | ||
| let cleanS = s; | ||
|
|
||
| cleanS = cleanS.split('<span>').join(''); | ||
| cleanS = cleanS.split('&').join('&'); | ||
| cleanS = cleanS.split(''').join("'"); | ||
| cleanS = cleanS.split('<').join('<'); | ||
| cleanS = cleanS.split('>').join('>'); | ||
| cleanS = cleanS.split('"').join('"'); | ||
|
|
||
| return cleanS; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider sanitizing more aggressively or using a template engine.
The current manual string replacements could miss an unhandled HTML entity. A robust HTML sanitization library may help avoid edge cases and XSS-like vulnerabilities.
| const getFinalTool = async (toolObject: AsyncAPITool) => { | ||
| const finalObject = toolObject; | ||
|
|
||
| // there might be a tool without language | ||
| if (toolObject.filters.language) { |
There was a problem hiding this comment.
Make sure optional filters are properly handled.
The code only checks toolObject.filters.language and toolObject.filters.technology. If filters is undefined, this can break.
-if (toolObject.filters.language) {
+if (toolObject.filters?.language) {📝 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.
| const getFinalTool = async (toolObject: AsyncAPITool) => { | |
| const finalObject = toolObject; | |
| // there might be a tool without language | |
| if (toolObject.filters.language) { | |
| const getFinalTool = async (toolObject: AsyncAPITool) => { | |
| const finalObject = toolObject; | |
| // there might be a tool without language | |
| if (toolObject.filters?.language) { |
| async function getHotDiscussions(discussions: HotDiscussionsIssuesNode[]) { | ||
| const result: ProcessedDiscussion[] = []; | ||
| const batchSize = 5; | ||
|
|
||
| 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); | ||
| } | ||
|
|
||
| result.sort((ElemA, ElemB) => ElemB.score - ElemA.score); | ||
| const filteredResult = result.filter((issue) => issue.author !== 'asyncapi-bot'); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider concurrency approach for getHotDiscussions
Processing discussions in batches (line 145) is good, but consider adjusting the pause interval or using Promise.allSettled for improved throughput. If the GitHub rate limit is generous enough, you could parallelize some portion of these calls.
rename.ps1
Outdated
| # Define the root folder | ||
| $rootFolder = "scripts" | ||
|
|
||
| # Get all .js files recursively | ||
| $jsFiles = Get-ChildItem -Path $rootFolder -Recurse -Filter "*.js" | ||
|
|
||
| # Loop through each .js file and run the git mv command | ||
| foreach ($file in $jsFiles) { | ||
| # Construct the new file name with .ts extension | ||
| $newFileName = $file.FullName -replace "\.js$", ".ts" | ||
|
|
||
| # Execute the git mv command | ||
| git mv $file.FullName $newFileName | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and validation to the rename script
While the script's basic functionality works, it needs additional safeguards for production use:
- Validate file existence before renaming
- Handle git mv failures
- Verify genuine JS files (not just .js extension)
- Add a dry-run option
Here's a more robust implementation:
# Define the root folder
$rootFolder = "scripts"
+
+# Add dry-run parameter
+param(
+ [switch]$DryRun = $false
+)
+
+# Verify the root folder exists
+if (-not (Test-Path $rootFolder)) {
+ Write-Error "Root folder '$rootFolder' does not exist"
+ exit 1
+}
# Get all .js files recursively
$jsFiles = Get-ChildItem -Path $rootFolder -Recurse -Filter "*.js"
+
+# Verify we found files to process
+if ($jsFiles.Count -eq 0) {
+ Write-Warning "No .js files found in $rootFolder"
+ exit 0
+}
# Loop through each .js file and run the git mv command
foreach ($file in $jsFiles) {
+ # Verify it's actually a JavaScript file (not just .js extension)
+ $content = Get-Content $file.FullName -First 1
+ if (-not ($content -match '^(//|/\*|import\s|const\s|let\s|var\s|function\s|class\s)')) {
+ Write-Warning "Skipping $($file.FullName) - doesn't appear to be a JavaScript file"
+ continue
+ }
+
# Construct the new file name with .ts extension
$newFileName = $file.FullName -replace "\.js$", ".ts"
+ if ($DryRun) {
+ Write-Host "Would rename $($file.FullName) to $newFileName"
+ continue
+ }
+
# Execute the git mv command
- git mv $file.FullName $newFileName
+ $result = git mv $file.FullName $newFileName 2>&1
+ if ($LASTEXITCODE -ne 0) {
+ Write-Error "Failed to rename $($file.FullName): $result"
+ continue
+ }
+ Write-Host "Successfully renamed $($file.FullName) to $newFileName"
}📝 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.
| # Define the root folder | |
| $rootFolder = "scripts" | |
| # Get all .js files recursively | |
| $jsFiles = Get-ChildItem -Path $rootFolder -Recurse -Filter "*.js" | |
| # Loop through each .js file and run the git mv command | |
| foreach ($file in $jsFiles) { | |
| # Construct the new file name with .ts extension | |
| $newFileName = $file.FullName -replace "\.js$", ".ts" | |
| # Execute the git mv command | |
| git mv $file.FullName $newFileName | |
| } | |
| # Define the root folder | |
| $rootFolder = "scripts" | |
| # Add dry-run parameter | |
| param( | |
| [switch]$DryRun = $false | |
| ) | |
| # Verify the root folder exists | |
| if (-not (Test-Path $rootFolder)) { | |
| Write-Error "Root folder '$rootFolder' does not exist" | |
| exit 1 | |
| } | |
| # Get all .js files recursively | |
| $jsFiles = Get-ChildItem -Path $rootFolder -Recurse -Filter "*.js" | |
| # Verify we found files to process | |
| if ($jsFiles.Count -eq 0) { | |
| Write-Warning "No .js files found in $rootFolder" | |
| exit 0 | |
| } | |
| # Loop through each .js file and run the git mv command | |
| foreach ($file in $jsFiles) { | |
| # Verify it's actually a JavaScript file (not just .js extension) | |
| $content = Get-Content $file.FullName -First 1 | |
| if (-not ($content -match '^(//|/\*|import\s|const\s|let\s|var\s|function\s|class\s)')) { | |
| Write-Warning "Skipping $($file.FullName) - doesn't appear to be a JavaScript file" | |
| continue | |
| } | |
| # Construct the new file name with .ts extension | |
| $newFileName = $file.FullName -replace "\.js$", ".ts" | |
| if ($DryRun) { | |
| Write-Host "Would rename $($file.FullName) to $newFileName" | |
| continue | |
| } | |
| # Execute the git mv command | |
| $result = git mv $file.FullName $newFileName 2>&1 | |
| if ($LASTEXITCODE -ne 0) { | |
| Write-Error "Failed to rename $($file.FullName): $result" | |
| continue | |
| } | |
| Write-Host "Successfully renamed $($file.FullName) to $newFileName" | |
| } |
| import axios from 'axios'; | ||
| import dotenv from 'dotenv'; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Verify environment variable presence
Add validation for the required GITHUB_TOKEN environment variable:
import axios from 'axios';
import dotenv from 'dotenv';
dotenv.config();
+
+if (!process.env.GITHUB_TOKEN) {
+ throw new Error('GITHUB_TOKEN environment variable is required');
+}📝 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.
| import axios from 'axios'; | |
| import dotenv from 'dotenv'; | |
| import axios from 'axios'; | |
| import dotenv from 'dotenv'; | |
| dotenv.config(); | |
| if (!process.env.GITHUB_TOKEN) { | |
| throw new Error('GITHUB_TOKEN environment variable is required'); | |
| } |
| name: 'Scala', | ||
| color: 'bg-[#D7C7F2]', | ||
| borderColor: 'border-[#A387D2]' | ||
| }, |
There was a problem hiding this comment.
Remove duplicate Scala entry
Scala is already defined in the languagesColor array (lines 51-54). It should not appear in the technologiesColor array.
- {
- name: 'Scala',
- color: 'bg-[#D7C7F2]',
- borderColor: 'border-[#A387D2]'
- },| - if: steps.packagejson.outputs.exists == 'true' | ||
| name: Install dependencies | ||
| run: npm install |
There was a problem hiding this comment.
Consider using npm ci instead of npm install
The change from npm ci to npm install might lead to inconsistent dependency installations across different environments. npm ci is preferred in CI environments as it:
- Uses package-lock.json exclusively
- Is more reliable for reproducible builds
- Removes node_modules before installation
- run: npm install
+ run: npm ci📝 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.
| - if: steps.packagejson.outputs.exists == 'true' | |
| name: Install dependencies | |
| run: npm install | |
| - if: steps.packagejson.outputs.exists == 'true' | |
| name: Install dependencies | |
| run: npm ci |
scripts/build-docs.ts
Outdated
| if (post?.isRootSection || post?.isSection || index == 0) { | ||
| if (post?.isRootSection || index == 0) | ||
| rootSections.push(post.title) | ||
| return post | ||
| if (post?.isRootSection || index == 0) rootSections.push(post.title); |
There was a problem hiding this comment.
Fix loose equality comparisons
Replace loose equality (==) with strict equality (===) for type-safe comparisons.
-if (post?.isRootSection || post?.isSection || index == 0) {
- if (post?.isRootSection || index == 0) rootSections.push(post.title);
+if (post?.isRootSection || post?.isSection || index === 0) {
+ if (post?.isRootSection || index === 0) rootSections.push(post.title);📝 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.
| if (post?.isRootSection || post?.isSection || index == 0) { | |
| if (post?.isRootSection || index == 0) | |
| rootSections.push(post.title) | |
| return post | |
| if (post?.isRootSection || index == 0) rootSections.push(post.title); | |
| if (post?.isRootSection || post?.isSection || index === 0) { | |
| if (post?.isRootSection || index === 0) rootSections.push(post.title); |
🧰 Tools
🪛 eslint
[error] 146-146: Expected '===' and instead saw '=='.
(eqeqeq)
[error] 147-147: Expected '===' and instead saw '=='.
(eqeqeq)
|
|
||
| export default languageDetector({ | ||
| supportedLngs: i18nextConfig.i18n.locales, | ||
| fallbackLng: i18nextConfig.i18n.defaultLocale | ||
| fallbackLng: i18nextConfig.i18n.defaultLocale, |
There was a problem hiding this comment.
Remove trailing comma to match ESLint rules
The trailing comma conflicts with the project's ESLint configuration which has comma-dangle set to "never".
Apply this fix:
- fallbackLng: i18nextConfig.i18n.defaultLocale,
+ fallbackLng: i18nextConfig.i18n.defaultLocale📝 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.
| fallbackLng: i18nextConfig.i18n.defaultLocale, | |
| fallbackLng: i18nextConfig.i18n.defaultLocale |
🧰 Tools
🪛 eslint
[error] 7-7: Delete ,
(prettier/prettier)
[error] 7-7: Unexpected trailing comma.
(comma-dangle)
| lang: lng, | ||
| }, |
There was a problem hiding this comment.
Remove trailing commas to match ESLint rules
Multiple trailing commas have been added that conflict with the project's ESLint configuration.
Apply these fixes:
- lang: lng,
- },
+ lang: lng
+ }
- paths: getI18nPaths(),
+ paths: getI18nPaths()
- ...(await serverSideTranslations(locale, ns)),
+ ...(await serverSideTranslations(locale, ns))
- props: await getI18nProps(ctx, ns as any),
+ props: await getI18nProps(ctx, ns as any)Also applies to: 22-22, 34-34, 48-48
🧰 Tools
🪛 eslint
[error] 12-12: Delete ,
(prettier/prettier)
[error] 12-12: Unexpected trailing comma.
(comma-dangle)
[error] 13-13: Delete ,
(prettier/prettier)
[error] 13-13: Unexpected trailing comma.
(comma-dangle)
There was a problem hiding this comment.
We haven't added trailingComma config in eslintrc. Why are you suggesting trailing commas?
There was a problem hiding this comment.
@akshatnema, it looks like the ESLint/Prettier configuration may be pulling in a “trailingComma” rule from shared or default settings, even if it’s not explicitly listed in your .eslintrc file. Sometimes frameworks or recommended configs bundle these rules by default. If you prefer to allow or enforce trailing commas, you can override that rule locally in your ESLint or Prettier configuration. Otherwise, you can remove any trailing commas to avoid the warnings.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (6)
scripts/index.ts (2)
Line range hint
13-47: Add type annotations and error handling to the start functionThe start function would benefit from proper TypeScript annotations and error handling for build steps.
Consider this improvement:
-async function start() { +async function start(): Promise<void> { try { await buildPostList(); - rssFeed('blog', 'AsyncAPI Initiative Blog RSS Feed', 'AsyncAPI Initiative Blog', 'rss.xml'); + await rssFeed('blog', 'AsyncAPI Initiative Blog RSS Feed', 'AsyncAPI Initiative Blog', 'rss.xml'); await buildCaseStudiesList('config/casestudies', resolve(currentDirPath, '../config', 'case-studies.json')); await buildAdoptersList(); const financeDir = resolve('.', 'config', 'finance'); // ... rest of the function } catch (error) { console.error('Failed to build site:', error); process.exit(1); } }
Line range hint
48-51: Prevent race condition in module executionThe immediate execution of
start()after export could cause issues in testing or when importing this module.Consider this pattern:
export { start }; -start(); +if (process.argv[1] === fileURLToPath(import.meta.url)) { + start().catch((error) => { + console.error('Failed to build site:', error); + process.exit(1); + }); +}scripts/build-pages.ts (2)
Line range hint
24-49: Add error handling to file system operationsThe copyAndRenameFiles function should handle potential file system errors.
-export function copyAndRenameFiles(srcDir: string, targetDir: string) { +export function copyAndRenameFiles(srcDir: string, targetDir: string): void { + try { const entries = fs.readdirSync(srcDir, { withFileTypes: true }); // ... rest of the function + } catch (error) { + console.error(`Failed to process directory ${srcDir}:`, error); + throw error; + } }
Line range hint
51-51: Add execution guard for direct script runningSimilar to other scripts, add a guard for direct execution.
-copyAndRenameFiles(SRC_DIR, TARGET_DIR); +if (process.argv[1] === fileURLToPath(import.meta.url)) { + try { + copyAndRenameFiles(SRC_DIR, TARGET_DIR); + } catch (error) { + console.error('Failed to build pages:', error); + process.exit(1); + } +}scripts/dashboard/issue-queries.ts (1)
Consider implementing pagination for comments query
Based on the verification results, there are issues in the AsyncAPI organization with significant comment counts (one issue has 376 comments, another has 125). With the current implementation using
comments(last: 100), some comments would be missed for issues with more than 100 comments. Implement cursor-based pagination to ensure all comments are retrieved.
scripts/dashboard/issue-queries.ts: Update the comments query to use pagination with cursorcomments(last: 100, before: $cursor) { pageInfo { hasPreviousPage startCursor } nodes { ...CommentFields } }🔗 Analysis chain
Line range hint
73-73: Review pagination settingsThe
comments(last: 100)in theissueByIdquery might hit rate limits with large repositories. Consider implementing pagination or reducing the page size.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are issues with many comments that might hit rate limits gh api graphql -f query=' query { search(query: "org:asyncapi is:issue comments:>100", type: ISSUE, first: 5) { nodes { ... on Issue { number comments { totalCount } } } } }'Length of output: 357
Script:
#!/bin/bash # Let's check the actual query implementation to understand the context better rg -U "issueById.*comments.*last:" --type tsLength of output: 46
Script:
#!/bin/bash # Let's try a broader search to find the GraphQL query implementation rg -U "comments.*last:" --type tsLength of output: 98
scripts/build-docs.ts (1)
Line range hint
1-205: Consider adding TypeScript interfacesSince we're migrating to TypeScript, consider adding interfaces to improve type safety and code documentation:
interface NavItem { title: string; weight: number; isRootSection?: boolean; isSection?: boolean; rootSectionId?: string; sectionWeight?: number; slug: string; parent?: string; sectionId?: string; isPrerelease?: boolean; } interface NavTree { [key: string]: { item: NavItem; children: { [key: string]: { item: NavItem; children: NavItem[]; }; }; }; }🧰 Tools
🪛 eslint
[error] 80-82: Blocks are nested too deeply (5). Maximum allowed is 4.
(max-depth)
[error] 138-138: Use array destructuring.
(prefer-destructuring)
[error] 146-146: Expected '===' and instead saw '=='.
(eqeqeq)
[error] 147-147: Expected '===' and instead saw '=='.
(eqeqeq)
♻️ Duplicate comments (5)
scripts/casestudies/index.ts (1)
12-12: 🛠️ Refactor suggestionUse path.join for cross-platform compatibility
Using string concatenation for path joining can lead to issues across different operating systems.
+import { join } from 'path'; // ... -const caseStudyFileName = [dirWithCaseStudy, file].join('/'); +const caseStudyFileName = join(dirWithCaseStudy, file);scripts/utils.ts (1)
3-8: 🛠️ Refactor suggestionImprove type safety with explicit types
The function should have explicit return type and input type annotations.
-function convertToJson(contentYAMLorJSON: string) { +function convertToJson(contentYAMLorJSON: string | unknown): unknown {scripts/build-tools.ts (1)
20-21:⚠️ Potential issueReplace dynamic require with static import
Using dynamic require with eslint-disable is not recommended in TypeScript/ESM.
Consider using fs.readFile for JSON:
- // eslint-disable-next-line import/no-dynamic-require, global-require - await combineTools(automatedTools, require(manualToolsPath), toolsPath, tagsPath); + const manualTools = JSON.parse(await fs.readFile(manualToolsPath, 'utf-8')); + await combineTools(automatedTools, manualTools, toolsPath, tagsPath);utils/languageDetector.ts (1)
7-7:⚠️ Potential issueRemove trailing comma to match ESLint rules
The trailing comma conflicts with the project's ESLint configuration which has
comma-dangleset to "never".- fallbackLng: i18nextConfig.i18n.defaultLocale, + fallbackLng: i18nextConfig.i18n.defaultLocale🧰 Tools
🪛 eslint
[error] 7-7: Delete
,(prettier/prettier)
[error] 7-7: Unexpected trailing comma.
(comma-dangle)
utils/getStatic.ts (1)
12-13:⚠️ Potential issueRemove trailing commas to comply with ESLint rules
Multiple trailing commas have been added that conflict with the project's ESLint configuration. As discussed in previous comments, the project doesn't have explicit trailing comma configuration.
Apply these fixes:
- lang: lng, - }, + lang: lng + } - paths: getI18nPaths(), + paths: getI18nPaths() - ...(await serverSideTranslations(locale, ns)), + ...(await serverSideTranslations(locale, ns)) - props: await getI18nProps(ctx, ns as any), + props: await getI18nProps(ctx, ns as any)Also applies to: 22-22, 34-34, 48-48
🧰 Tools
🪛 eslint
[error] 12-12: Delete
,(prettier/prettier)
[error] 12-12: Unexpected trailing comma.
(comma-dangle)
[error] 13-13: Delete
,(prettier/prettier)
[error] 13-13: Unexpected trailing comma.
(comma-dangle)
🧹 Nitpick comments (38)
scripts/build-newsroom-videos.ts (1)
17-63: Optional: Remove console logs or conditionally enable them.
Repeated console logs (lines 34 and 55) might clutter production logs. Consider removing or using a logger with adjustable log levels.scripts/build-rss.ts (1)
90-108: Optional: Improve enclosure defaults.
Currently, a single dummy value is used for enclosure lengths (line 94). Consider deriving a real file size to improve RSS clients’ accuracy.scripts/tools/tools-object.ts (1)
89-119: Drop or handle invalid tools more gracefully.
Logging invalid tool files is good, but consider storing them in a separate “invalidTools” list or continuing with safe defaults. Keeping partial data and logs might be confusing for end users.scripts/markdown/check-markdown.ts (4)
10-19: Ensure edge cases around “isValidURL” are handled.
Your URL check is simple and effective, but consider edge cases like empty strings or protocol-less URLs (e.g., "www.example.com"). It may be beneficial to return false for strings that do not contain a protocol or domain.
31-40: Handle missing required attributes more robustly.
If a blog post does not contain one of the required attributes, you currently push an error message but keep going. Consider returning early or adding more context to the error message (e.g., file name). This ensures quicker debugging and reduces the risk of incomplete data.
58-74: Validate the photo attribute similar to link validation.
You're already validating that 'link' is a valid URL. It might be useful to do the same for the 'photo' field, as broken or invalid URLs for author photos can lead to rendering issues.
95-122: Optimize for concurrent file reads.
Currently, we load each file’s content sequentially. You are using Promise.all in some places, but a deeper concurrency approach (like processing each file in parallel up front) may speed up validation, especially for large directories.scripts/tools/combine-tools.ts (1)
165-167: Maintain a single source for writing languages & technologies.
You’re writing two separate JSON files (tools and tags). If future changes occur, ensure consistent synchronization. You might consider storing them in a single structure if these always need to be updated together.scripts/dashboard/build-dashboard.ts (4)
25-31: Potential for date parsing errors.
“monthsSince” function relies on a properly formatted date. If the string cannot be parsed as a date, new Date('…') may produce unexpected results. Consider validating or throwing an error when the date is invalid.
70-77: Cascading recursion in getDiscussions may increase call depth.
The function calls itself recursively when hasNextPage is true. For large result sets, consider an iterative approach (while loop) to reduce the risk of a deep call stack.
Line range hint
96-139: Consider partial concurrency in processHotDiscussions.
This function processes discussions in batches of 5, then pauses for 1 second. If the rate limit constraints allow, you could reduce or remove the pause for improved throughput, or use a dynamic pause approach based on actual rateLimit data.
Line range hint
160-174: Capture more detail in error logs.
When writing to file fails, you log only the error’s message. Consider logging the entire error stack or relevant code snippet for deeper debugging.scripts/build-post-list.ts (4)
61-68: Ensure consistent classification for new sections.
When “details.slug” starts with “/about,” you place it into finalResult.about, but in future expansions (like new sections), you’d need an additional condition. Consider a more flexible approach (e.g., a lookup table) to handle future top-level sections.
95-103: Consider validating the _section.mdx file if it exists.
You do parse the frontMatter from “_section.mdx,” but you accept it even if mandatory fields are missing. For more consistency, you may want to reuse an existing validator (similar to other frontMatter validations).
123-126: Limit excerpt length gracefully.
Truncating to 200 chars might cut words abruptly. Consider using a word-based approach or adding ellipses (…) to indicate the text is longer.
178-188: Production environment logging.
You are suppressing console output in production. Verify if you need a debug or trace logging approach for diagnosing issues in a real environment.types/packages/jgexml__json2xml.d.ts (1)
6-8: Consider using ES module export syntax.Since this is part of a TypeScript migration, consider using modern ES module syntax:
- const json2xml: Json2Xml; - export = json2xml; + const json2xml: Json2Xml; + export default json2xml;next-i18next.config.cjs (1)
11-11: Improve inline comment documentation.The comment "this line" doesn't provide any context about why
useSuspenseis set tofalse.- react: { useSuspense: false } // this line + react: { useSuspense: false } // Disable Suspense to prevent hydration issues with SSRtypes/packages/markdown-toc.d.ts (1)
1-1: Update file path commentThe comment indicates
src/types/markdown-toc.d.tsbut the file is located attypes/packages/markdown-toc.d.ts.-// src/types/markdown-toc.d.ts +// types/packages/markdown-toc.d.tstypes/scripts/build-posts-list.ts (1)
7-27: Consider constraining the index signature typeThe index signature
[key: string]: anyallows any type of value for additional properties, which reduces TypeScript's type safety benefits.Consider constraining the type to more specific types that might be needed:
- [key: string]: any; // For any additional properties + [key: string]: string | number | boolean | undefined; // For any additional propertiesscripts/utils/readAndWriteJson.ts (1)
5-29: Enhance error handling and add return typeThe function implementation is good, but could be improved in several ways:
- Add explicit return type
- Include more specific error context
- Add formatting to JSON output
-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(`Failed to read file from ${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(`Failed to convert content from ${readPath} to JSON\nError: ${err}`); } // Attempt to write the JSON content to file try { - await writeFile(writePath, JSON.stringify(jsonContent)); + await writeFile(writePath, JSON.stringify(jsonContent, null, 2)); } catch (err) { - throw new Error(`Error while writing file\nError: ${err}`); + throw new Error(`Failed to write JSON to ${writePath}\nError: ${err}`); } }types/scripts/build-rss.ts (1)
18-37: Consider extracting XML attribute typesThe RSS type effectively captures the RSS structure, but could be more maintainable by extracting common XML attribute patterns.
Consider creating a reusable type for XML attributes:
type XMLAttribute = `@${string}`; type XMLAttributes = { [K in XMLAttribute]: string; }; export type RSS = XMLAttributes & { channel: { // ... rest of the type definition }; };scripts/casestudies/index.ts (2)
5-5: Add return type annotation for better type safetyThe function should explicitly declare its return type for better type safety and documentation.
-export async function buildCaseStudiesList(dirWithCaseStudy: string, writeFilePath: string) { +export async function buildCaseStudiesList(dirWithCaseStudy: string, writeFilePath: string): Promise<unknown[]> {Also applies to: 22-22
23-25: Enhance error handling with specific error typesThe current error handling loses the stack trace and type information. Consider preserving the original error or using a custom error class.
- } catch (err) { - throw new Error(err instanceof Error ? err.message : String(err)); + } catch (err) { + if (err instanceof Error) { + throw err; + } + throw new Error(String(err)); }scripts/utils.ts (1)
23-23: Structure error messages for better debuggingThe current error message concatenation makes it difficult to programmatically handle different error types.
- throw new Error(`Invalid content format:\nJSON Parse Error: ${jsonError}\nYAML Parse Error: ${yamlError}`); + throw new Error(JSON.stringify({ + message: 'Invalid content format', + jsonError: jsonError instanceof Error ? jsonError.message : String(jsonError), + yamlError: yamlError instanceof Error ? yamlError.message : String(yamlError) + }));scripts/finance/index.ts (1)
23-24: Consider centralizing path configurationsFile paths and names are hardcoded throughout the function. Consider moving these to a configuration object or constants.
+const FINANCE_FILES = { + expenses: { + source: 'Expenses.yml', + target: 'Expenses.json' + }, + expensesLink: { + source: 'ExpensesLink.yml', + target: 'ExpensesLink.json' + } +} as const;Also applies to: 32-33, 36-37
scripts/build-tools.ts (1)
13-26: Add return type annotation to buildTools functionThe function's return type should be explicitly declared for better type safety.
-const buildTools = async (automatedToolsPath: string, manualToolsPath: string, toolsPath: string, tagsPath: string) => { +const buildTools = async (automatedToolsPath: string, manualToolsPath: string, toolsPath: string, tagsPath: string): Promise<void> => {scripts/build-pages.ts (1)
Line range hint
4-8: Add type annotations to constantsThe constants would benefit from explicit type annotations.
-const SRC_DIR = 'markdown'; -const TARGET_DIR = 'pages'; -const capitalizeTags = ['table', 'tr', 'td', 'th', 'thead', 'tbody']; +const SRC_DIR: string = 'markdown'; +const TARGET_DIR: string = 'pages'; +const capitalizeTags: readonly string[] = ['table', 'tr', 'td', 'th', 'thead', 'tbody'] as const;types/scripts/tools.ts (1)
7-25: Consider using an enum for Category typeWhile the union type works, using an enum would provide better type safety and maintainability.
-type Category = - | 'api' - | 'code-first' - // ... other categories - | 'ide-extension'; +enum Category { + API = 'api', + CODE_FIRST = 'code-first', + // ... other categories + IDE_EXTENSION = 'ide-extension' +}scripts/build-meetings.ts (1)
42-44: Consider using type guard for event validationInstead of throwing an error, consider using a type guard to filter out invalid events.
-if (!e.start || !e.start.dateTime) { - throw new Error('start.dateTime is missing in the event'); -} +const isValidEvent = (e: any): e is { start: { dateTime: string } } => { + return e?.start?.dateTime != null; +}; + +eventsItems = eventsList.data.items + .filter(isValidEvent) + .map((e) => {scripts/tools/tags-color.ts (3)
164-172: Standardize color schemes for technologiesJenkins and Flask are using the same color scheme as Kubernetes-native. Consider using distinct colors for better visual differentiation.
{ name: 'Jenkins', - color: 'bg-[#D7C7F2]', - borderColor: 'border-[#A387D2]' + color: 'bg-[#335061]', + borderColor: 'border-[#D33833]' }, { name: 'Flask', - color: 'bg-[#D7C7F2]', - borderColor: 'border-[#A387D2]' + color: 'bg-[#FFFFFF]', + borderColor: 'border-[#000000]' }
6-6: Standardize language namingFor consistency, consider using just 'Go' instead of 'Go/Golang' as other language names use their primary name.
- name: 'Go/Golang', + name: 'Go',
1-1: Consider enhancing type safetyConsider using string literals for the
namefield inLanguageColorItemtype to prevent typos and ensure consistency.type LanguageName = 'Go' | 'Java' | 'JavaScript' | /* ... other languages ... */; type TechnologyName = 'Node.js' | 'Hermes' | /* ... other technologies ... */; type ColorItem<T extends string> = { name: T; color: string; borderColor: string; };scripts/dashboard/issue-queries.ts (1)
1-1: Consider adding TypeScript types for query responsesAdd TypeScript interfaces for the GraphQL query responses to improve type safety and developer experience.
interface PullRequestResponse { node: { __typename: 'PullRequest'; assignees: { totalCount: number; }; // ... other fields }; rateLimit: RateLimit; } export const Queries = Object.freeze({ pullRequestById: /* GraphQL */ `...` as const, // ... other queries });scripts/compose.ts (1)
19-23: Add input validation for tagsConsider adding validation for the tags input to ensure they meet your formatting requirements.
const tagArray = answers.tags.split(','); + const validTagRegex = /^[a-z0-9-]+$/i; tagArray.forEach((tag: string, index: number) => { - tagArray[index] = tag.trim(); + const trimmedTag = tag.trim(); + if (!validTagRegex.test(trimmedTag)) { + throw new Error(`Invalid tag format: ${trimmedTag}`); + } + tagArray[index] = trimmedTag; });scripts/build-docs.ts (1)
113-115: Improve error handlingThe error messages could be more descriptive by including the original error details:
- throw new Error('Error in convertDocPosts:', err); + throw new Error(`Error in convertDocPosts: ${err.message}`);- throw new Error('An error occurred while adding doc buttons:', err); + throw new Error(`An error occurred while adding doc buttons: ${err.message}`);Also applies to: 199-200
scripts/tools/tools-schema.json (2)
14-15: Fix grammatical errors in descriptionThe description contains grammatical errors that should be corrected for better clarity.
-"description": "By default scripts read description of repository there project is stored. You can override this behaviour by providing custom description." +"description": "By default, scripts read the description from the repository where the project is stored. You can override this behavior by providing a custom description."
147-147: Fix typo in categories descriptionThe description contains a grammatical error that affects readability.
-"description": "Categories are used to group tools by different use case, like documentation or code generation. If have a list of fixed categories. If you use different one that your tool lands under \"other\" category. Feel free to add your category through a pull request to AsyncAPI website repository.", +"description": "Categories are used to group tools by different use cases, like documentation or code generation. We have a list of fixed categories. If you use a different one, your tool will land under the \"other\" category. Feel free to add your category through a pull request to the AsyncAPI website repository."
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (53)
.eslintrc(1 hunks).github/workflows/if-nodejs-pr-testing.yml(1 hunks)next-i18next.config.cjs(1 hunks)next-i18next.config.js(0 hunks)package.json(3 hunks)pages/_document.tsx(1 hunks)scripts/adopters/index.js(0 hunks)scripts/adopters/index.ts(1 hunks)scripts/build-docs.ts(7 hunks)scripts/build-meetings.ts(2 hunks)scripts/build-newsroom-videos.js(0 hunks)scripts/build-newsroom-videos.ts(1 hunks)scripts/build-pages.ts(2 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.ts(9 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-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/readAndWriteJson.js(0 hunks)scripts/utils/readAndWriteJson.ts(1 hunks)tsconfig.json(1 hunks)types/packages/jgexml__json2xml.d.ts(1 hunks)types/packages/markdown-toc.d.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(5 hunks)utils/languageDetector.ts(1 hunks)
💤 Files with no reviewable changes (16)
- scripts/adopters/index.js
- scripts/tools/extract-tools-github.js
- next-i18next.config.js
- scripts/tools/categorylist.js
- scripts/utils.js
- scripts/build-tools.js
- scripts/tools/combine-tools.js
- scripts/tools/tags-color.js
- scripts/utils/readAndWriteJson.js
- scripts/build-post-list.js
- scripts/casestudies/index.js
- scripts/build-rss.js
- scripts/tools/tools-object.js
- scripts/build-newsroom-videos.js
- scripts/markdown/check-markdown.js
- scripts/finance/index.js
✅ Files skipped from review due to trivial changes (1)
- pages/_document.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
scripts/tools/extract-tools-github.ts
[error] 18-18: 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)
types/packages/markdown-toc.d.ts
[error] 29-29: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
scripts/build-meetings.ts
[error] 52-52: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 eslint
scripts/build-docs.ts
[error] 138-138: Use array destructuring.
(prefer-destructuring)
[error] 146-146: Expected '===' and instead saw '=='.
(eqeqeq)
[error] 147-147: Expected '===' and instead saw '=='.
(eqeqeq)
utils/getStatic.ts
[error] 12-12: Delete ,
(prettier/prettier)
[error] 12-12: Unexpected trailing comma.
(comma-dangle)
[error] 13-13: Delete ,
(prettier/prettier)
[error] 13-13: Unexpected trailing comma.
(comma-dangle)
[error] 22-22: Delete ,
(prettier/prettier)
[error] 22-22: Unexpected trailing comma.
(comma-dangle)
[error] 34-34: Delete ,
(prettier/prettier)
[error] 34-34: Unexpected trailing comma.
(comma-dangle)
[error] 48-48: Delete ,
(prettier/prettier)
[error] 48-48: Unexpected trailing comma.
(comma-dangle)
utils/languageDetector.ts
[error] 7-7: Delete ,
(prettier/prettier)
[error] 7-7: Unexpected trailing comma.
(comma-dangle)
🔇 Additional comments (32)
scripts/build-newsroom-videos.ts (2)
12-15: Check for undefined environment variables.
Please ensure that "YOUTUBE_TOKEN" is present before creating the YouTube client.
46-50: Use type guards instead of non-null assertions.
Applying type guards or optional chaining will make the code more robust.
types/scripts/dashboard.ts (1)
107-139: Reduce duplication via interface inheritance.
“HotDiscussionsIssuesNode” and “HotDiscussionsPullRequestsNode” repeat many properties. Extending from a base interface can improve maintainability.
scripts/build-rss.ts (1)
13-24: Consider a more robust sanitization approach.
Manual string replacements may miss edge cases or lead to partial sanitization. A templating or sanitization library might reduce XSS risks.
scripts/tools/tools-object.ts (1)
80-86: Validate URL references.
You rely on GitHub’s raw URL for the tool file (line 81). Ensure all fetch failures are handled to avoid partial tool listings.
scripts/markdown/check-markdown.ts (1)
143-161: Exit code logic is well-structured.
The approach of setting process.exit(1) only after all checks have run allows for complete error logging. This ensures thorough scanning through the entire folder, which is beneficial for debugging.
scripts/tools/combine-tools.ts (2)
46-50: Ensure filters are defined before accessing properties.
This was noted in a previous review: if toolObject.filters is undefined, referencing toolObject.filters.language can cause runtime errors. Add a guard check:
-if (toolObject.filters.language) {
+if (toolObject.filters?.language) {124-157: Validation approach is non-blocking, which is great.
The script continues execution even if some manual tools fail schema validation. This is good for batch processing. Just make sure you can easily track any invalid tool entries so they can be fixed before going live.
scripts/dashboard/build-dashboard.ts (1)
Line range hint 179-193: mapGoodFirstIssues logic is concise.
The function’s filtering approach for labels is clear and straightforward. This is a well-structured transformation, reducing overhead for later code.
scripts/build-post-list.ts (1)
37-52: Confirm correct slug generation for headings.
By design, the “slugifyToC” function tries to parse inline heading IDs first, then fallback to default slugify logic. Ensure no collisions occur if multiple headings share the same text but differ only by custom anchors.
Please run this script to check for duplicate headings with different anchors:
✅ Verification successful
Heading slug generation implementation is safe and working as expected.
The verification shows that:
- The codebase uses
<a name="..."/>style anchors consistently, primarily in specification documentation. - No instances of
{#...}style heading IDs were found, but the code safely handles this pattern if introduced in the future. - All found anchor names follow consistent patterns (alphanumeric with hyphens/underscores) that are compatible with the slugify function.
- No cases of duplicate heading text with different anchors were found that could cause collisions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# This script searches for headings in .mdx files that might produce the same slug
rg "^#{1,6} .*\{#.*\}" -A1
Length of output: 28
Script:
#!/bin/bash
# Let's try a broader search for both heading patterns mentioned in the code
# Pattern 1: Headings with {#...} style IDs
rg "^#{1,6}.*\{#.*\}" --type md --type mdx
# Pattern 2: Headings with <a name="..."/> style anchors
rg "^#{1,6}.*<a.*name=" --type md --type mdx
# Also search for any markdown files to confirm they exist
fd "\.(md|mdx)$"
Length of output: 11112
Script:
#!/bin/bash
# Let's try the search without file type restrictions since we found the markdown files
# Pattern 1: Headings with {#...} style IDs
rg "^#{1,6}.*\{#.*\}"
# Pattern 2: Headings with <a name="..."/> style anchors
rg "^#{1,6}.*<a.*name="
# Let's also check the actual implementation of slugify function that's being used as fallback
rg -l "slugify.*\{.*firsth1.*maxdepth"
Length of output: 6878
types/packages/jgexml__json2xml.d.ts (1)
1-4: LGTM! Well-structured type declaration.
The interface definition is clear and type-safe. Good use of unknown type for the feed parameter instead of any, following TypeScript best practices.
next-i18next.config.cjs (1)
1-3: LGTM! Well-documented configuration setup.
The comment clearly explains the reason for the .cjs extension and includes a reference link for future maintainers.
scripts/tools/extract-tools-github.ts (2)
1-4: Verify environment variable presence
The environment variable validation is still missing.
6-20: Add TypeScript types and improve error handling
The function still needs proper TypeScript types and better error handling.
🧰 Tools
🪛 Biome (1.9.4)
[error] 18-18: 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)
tsconfig.json (1)
21-24: Review module resolution strategy
The moduleResolution: "bundler" in compilerOptions might conflict with ts-node's experimentalSpecifierResolution: "node". Consider using consistent resolution strategies or adding a separate tsconfig for ts-node.
types/packages/markdown-toc.d.ts (1)
28-30: Replace broad 'Object' type with specific interface
The options parameter type should be more specific to improve type safety.
🧰 Tools
🪛 Biome (1.9.4)
[error] 29-29: Don't use 'Object' as a type.
Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.
(lint/complexity/noBannedTypes)
types/scripts/build-posts-list.ts (2)
1-5: LGTM! Well-structured interface for table of contents.
The interface is clear and uses appropriate types for its purpose.
29-34: Replace any[] with specific types
The Result interface uses any[] which defeats TypeScript's type safety benefits.
types/scripts/build-rss.ts (1)
9-17: Improve type safety of the RSSItemType
The guid property is typed as any, which defeats TypeScript's type checking benefits.
scripts/finance/index.ts (2)
15-21: Add return type annotation
The async function should have an explicit return type.
export async function buildFinanceInfoList({
currentDir,
configDir,
financeDir,
year,
jsonDataDir
-}: BuildFinanceInfoListProps) {
+}: BuildFinanceInfoListProps): Promise<void> {34-34: 🛠️ Refactor suggestion
Add file existence checks before operations
The function should verify that source files exist before attempting to write JSON files.
+import { access } from 'fs/promises';
+
+async function checkFileExists(path: string): Promise<boolean> {
+ try {
+ await access(path);
+ return true;
+ } catch {
+ return false;
+ }
+}
// In the main function
+if (!(await checkFileExists(expensesPath))) {
+ throw new Error(`Source file not found: ${expensesPath}`);
+}
await writeJSON(expensesPath, expensesJsonPath);Also applies to: 38-38
types/scripts/tools.ts (1)
1-5: LGTM: Well-structured Links interface with clear documentation
The Links interface is well-documented with clear comments explaining each optional URL's purpose.
scripts/build-meetings.ts (2)
36-39: Good addition of response validation
The explicit check for valid response structure helps prevent runtime errors.
13-13: Add environment variable validation
Similar to the existing review comment about calendar credentials validation.
scripts/tools/categorylist.ts (1)
22-26: Fix incorrect description for Converters category
The description for converters is incorrect as noted in previous review.
scripts/tools/tags-color.ts (1)
154-157: Remove duplicate Scala entry
Scala is already defined in the languagesColor array (lines 51-54). It should not appear in the technologiesColor array.
.github/workflows/if-nodejs-pr-testing.yml (1)
70-72: Consider using npm ci instead of npm install
The change from npm ci to npm install might lead to inconsistent dependency installations across different environments. npm ci is preferred in CI environments as it:
- Uses package-lock.json exclusively
- Is more reliable for reproducible builds
- Removes node_modules before installation
- run: npm install
+ run: npm cipackage.json (1)
6-6: LGTM! TypeScript migration changes look good
The changes consistently implement TypeScript support across the project:
- ES modules are properly enabled
- Script commands are updated to use tsx
- Required TypeScript dependencies and type definitions are added
Also applies to: 11-12, 14-14, 22-25, 27-27, 118-118, 134-135
scripts/build-docs.ts (1)
146-147: Fix loose equality comparisons
Replace loose equality (==) with strict equality (===) for type-safe comparisons.
-if (post?.isRootSection || post?.isSection || index == 0) {
- if (post?.isRootSection || index == 0) rootSections.push(post.title);
+if (post?.isRootSection || post?.isSection || index === 0) {
+ if (post?.isRootSection || index === 0) rootSections.push(post.title);🧰 Tools
🪛 eslint
[error] 146-146: Expected '===' and instead saw '=='.
(eqeqeq)
[error] 147-147: Expected '===' and instead saw '=='.
(eqeqeq)
.eslintrc (1)
317-322: LGTM! Rule overrides are well-justified
The override for scripts directory is appropriate:
- Disabling
no-consoleis necessary for CLI feedback - Disabling
require-jsdocis reasonable for scripts - Disabling
import/no-extraneous-dependenciesmakes sense for build scripts
utils/languageDetector.ts (1)
3-3: LGTM! Import path updated correctly
The change to import from .cjs extension is appropriate as part of the TypeScript migration.
utils/getStatic.ts (1)
3-3: Verify the existence of next-i18next.config.cjs
The import path has been updated to use the .cjs extension, which is correct for CommonJS modules. However, let's verify that this file exists and the migration is complete.
✅ Verification successful
Import path update to .cjs is correct and verified
The file next-i18next.config.cjs exists and is properly exported using CommonJS module syntax (module.exports), confirming that the import path change in utils/getStatic.ts is correct and aligned with the codebase structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the config file exists and verify its module type
if [ -f "next-i18next.config.cjs" ]; then
echo "File exists. Checking content..."
# Check if it's properly exported as CommonJS
rg "module.exports" next-i18next.config.cjs
else
echo "Warning: next-i18next.config.cjs not found"
# Check if old file still exists
[ -f "next-i18next.config.js" ] && echo "Warning: Old config file still exists"
fi
Length of output: 174
| } | ||
| } else { | ||
| for (const language of toolObject.filters.language) { | ||
| const languageSearch = await languageFuse.search(language); | ||
|
|
||
| if (languageSearch.length > 0) { | ||
| languageArray.push(languageSearch[0].item); | ||
| } else { | ||
| // adds a new language object in the Fuse list as well as in tool object | ||
| // so that it isn't missed out in the UI. | ||
| const languageObject = { | ||
| name: language as string, | ||
| color: 'bg-[#57f281]', | ||
| borderColor: 'border-[#37f069]' | ||
| }; | ||
|
|
||
| languageList.push(languageObject); | ||
| languageArray.push(languageObject); | ||
| languageFuse = new Fuse(languageList, options); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reinitialize Fuse instances only when necessary.
Each time a new language is added, we recreate the Fuse instance. While this ensures up-to-date searches, it might be an overkill if many tools have distinct languages. Consider batching these additions if performance is a concern.
| export async function buildAdoptersList() { | ||
| writeJSON('config/adopters.yml', resolve(currentDirPath, '../../config', 'adopters.json')); | ||
| } |
There was a problem hiding this comment.
Add error handling and improve path resolution.
The function lacks error handling for the writeJSON operation and could benefit from more robust path resolution.
Consider this implementation:
export async function buildAdoptersList() {
- writeJSON('config/adopters.yml', resolve(currentDirPath, '../../config', 'adopters.json'));
+ try {
+ const outputPath = 'config/adopters.yml';
+ const inputPath = resolve(currentDirPath, '../../config', 'adopters.json');
+ await writeJSON(outputPath, inputPath);
+ console.log(`Successfully built adopters list at ${outputPath}`);
+ } catch (error) {
+ console.error('Failed to build adopters list:', error);
+ throw error;
+ }
}📝 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() { | |
| try { | |
| const outputPath = 'config/adopters.yml'; | |
| const inputPath = resolve(currentDirPath, '../../config', 'adopters.json'); | |
| await writeJSON(outputPath, inputPath); | |
| console.log(`Successfully built adopters list at ${outputPath}`); | |
| } catch (error) { | |
| console.error('Failed to build adopters list:', error); | |
| throw error; | |
| } | |
| } |
| const result = await axios.get('https://api.github.com/search/code?q=filename:.asyncapi-tool', { | ||
| headers: { | ||
| accept: 'application/vnd.github.text-match+json', | ||
| authorization: `token ${process.env.GITHUB_TOKEN}` | ||
| } | ||
| }); |
There was a problem hiding this comment.
Add pagination handling for GitHub API
The GitHub Search API is paginated and may return incomplete results. Consider implementing pagination handling to ensure all tools are retrieved.
- const result = await axios.get('https://api.github.com/search/code?q=filename:.asyncapi-tool', {
+ const results = [];
+ let page = 1;
+ let hasNextPage = true;
+
+ while (hasNextPage) {
+ const result = await axios.get(
+ `https://api.github.com/search/code?q=filename:.asyncapi-tool&page=${page}`,
+ {
+ headers: {
+ accept: 'application/vnd.github.text-match+json',
+ authorization: `token ${process.env.GITHUB_TOKEN}`
+ }
+ }
+ );
+
+ results.push(...result.data.items);
+ hasNextPage = result.data.items.length === 30; // GitHub's default page size
+ page++;
+ }
+
+ return { items: results };Committable suggestion skipped: line range outside the PR's diff.
| json: TocItem[]; | ||
| content: string; | ||
| highest: number; | ||
| tokens: any[]; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace 'any[]' with specific type for tokens
The tokens property uses any[] which reduces type safety. Consider defining a specific type for the tokens array.
- tokens: any[];
+ tokens: Array<{
+ type: string;
+ text: string;
+ depth?: number;
+ }>;📝 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.
| tokens: any[]; | |
| tokens: Array<{ | |
| type: string; | |
| text: string; | |
| depth?: number; | |
| }>; |
types/scripts/build-rss.ts
Outdated
| export type BlogPostTypes = 'docs' | 'blog' | 'about' | 'docsTree'; | ||
| export type Enclosure = { | ||
| '@url': string; | ||
| '@length': number; | ||
| '@type': string; | ||
| enclosure: Enclosure; | ||
| }; |
There was a problem hiding this comment.
Fix potential infinite recursion in Enclosure type
The Enclosure type includes itself recursively without a termination condition, which could lead to infinite type recursion.
Consider making the recursive property optional:
export type Enclosure = {
'@url': string;
'@length': number;
'@type': string;
- enclosure: Enclosure;
+ enclosure?: Enclosure;
};📝 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 type BlogPostTypes = 'docs' | 'blog' | 'about' | 'docsTree'; | |
| export type Enclosure = { | |
| '@url': string; | |
| '@length': number; | |
| '@type': string; | |
| enclosure: Enclosure; | |
| }; | |
| export type BlogPostTypes = 'docs' | 'blog' | 'about' | 'docsTree'; | |
| export type Enclosure = { | |
| '@url': string; | |
| '@length': number; | |
| '@type': string; | |
| enclosure?: Enclosure; | |
| }; |
| interface Filters { | ||
| language?: Array<LanguageColorItem | string>; | ||
| technology?: Array<LanguageColorItem | string>; | ||
| categories: Array<Category | string>; // Categories are used to group tools by different use cases. | ||
| hasCommercial?: boolean; // Indicate if your tool is open source or commercial offering. | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider stricter typing for Filters interface
The union with string in arrays allows any string value, which could lead to runtime errors.
interface Filters {
- language?: Array<LanguageColorItem | string>;
- technology?: Array<LanguageColorItem | string>;
- categories: Array<Category | string>;
+ language?: Array<LanguageColorItem>;
+ technology?: Array<LanguageColorItem>;
+ categories: Array<Category>;
hasCommercial?: boolean;
}📝 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.
| interface Filters { | |
| language?: Array<LanguageColorItem | string>; | |
| technology?: Array<LanguageColorItem | string>; | |
| categories: Array<Category | string>; // Categories are used to group tools by different use cases. | |
| hasCommercial?: boolean; // Indicate if your tool is open source or commercial offering. | |
| } | |
| interface Filters { | |
| language?: Array<LanguageColorItem>; | |
| technology?: Array<LanguageColorItem>; | |
| categories: Array<Category>; | |
| hasCommercial?: boolean; // Indicate if your tool is open source or commercial offering. | |
| } |
| `https://github.com/asyncapi/community/issues/${e.extendedProperties.private.ISSUE_ID}`, | ||
| banner: | ||
| e.extendedProperties?.private && e.extendedProperties.private.BANNER, | ||
| date: new Date(e.start.dateTime), | ||
| banner: e.extendedProperties?.private && e.extendedProperties.private.BANNER, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use optional chaining as suggested by static analysis
The static analysis tool correctly identifies a potential null reference.
- e.extendedProperties?.private &&
- `https://github.com/asyncapi/community/issues/${e.extendedProperties.private.ISSUE_ID}`,
+ e.extendedProperties?.private?.ISSUE_ID &&
+ `https://github.com/asyncapi/community/issues/${e.extendedProperties.private.ISSUE_ID}`,Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 52-52: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| name: 'AsyncAPI Generator Templates', | ||
| tag: 'generator-template', | ||
| description: | ||
| 'The following is a list of templates compatible with AsyncAPI Generator. You can use them to generate apps, clients or documentation from your AsyncAPI documents.' | ||
| }, |
There was a problem hiding this comment.
Ensure generator-template tag is included in Category type
The 'generator-template' tag used here isn't included in the Category type definition.
Add the tag to the Category type in types/scripts/tools.ts:
type Category =
| 'api'
| 'code-first'
// ... other categories
+ | 'generator-template'
| 'ide-extension';Committable suggestion skipped: line range outside the PR's diff.
| fs.writeFile(filePath, frontMatter, { flag: 'wx' }, (err) => { | ||
| if (err) { | ||
| throw err | ||
| throw err; | ||
| } else { | ||
| console.log(`Blog post generated successfully at ${filePath}`) | ||
| console.log(`Blog post generated successfully at ${filePath}`); | ||
| } | ||
| }) | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling for file operations
The current error handling could be more specific. Consider handling different types of errors (e.g., file already exists, permission issues) separately.
- fs.writeFile(filePath, frontMatter, { flag: 'wx' }, (err) => {
- if (err) {
- throw err;
- } else {
- console.log(`Blog post generated successfully at ${filePath}`);
- }
- });
+ fs.writeFile(filePath, frontMatter, { flag: 'wx' }, (err) => {
+ if (err) {
+ if (err.code === 'EEXIST') {
+ console.error(`File ${filePath} already exists`);
+ } else if (err.code === 'EACCES') {
+ console.error(`Permission denied to create ${filePath}`);
+ } else {
+ console.error(`Error creating file: ${err.message}`);
+ }
+ process.exit(1);
+ }
+ console.log(`Blog post generated successfully at ${filePath}`);
+ });📝 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.
| fs.writeFile(filePath, frontMatter, { flag: 'wx' }, (err) => { | |
| if (err) { | |
| throw err | |
| throw err; | |
| } else { | |
| console.log(`Blog post generated successfully at ${filePath}`) | |
| console.log(`Blog post generated successfully at ${filePath}`); | |
| } | |
| }) | |
| }); | |
| fs.writeFile(filePath, frontMatter, { flag: 'wx' }, (err) => { | |
| if (err) { | |
| if (err.code === 'EEXIST') { | |
| console.error(`File ${filePath} already exists`); | |
| } else if (err.code === 'EACCES') { | |
| console.error(`Permission denied to create ${filePath}`); | |
| } else { | |
| console.error(`Error creating file: ${err.message}`); | |
| } | |
| process.exit(1); | |
| } | |
| console.log(`Blog post generated successfully at ${filePath}`); | |
| }); |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
utils/getStatic.ts (1)
Line range hint
44-51: Improve type safety by removinganytypesThe function uses
anytypes which bypass TypeScript's type checking. Let's improve type safety:+ import type { GetStaticPropsContext, GetStaticPropsResult } from 'next'; + - export function makeStaticProps(ns = {}) { + export function makeStaticProps(ns: string[] = ['common']) { - return async function getStaticProps(ctx: any) { + return async function getStaticProps( + ctx: GetStaticPropsContext<{ lang: string }> + ): Promise<GetStaticPropsResult<unknown>> { return { props: await getI18nProps(ctx, ns) }; }; }🧰 Tools
🪛 eslint
[error] 48-48: Delete
,(prettier/prettier)
[error] 48-48: Unexpected trailing comma.
(comma-dangle)
♻️ Duplicate comments (4)
utils/getStatic.ts (4)
12-13:⚠️ Potential issueRemove trailing commas to match ESLint rules
The trailing commas conflict with the project's ESLint configuration.
Apply this fix:
- lang: lng, - }, + lang: lng + }🧰 Tools
🪛 eslint
[error] 12-12: Delete
,(prettier/prettier)
[error] 12-12: Unexpected trailing comma.
(comma-dangle)
[error] 13-13: Delete
,(prettier/prettier)
[error] 13-13: Unexpected trailing comma.
(comma-dangle)
22-22:⚠️ Potential issueRemove trailing comma to match ESLint rules
The trailing comma conflicts with the project's ESLint configuration.
Apply this fix:
- paths: getI18nPaths(), + paths: getI18nPaths()🧰 Tools
🪛 eslint
[error] 22-22: Delete
,(prettier/prettier)
[error] 22-22: Unexpected trailing comma.
(comma-dangle)
34-34:⚠️ Potential issueRemove trailing comma to match ESLint rules
The trailing comma conflicts with the project's ESLint configuration.
Apply this fix:
- ...(await serverSideTranslations(locale, ns)), + ...(await serverSideTranslations(locale, ns))🧰 Tools
🪛 eslint
[error] 34-34: Delete
,(prettier/prettier)
[error] 34-34: Unexpected trailing comma.
(comma-dangle)
48-48:⚠️ Potential issueRemove trailing comma to match ESLint rules
The trailing comma conflicts with the project's ESLint configuration.
Apply this fix:
- props: await getI18nProps(ctx, ns as any), + props: await getI18nProps(ctx, ns as any)🧰 Tools
🪛 eslint
[error] 48-48: Delete
,(prettier/prettier)
[error] 48-48: Unexpected trailing comma.
(comma-dangle)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
utils/getStatic.ts(5 hunks)
🧰 Additional context used
🪛 eslint
utils/getStatic.ts
[error] 12-12: Delete ,
(prettier/prettier)
[error] 12-12: Unexpected trailing comma.
(comma-dangle)
[error] 13-13: Delete ,
(prettier/prettier)
[error] 13-13: Unexpected trailing comma.
(comma-dangle)
[error] 22-22: Delete ,
(prettier/prettier)
[error] 22-22: Unexpected trailing comma.
(comma-dangle)
[error] 34-34: Delete ,
(prettier/prettier)
[error] 34-34: Unexpected trailing comma.
(comma-dangle)
[error] 48-48: Delete ,
(prettier/prettier)
[error] 48-48: Unexpected trailing comma.
(comma-dangle)
🔇 Additional comments (1)
utils/getStatic.ts (1)
3-3: LGTM! Good use of explicit .cjs extension
The change to use .cjs extension explicitly indicates CommonJS module format, which is good practice for clarity in mixed module environments.
| @@ -31,7 +31,7 @@ export const getStaticPaths = () => ({ | |||
| export async function getI18nProps(ctx: any, ns = ['common']) { | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding proper TypeScript types for the context parameter
Since we're migrating to TypeScript, we should avoid using any type as it defeats TypeScript's type safety benefits. Consider using Next.js's built-in types.
- export async function getI18nProps(ctx: any, ns = ['common']) {
+ import type { GetStaticPropsContext } from 'next';
+ export async function getI18nProps(
+ ctx: GetStaticPropsContext<{ lang: string }>,
+ ns = ['common']
+ ) {Committable suggestion skipped: line range outside the PR's diff.
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3423--asyncapi-website.netlify.app/ |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
scripts/build-docs.ts (1)
36-39: Gracefully handle missing parent sections.
Whenitem.parentis missing in the tree, the code throws an error. Consider logging available section keys for better troubleshooting context, or handle the case in which the parent is optional to avoid unexpected exceptions.types/scripts/build-posts-list.ts (1)
33-33: Avoid broad “any” property expansions.
Using[key: string]: any;can undermine type safety. Consider specifying a more precise type or using generics to limit usage.scripts/build-post-list.ts (2)
74-117: Consider splitting out recursion for clarity and maintainability.
walkDirectoriesis a large function that uses nested loops and recursive calls. Breaking it into smaller helper functions can simplify debugging and testing, reducing cognitive overhead.
183-185: Finalize writing results outside of production check.
You currently write toposts.jsonunconditionally at line 189, but consider running this step even ifNODE_ENVis not production to simplify local development workflows.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
scripts/build-docs.ts(1 hunks)scripts/build-post-list.ts(1 hunks)types/scripts/build-docs.ts(1 hunks)types/scripts/build-posts-list.ts(1 hunks)
🔇 Additional comments (3)
types/scripts/build-docs.ts (1)
3-7: Good use of typed navigation structures.
The new NavTreeItem, RecursiveChildren, and NavTree types provide clarity on the hierarchical relationships. Keep an eye on maintaining consistency when children can be either RecursiveChildren or an array of Details.
Also applies to: 10-12
types/scripts/build-posts-list.ts (1)
36-41: Ensure references to your Result interface remain consistent.
The Result interface neatly organizes your content arrays and docsTree. If you plan to expand it further, maintain descriptive property names and keep them strongly typed to avoid confusion later on.
scripts/build-post-list.ts (1)
117-117: Avoid infinite recursion with cyclical directories.
When calling walkDirectories recursively, ensure there’s no potential cycle in directory references that could cause a crash or memory blowout.
| if (rootKey === 'reference' && key === 'specification') { | ||
| allChildren[key].item.href = childrenOfAllChildren.find((c) => c.isPrerelease === undefined)!.slug; | ||
| } |
There was a problem hiding this comment.
Check for potential undefined value in .find() result.
Using the non-null assertion operator (!) can cause runtime errors if find() returns undefined. Safely handle this with a conditional check or a fallback error, for example:
- allChildren[key].item.href = childrenOfAllChildren.find((c) => c.isPrerelease === undefined)!.slug;
+ const foundItem = childrenOfAllChildren.find((c) => c.isPrerelease === undefined);
+ if (!foundItem) {
+ throw new Error('No suitable specification version found for reference > specification');
+ }
+ allChildren[key].item.href = foundItem.slug;📝 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.
| if (rootKey === 'reference' && key === 'specification') { | |
| allChildren[key].item.href = childrenOfAllChildren.find((c) => c.isPrerelease === undefined)!.slug; | |
| } | |
| if (rootKey === 'reference' && key === 'specification') { | |
| const foundItem = childrenOfAllChildren.find((c) => c.isPrerelease === undefined); | |
| if (!foundItem) { | |
| throw new Error('No suitable specification version found for reference > specification'); | |
| } | |
| allChildren[key].item.href = foundItem.slug; | |
| } |
| if (!structuredPosts[index + 1].isRootElement && !structuredPosts[index + 1].isSection) { | ||
| nextPage = { | ||
| title: structuredPosts[index + 1].title, | ||
| href: structuredPosts[index + 1].slug | ||
| }; | ||
| } else { | ||
| nextPage = { | ||
| title: `${structuredPosts[index + 1].title} - ${structuredPosts[index + 2].title}`, | ||
| href: structuredPosts[index + 2].slug | ||
| }; | ||
| } | ||
|
|
There was a problem hiding this comment.
Use consistent naming for root detection.
The code references isRootElement in conditions, but the interface does not define this property (only isRootSection). This mismatch may introduce confusion or bugs. Replace isRootElement with isRootSection (or vice versa) for consistency.
- if (!structuredPosts[index + 1].isRootElement && !structuredPosts[index + 1].isSection) {
+ if (!structuredPosts[index + 1].isRootSection && !structuredPosts[index + 1].isSection) {📝 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.
| if (!structuredPosts[index + 1].isRootElement && !structuredPosts[index + 1].isSection) { | |
| nextPage = { | |
| title: structuredPosts[index + 1].title, | |
| href: structuredPosts[index + 1].slug | |
| }; | |
| } else { | |
| nextPage = { | |
| title: `${structuredPosts[index + 1].title} - ${structuredPosts[index + 2].title}`, | |
| href: structuredPosts[index + 2].slug | |
| }; | |
| } | |
| if (!structuredPosts[index + 1].isRootSection && !structuredPosts[index + 1].isSection) { | |
| nextPage = { | |
| title: structuredPosts[index + 1].title, | |
| href: structuredPosts[index + 1].slug | |
| }; | |
| } else { | |
| nextPage = { | |
| title: `${structuredPosts[index + 1].title} - ${structuredPosts[index + 2].title}`, | |
| href: structuredPosts[index + 2].slug | |
| }; | |
| } |
| const fileBaseName = basename(data.slug); // ex. v2.0.0 | v2.1.0-next-spec.1 | ||
| const fileNameOfBaseName = fileBaseName.split('-')[0]; // v2.0.0 | v2.1.0 | ||
|
|
There was a problem hiding this comment.
Verify usage of basename with potential undefined slug.
data.slug might be undefined in some edge cases. Use a fallback or guard clause to avoid unexpected errors when calling basename.
- const fileBaseName = basename(data.slug);
+ if (!data.slug) {
+ throw new Error('Missing slug property for file in specification folder.');
+ }
+ const fileBaseName = basename(data.slug);📝 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.
| const fileBaseName = basename(data.slug); // ex. v2.0.0 | v2.1.0-next-spec.1 | |
| const fileNameOfBaseName = fileBaseName.split('-')[0]; // v2.0.0 | v2.1.0 | |
| if (!data.slug) { | |
| throw new Error('Missing slug property for file in specification folder.'); | |
| } | |
| const fileBaseName = basename(data.slug); // ex. v2.0.0 | v2.1.0-next-spec.1 | |
| const fileNameOfBaseName = fileBaseName.split('-')[0]; // v2.0.0 | v2.1.0 | |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
scripts/build-rss.ts (1)
13-24: 🛠️ Refactor suggestionConsider using a robust HTML sanitization library.
The current manual string replacements could miss edge cases:
- Nested HTML entities
- Malformed input
- Less common HTML entities
Consider using a dedicated HTML sanitization library like
html-entities:-function clean(s: string) { - let cleanS = s; - - cleanS = cleanS.split('<span>').join(''); - cleanS = cleanS.split('&').join('&'); - cleanS = cleanS.split(''').join("'"); - cleanS = cleanS.split('<').join('<'); - cleanS = cleanS.split('>').join('>'); - cleanS = cleanS.split('"').join('"'); - - return cleanS; -} +import { decode } from 'html-entities'; + +function clean(s: string) { + return decode(s); +}
🧹 Nitpick comments (2)
scripts/build-rss.ts (2)
48-51: Avoid type assertions with empty objects.Using type assertions with empty objects can lead to runtime errors.
Consider initializing with required properties:
-const feed = {} as { rss: RSS }; -const rss = {} as RSS; +const feed = { rss: null as unknown as RSS }; +const rss: RSS = { + '@version': '2.0', + '@xmlns:atom': 'http://www.w3.org/2005/Atom', + channel: null as unknown as RSS['channel'] +};
121-124: Improve error handling specificity.The current error handling catches all errors and wraps them in a generic message.
Consider handling specific error types:
- } catch (err) { - assert(err instanceof Error); - throw new Error(`Failed to generate RSS feed: ${err.message}`); + } catch (err) { + if (err instanceof TypeError) { + throw new Error(`Invalid data structure: ${err.message}`); + } else if (err instanceof SyntaxError) { + throw new Error(`Invalid JSON format: ${err.message}`); + } else { + assert(err instanceof Error); + throw new Error(`Failed to generate RSS feed: ${err.message}`); + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/build-rss.ts(1 hunks)types/scripts/build-rss.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- types/scripts/build-rss.ts
🧰 Additional context used
📓 Learnings (1)
scripts/build-rss.ts (1)
Learnt from: akshatnema
PR: asyncapi/website#3101
File: scripts/build-rss.js:4-6
Timestamp: 2024-11-12T06:59:33.852Z
Learning: Converting `getAllPosts()` to asynchronous in `scripts/build-rss.js` causes the existing build system to fail.
🔇 Additional comments (1)
scripts/build-rss.ts (1)
1-5: LGTM! Clean import organization.
The imports are well-organized, separating runtime dependencies from type imports.
| async function getAllPosts() { | ||
| const posts = (await import('../config/posts.json', { assert: { type: 'json' } })).default; | ||
|
|
||
| return posts; | ||
| } |
There was a problem hiding this comment.
Potential build system incompatibility.
Based on previous learnings from PR #3101, making getAllPosts() asynchronous can cause build system failures. Additionally, directly importing posts at build time may not be the best approach.
Consider refactoring to:
- Make the function synchronous
- Accept posts as a parameter instead of importing directly
-async function getAllPosts() {
- const posts = (await import('../config/posts.json', { assert: { type: 'json' } })).default;
-
- return posts;
-}
+function getAllPosts(posts: any) {
+ return posts;
+}📝 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.
| async function getAllPosts() { | |
| const posts = (await import('../config/posts.json', { assert: { type: 'json' } })).default; | |
| return posts; | |
| } | |
| function getAllPosts(posts: any) { | |
| return posts; | |
| } |
|
|
||
| item.enclosure = { | ||
| '@url': base + post.cover, | ||
| '@length': 15026, // dummy value, anything works |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace dummy enclosure length.
Using a dummy value for the enclosure length could cause RSS reader compatibility issues.
Consider calculating the actual file size:
-'@length': 15026, // dummy value, anything works
+// Add this helper function
+async function getImageSize(url: string): Promise<number> {
+ try {
+ const response = await fetch(url);
+ const contentLength = response.headers.get('content-length');
+ return contentLength ? parseInt(contentLength, 10) : 0;
+ } catch (error) {
+ console.warn(`Failed to get image size for ${url}:`, error);
+ return 0;
+ }
+}
// Update the enclosure assignment
-item.enclosure = {
- '@url': base + post.cover,
- '@length': 15026,
- '@type': mimeType
-};
+const imageUrl = base + post.cover;
+const imageSize = await getImageSize(imageUrl);
+item.enclosure = {
+ '@url': imageUrl,
+ '@length': imageSize,
+ '@type': mimeType
+};Committable suggestion skipped: line range outside the PR's diff.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Description
Related issue(s)
Related to #3187
Summary by CodeRabbit
New Features
next-i18next.Bug Fixes
Refactor
Chores