refactor: migrate build script to typescript #3423
Conversation
WalkthroughThis pull request introduces extensive changes across configuration files, scripts, utilities, and tests. Major updates include ESLint and Next-i18next configuration modifications, a shift from CommonJS to ES module syntax, and migration of many JavaScript files to TypeScript with richer type annotations and error handling. Multiple build scripts (for docs, meetings, newsroom videos, posts, tools, case studies, etc.) have been refactored or replaced. Additionally, new logging via Winston has been integrated, types and declaration files have been added or updated, and tests have been revised to use the new TypeScript modules and logger mocks. Changes
Sequence Diagram(s)sequenceDiagram
participant S as buildMeetings
participant G as GoogleAuth
participant API as Google Calendar API
participant FS as File System
S->>G: Initiate authentication
G-->>S: Return authentication token
S->>API: Request calendar events
API-->>S: Return events data
S->>S: Validate response data
S->>FS: Write events data to file
S-->>Caller: Complete process successfully
sequenceDiagram
participant V as buildNewsroomVideos
participant Fetch as Fetch API
participant Logger as Logger Utility
participant FS as File System
V->>Fetch: Verify YOUTUBE_TOKEN exists
V->>Fetch: Request video data from YouTube API
Fetch-->>V: Return API response
V->>V: Validate and process video items
V->>Logger: Log video data for debugging
V->>FS: Write processed video data to JSON file
V-->>Caller: Return processed results
Possibly related PRs
Suggested labels
Suggested reviewers
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
|
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
asyncapi-bot
requested review from
devilkiller-ag,
sambhavgupta0705,
BhaswatiRoy,
J0SAL,
TRohit20 and
VaishnaviNandakumar
JeelRajodiya
changed the title
scripts/casestudies/index.ts
Outdated
| const caseStudiesList = await Promise.all( | ||
| files.map(async (file) => { | ||
| const caseStudyFileName = [dirWithCaseStudy, file].join('/'); | ||
| const caseStudyContent = await readFile(caseStudyFileName, 'utf-8'); | ||
|
|
||
| return convertToJson(caseStudyContent); | ||
| }) | ||
| ); |
There was a problem hiding this comment.
This part refactored to avoid using await inside a loop (eslint error)
https://eslint.org/docs/latest/rules/no-await-in-loop
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/validate-case-studies-structures.yaml (2)
4-4: Fix trailing spaces.Remove trailing spaces after
push:to maintain consistent code style.- push: + push:🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 4-4: trailing spaces
(trailing-spaces)
4-4: Consider adding path filters to the push trigger.The current configuration will run on all pushes, which could be inefficient. Consider adding path filters to only run when relevant files are changed:
push: paths: - 'config/casestudies/*.yml'🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 4-4: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/validate-case-studies-structures.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/validate-case-studies-structures.yaml
[error] 4-4: trailing spaces
(trailing-spaces)
…uest" This reverts commit cc194a6.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/build-docs.ts (1)
115-117: Improve error handling for better type safety.The error handling could be more type-safe and informative.
- throw new Error(`Failed to build navigation tree: ${err}`); + throw new Error(`Failed to build navigation tree: ${err instanceof Error ? err.message : String(err)}`);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/build-docs.ts(1 hunks)
🔇 Additional comments (5)
scripts/build-docs.ts (5)
1-7: LGTM! Clean imports with good practices.The imports are well-organized, using named imports for better tree-shaking, and properly importing TypeScript types.
149-150: Fix error message construction and improve type safety.The error message construction is incorrect and the type assertion could be handled better.
244-245: Improve error message construction in addDocButtons.Similar to the previous error handling issue, the error message construction could be improved.
250-251: LGTM! Clean exports.The named exports are well-organized and follow TypeScript best practices.
201-215:⚠️ Potential issueAdd bounds checking for array access in navigation logic.
When constructing the next page title, ensure index + 2 is within bounds.
if (index + 1 < countDocPages) { if (!structuredPosts[index + 1].isRootElement && !structuredPosts[index + 1].isSection) { nextPage = { title: structuredPosts[index + 1].title, href: structuredPosts[index + 1].slug }; } else { + if (index + 2 < countDocPages) { nextPage = { title: `${structuredPosts[index + 1].title} - ${structuredPosts[index + 2].title}`, href: structuredPosts[index + 2].slug }; + } }Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
scripts/tools/combine-tools.ts (3)
67-112: 🛠️ Refactor suggestionExtract shared tag processing logic.
The language and technology processing logic is still duplicated. Consider extracting this into a shared function.
type TagType = 'language' | 'technology'; interface TagConfig { defaultColor: string; defaultBorderColor: string; fuseInstance: Fuse<LanguageColorItem>; tagList: LanguageColorItem[]; } async function processTag( tag: string, config: TagConfig ): Promise<LanguageColorItem> { const search = await config.fuseInstance.search(tag); if (search.length > 0) { return search[0].item; } const tagObject = { name: tag, color: config.defaultColor, borderColor: config.defaultBorderColor }; config.tagList.push(tagObject); config.fuseInstance = new Fuse(config.tagList, options); return tagObject; }Also applies to: 113-138
195-197:⚠️ Potential issueRemove non-null assertions from sort comparison.
The sort comparison still uses non-null assertions which could lead to runtime errors.
- finalTools[key].toolsList = [...automatedResults, ...manualResults].sort((tool, anotherTool) => - tool!.title.localeCompare(anotherTool!.title) - ) as FinalAsyncAPITool[]; + finalTools[key].toolsList = [...automatedResults, ...manualResults].sort((tool, anotherTool) => { + if (!tool?.title || !anotherTool?.title) return 0; + return tool.title.localeCompare(anotherTool.title); + }) as FinalAsyncAPITool[];
200-204: 🛠️ Refactor suggestionEnhance error handling for file operations.
The file operations error handling could be more informative.
- fs.writeFileSync(toolsPath, JSON.stringify(finalTools)); - fs.writeFileSync(tagsPath, JSON.stringify({ languages: languageList, technologies: technologyList })); + try { + fs.writeFileSync(toolsPath, JSON.stringify(finalTools, null, 2)); + fs.writeFileSync(tagsPath, JSON.stringify({ + languages: languageList, + technologies: technologyList + }, null, 2)); + } catch (error) { + throw new Error( + `Failed to write output files:\n` + + `Tools path: ${toolsPath}\n` + + `Tags path: ${tagsPath}\n` + + `Error: ${error.message}` + ); + }scripts/tools/extract-tools-github.ts (1)
56-58:⚠️ Potential issueFix incorrect merging of items.
Line 56 incorrectly pushes all items into result.data.items, which already includes the first page of items, causing duplication.
- result.data.items.push(...allItems); + result.data.items = allItems;
🧹 Nitpick comments (4)
scripts/tools/combine-tools.ts (2)
20-23: Add error handling for schema validation setup.The schema validation setup could fail silently. Consider adding error handling:
-const ajv = new Ajv(); -addFormats(ajv, ['uri']); -const validate = ajv.compile(schema); +const ajv = new Ajv(); +try { + addFormats(ajv, ['uri']); + const validate = ajv.compile(schema); +} catch (error) { + throw new Error(`Failed to initialize schema validation: ${error.message}`); +}
35-40: Consider making Fuse configuration values configurable.The threshold value of 0.39 is hardcoded. Consider making it configurable for better flexibility:
+const FUSE_THRESHOLD = process.env.FUSE_THRESHOLD ? parseFloat(process.env.FUSE_THRESHOLD) : 0.39; + const options = { includeScore: true, shouldSort: true, - threshold: 0.39, + threshold: FUSE_THRESHOLD, keys: ['name', 'color', 'borderColor'] };scripts/tools/extract-tools-github.ts (2)
12-18: Add usage example to JSDoc.Consider adding an @example section to make the usage clearer for other developers.
/** * Fetches tool data from the GitHub API. * * @returns {Promise<ToolsData>} The data from the GitHub API. * @throws {Error} If there is an error fetching the data. + * @example + * const toolsData = await getData(); + * console.log(`Found ${toolsData.total_count} tools`); */
19-21: Enhance error message with setup instructions.The error message could be more helpful by including setup instructions.
- throw new Error('GITHUB_TOKEN environment variable is required'); + throw new Error( + 'GITHUB_TOKEN environment variable is required. Please set it in your .env file or environment variables. ' + + 'You can create a token at https://github.com/settings/tokens with `public_repo` scope.' + );🧰 Tools
🪛 GitHub Actions: PR testing - if Node project
[error] 20-20: GITHUB_TOKEN environment variable is required
[error] 20-20: GITHUB_TOKEN environment variable is required
[error] 20-20: GITHUB_TOKEN environment variable is required
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/tools/combine-tools.ts(1 hunks)scripts/tools/extract-tools-github.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
scripts/tools/extract-tools-github.ts
[error] 60-60: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
🪛 GitHub Actions: PR testing - if Node project
scripts/tools/extract-tools-github.ts
[error] 20-20: GITHUB_TOKEN environment variable is required
[error] 20-20: GITHUB_TOKEN environment variable is required
[error] 20-20: GITHUB_TOKEN environment variable is required
🔇 Additional comments (4)
scripts/tools/combine-tools.ts (2)
68-68: Add optional chaining for filters access.This is still vulnerable to undefined filters.
-if (toolObject.filters.language) { +if (toolObject.filters?.language) {
142-166: LGTM! Well-structured error handling.The function has proper error handling, type safety, and clear logging of validation failures.
scripts/tools/extract-tools-github.ts (2)
1-10: LGTM! Imports and type declarations are well organized.The ESLint disable for no-await-in-loop is justified here since we need sequential pagination to avoid rate limits.
37-55: LGTM! Pagination handling is well implemented.Good practices implemented:
- Proper pagination based on total count
- Rate limiting protection with 1-second pause
- Progress logging
…asyncapi-site into script-migration-2
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
scripts/tools/extract-tools-github.ts (1)
12-17: Enhance JSDoc documentation.Consider adding
@requirestag to document the required environment variables./** * Fetches tool data from the GitHub API. * + * @requires {string} process.env.GITHUB_TOKEN - GitHub API token for authentication * @returns {Promise<ToolsData>} The data from the GitHub API. * @throws {Error} If there is an error fetching the data. */
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/tools/extract-tools-github.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
scripts/tools/extract-tools-github.ts
[error] 57-57: 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)
⏰ Context from checks skipped due to timeout of 180000ms (5)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Lighthouse CI
🔇 Additional comments (4)
scripts/tools/extract-tools-github.ts (4)
1-10: LGTM! Well-organized imports and configuration.The imports are properly organized and typed. The ESLint disable comment for
no-await-in-loopis justified as we need to wait between API calls to avoid rate limiting.
18-18: Add environment variable validation.Add validation for the required GITHUB_TOKEN environment variable at the start of the function.
export async function getData(): Promise<ToolsData> { + if (!process.env.GITHUB_TOKEN) { + throw new Error('GITHUB_TOKEN environment variable is required'); + }
19-58: Improve error handling.The current try-catch block only rethrows errors without adding value. Let's enhance it with proper error handling.
- // eslint-disable-next-line no-useless-catch - try { + try { const allItems = []; let page = 1; // ... rest of the code ... } catch (error) { - throw err; + if (axios.isAxiosError(error)) { + if (error.response?.status === 403) { + throw new Error('GitHub API rate limit exceeded. Please try again later.'); + } + if (error.response?.status === 401) { + throw new Error('Invalid GitHub token. Please check your GITHUB_TOKEN.'); + } + throw new Error(`GitHub API error: ${error.response?.data?.message || error.message}`); + } + throw error; }🧰 Tools
🪛 Biome (1.9.4)
[error] 57-57: 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)
53-53: Fix incorrect merging of items.There's a bug in how items are merged. The line
result.data.items.push(...allItems)is incorrect asallItemsalready contains all items including those from the first page.-result.data.items.push(...allItems); +result.data.items = allItems;
|
|
||
| /** | ||
| * Builds the adopters list by converting a YAML file to JSON and writing it to a specified path. | ||
| * @returns {Promise<void>} |
There was a problem hiding this comment.
| * @returns {Promise<void>} |
| /* istanbul ignore next */ | ||
|
|
There was a problem hiding this comment.
| /* istanbul ignore next */ |
|
|
||
| // Skip the folder 'docs/reference/specification' | ||
|
|
||
| /* istanbul ignore next */ |
There was a problem hiding this comment.
Any reason why we are adding these comments at multiple places?
| import axios from 'axios'; | ||
| import dotenv from 'dotenv'; | ||
|
|
There was a problem hiding this comment.
@JeelRajodiya You can mock the this env token, in beforeEach block of the test.
| let auth; | ||
| let calendar; | ||
|
|
||
| try { | ||
| auth = new google.auth.GoogleAuth({ | ||
| scopes: ['https://www.googleapis.com/auth/calendar'], | ||
| credentials: process.env.CALENDAR_SERVICE_ACCOUNT ? JSON.parse(process.env.CALENDAR_SERVICE_ACCOUNT) : undefined, | ||
| credentials: process.env.CALENDAR_SERVICE_ACCOUNT ? JSON.parse(process.env.CALENDAR_SERVICE_ACCOUNT) : undefined |
There was a problem hiding this comment.
@JeelRajodiya Yeah add these checks in the code, and at the same end, update the tests.
| for (const dir of directories) { | ||
| const directory = posix.normalize(dir[0]); | ||
| /* istanbul ignore next */ | ||
| const sectionSlug = dir[1] || ''; | ||
| const files = await readdir(directory); | ||
|
|
||
| for (const file of files) { | ||
| let details: Details; | ||
| const fileName = normalize(join(directory, file)); | ||
| const fileNameWithSection = normalize(join(fileName, '_section.mdx')); | ||
| const slug = `/${normalize(relative(basePath, fileName)).replace(/\\/g, '/')}`; | ||
| const slugElements = slug.split('/'); | ||
|
|
||
| if (await isDirectory(fileName)) { | ||
| if (await pathExists(fileNameWithSection)) { | ||
| // Passing a second argument to frontMatter disables cache. See https://github.com/asyncapi/website/issues/1057 | ||
| details = frontMatter(await readFile(fileNameWithSection, 'utf-8'), {}).data as Details; | ||
| /* istanbul ignore next */ | ||
| details.title = details.title || capitalize(basename(fileName)); | ||
| } else { | ||
| details = { | ||
| title: capitalize(basename(fileName)) | ||
| }; | ||
| } | ||
| details.isSection = true; | ||
| if (slugElements.length > 3) { | ||
| details.parent = slugElements[slugElements.length - 2]; | ||
| details.sectionId = slugElements[slugElements.length - 1]; | ||
| } | ||
| if (!details.parent) { | ||
| details.isRootSection = true; | ||
| details.rootSectionId = slugElements[slugElements.length - 1]; | ||
| } | ||
| details.sectionWeight = sectionWeight; | ||
| details.slug = slug; | ||
| addItem(details); | ||
| const rootId = details.parent || details.rootSectionId; | ||
|
|
||
| await walkDirectories( | ||
| [[fileName, slug]], | ||
| resultObj, | ||
| basePath, | ||
| details.title, | ||
| details.sectionId, | ||
| rootId, | ||
| details.sectionWeight | ||
| ); | ||
| } else if (file.endsWith('.mdx') && !fileName.endsWith(`${sep}_section.mdx`)) { | ||
| const fileContent = await readFile(fileName, 'utf-8'); | ||
| // Passing a second argument to frontMatter disables cache. See https://github.com/asyncapi/website/issues/1057 | ||
| const { data, content } = frontMatter(fileContent, {}); | ||
|
|
||
| details = data as Details; | ||
| details.toc = toc(content, { slugify: slugifyToC }).json; | ||
| details.readingTime = Math.ceil(readingTime(content).minutes); | ||
| details.excerpt = details.excerpt || markdownToTxt(content).substr(0, 200); | ||
| /* istanbul ignore next */ | ||
| details.sectionSlug = sectionSlug || slug.replace(/\.mdx$/, ''); | ||
| details.sectionWeight = sectionWeight; | ||
| details.sectionTitle = sectionTitle; | ||
| details.sectionId = sectionId; | ||
| details.rootSectionId = rootSectionId; | ||
| details.id = fileName.replace(/\\/g, '/'); | ||
| details.isIndex = fileName.endsWith(join('index.mdx')); | ||
| details.slug = details.isIndex ? sectionSlug : slug.replace(/\.mdx$/, ''); | ||
| if (details.slug.includes('/reference/specification/') && !details.title) { | ||
| const fileBaseName = basename(details.slug); | ||
| const versionDetails = getVersionDetails(details.slug, specWeight--); | ||
|
|
||
| details.title = versionDetails.title; | ||
| details.weight = versionDetails.weight; | ||
|
|
||
| if (releaseNotes.includes(details.title)) { | ||
| details.releaseNoteLink = `/blog/release-notes-${details.title}`; | ||
| } | ||
|
|
||
| details = handleSpecificationVersion(details, fileBaseName); | ||
| } | ||
|
|
||
| // To create a list of available ReleaseNotes list, which will be used to add details.releaseNoteLink attribute. | ||
| if (file.startsWith('release-notes') && dir[1] === '/blog') { | ||
| const { name } = parse(file); | ||
| const version = name.split('-').pop(); | ||
|
|
||
| releaseNotes.push(version); | ||
| } | ||
|
|
||
| addItem(details); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
@JeelRajodiya We can add a try/catch statement as it can happen that we use this function somewhere else as well in future.
| async function getAllPosts() { | ||
| const posts = ((await import('../config/posts.json')) as any).default; | ||
|
|
||
| return posts; | ||
| } |
There was a problem hiding this comment.
@JeelRajodiya Why are we using any type here, instead of a Post type, that we are using for Docs or Blog?
| const toolsPath = resolve(currentDirPath, '../config', 'tools.json'); | ||
| const tagsPath = resolve(currentDirPath, '../config', 'all-tags.json'); | ||
|
|
||
| buildTools(automatedToolsPath, manualToolsPath, toolsPath, tagsPath); |
There was a problem hiding this comment.
@JeelRajodiya You should do the error handling here as well. In the future, we will remove the executing of the function to another file.
| for (const month in Expenses) { | ||
| if (Object.prototype.hasOwnProperty.call(Expenses, month)) { | ||
| Expenses[month as keyof typeof Expenses].forEach((entry: { Category: string }) => { | ||
| if (!allCategories.includes(entry.Category)) { | ||
| allCategories.push(entry.Category); | ||
| } | ||
| }); | ||
| } | ||
| return allCategories; | ||
| } |
There was a problem hiding this comment.
| for (const month in Expenses) { | |
| if (Object.prototype.hasOwnProperty.call(Expenses, month)) { | |
| Expenses[month as keyof typeof Expenses].forEach((entry: { Category: string }) => { | |
| if (!allCategories.includes(entry.Category)) { | |
| allCategories.push(entry.Category); | |
| } | |
| }); | |
| } | |
| return allCategories; | |
| } | |
| Object.keys(Expenses).forEach((month) => { | |
| Expenses[month as keyof typeof Expenses].forEach((entry: { Category: string }) => { | |
| if (!allCategories.includes(entry.Category)) { | |
| allCategories.push(entry.Category); | |
| } | |
| }); | |
| }); |
There was a problem hiding this comment.
To remove the eslint ignore check
Description
✅ Deploy Preview for asyncapi-website ready!
Built without sensitive environment variables
Toggle QR Code...
Use your smartphone camera to open QR code link.
To edit notification comments on pull requests, go to your Netlify site configuration.
Related issue(s)
Related to #3187
Summary by CodeRabbit
New Features
Refactor & Chores