-
-
Notifications
You must be signed in to change notification settings - Fork 117
Performance improvement for helpers #2001
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
|
Warning Rate limit exceeded@stijnvanhulle has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 27 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)
WalkthroughRefactors PluginManager mock signatures and matchFiles (deduplication, parsing/formatting), makes format async with early returns, resolves timeouts with handles and clears them, parallelizes plugin-zod generation and consolidates emissions, adds many generated Zod schemas, and updates a minor ignore pattern. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
More templates
@kubb/cli
@kubb/core
kubb
@kubb/mcp
@kubb/oas
@kubb/plugin-client
@kubb/plugin-cypress
@kubb/plugin-faker
@kubb/plugin-mcp
@kubb/plugin-msw
@kubb/plugin-oas
@kubb/plugin-react-query
@kubb/plugin-redoc
@kubb/plugin-solid-query
@kubb/plugin-svelte-query
@kubb/plugin-swr
@kubb/plugin-ts
@kubb/plugin-vue-query
@kubb/plugin-zod
unplugin-kubb
commit: |
|
The preview deployment for kubb-labs/kubb:main is ready. 🟢 Open Preview | Open Build Logs Last updated at: 2025-11-20 22:25:24 CET |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
examples/zod/kubb.config.js (1)
8-14: Consider using the timeout utility instead of duplicating the logic.This implements the same timeout pattern as the utility in
packages/core/src/utils/timeout.ts. Consider importing and using that utility to reduce code duplication.Apply this diff:
import { defineConfig } from '@kubb/core' +import { timeout } from '@kubb/core' import { pluginClient } from '@kubb/plugin-client' import { pluginOas, schemaKeywords } from '@kubb/plugin-oas' import { pluginTs } from '@kubb/plugin-ts' import { pluginZod } from '@kubb/plugin-zod' export default defineConfig(async () => { - await new Promise((resolve) => { - const timeout = setTimeout(() => { - resolve(timeout) - }, 1000) - }).then((timeout) => { - clearTimeout(timeout) - }) + await timeout(1000) return {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
examples/simple-single/src/gen/zod.tsis excluded by!**/gen/**
📒 Files selected for processing (5)
configs/mocks.ts(3 hunks)examples/simple-single/src/gen2/index.ts(6 hunks)examples/zod/kubb.config.js(1 hunks)packages/core/src/utils/timeout.ts(1 hunks)packages/plugin-zod/src/plugin.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/plugin-zod/src/plugin.ts (1)
packages/core/src/utils/getBarrelFiles.ts (1)
getBarrelFiles(36-76)
examples/zod/kubb.config.js (1)
packages/core/src/utils/timeout.ts (1)
timeout(1-9)
configs/mocks.ts (1)
packages/core/src/PluginManager.ts (1)
PluginManager(78-708)
🪛 Biome (2.1.2)
examples/simple-single/src/gen2/index.ts
[error] 71-71: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 83-83: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 133-133: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 140-140: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 145-145: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 172-172: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 183-183: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 188-188: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 294-294: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 306-306: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 311-311: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 316-316: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 364-364: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 376-376: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 381-381: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 386-386: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 398-398: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 416-416: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 449-449: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 454-454: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 630-631: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 662-662: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 667-668: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 691-692: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 708-709: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 713-714: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 742-743: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 747-748: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 898-898: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 923-924: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 941-942: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 959-960: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 971-972: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 976-977: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 981-982: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 993-994: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1011-1012: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1016-1017: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1021-1022: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1039-1040: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1044-1045: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1049-1050: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1061-1062: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1066-1067: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1071-1072: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1089-1090: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1094-1095: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1099-1100: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1250-1252: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1255-1257: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1289-1291: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1325-1327: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1330-1332: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1335-1337: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1347-1349: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1364-1366: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1375-1377: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1380-1382: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1392-1394: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1404-1406: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1409-1411: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1414-1416: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1486-1488: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1498-1500: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1503-1505: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1515-1517: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1534-1536: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1539-1541: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1557-1559: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1576-1578: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1593-1595: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1598-1600: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
🔇 Additional comments (8)
packages/plugin-zod/src/plugin.ts (2)
160-160: LGTM: Clean single-line formatting.The meta object formatting is more concise and has no functional impact.
154-154: ****The original review comment incorrectly identifies a race condition. Analysis shows that
OperationGeneratordoes not accessusedEnumNamesat all—onlySchemaGeneratormutates this state during its ownbuild()method (line 729 of SchemaGenerator.ts). SinceOperationGeneratoroperates independently and never touches this shared state, there is no concurrent access to the mutable object during parallelization. The two generators process different concerns (schemas vs. operations) and can safely run in parallel without conflict on this state.Likely an incorrect or invalid review comment.
packages/core/src/utils/timeout.ts (1)
3-7: LGTM! Cross-environment compatibility improvement.The use of
globalThis.setTimeoutandglobalThis.clearTimeoutensures compatibility across Node.js, browser, and other JavaScript environments. The timeout is properly cleaned up after resolution.configs/mocks.ts (5)
22-28: LGTM! Improved error handling.The early return for empty source and catch-without-binding pattern (when the error object isn't needed) are both appropriate improvements.
34-42: LGTM! Cleaner control flow.The switch statement is more readable than the previous if-else chain and maintains the same logic.
57-57: Mock signature simplified by removing unused parameter.The parameter was removed since this mock always returns
undefinedregardless of input. This aligns with the removal of thePluginimport.
71-97: LGTM! Improved performance with deduplication.The addition of the
processedMap prevents redundant file processing and caching results. The early returns and clearer control flow are good improvements.
67-67: The review comment is incorrect. This double assertion is not indicative of mock drift.The
as unknown as PluginManagerpattern is a standard TypeScript escape hatch used intentionally here because the mock is a partial implementation—it provides only the subset of PluginManager methods needed for specific use cases. This is appropriate design for a test configuration file.Additionally, the mock (
mockedPluginManagerandcreateMockedPluginManager) has zero usages across the codebase. Grep searches found no imports or calls, and the entireconfigs/directory is excluded from test coverage. The double assertion has existed through recent PluginManager refactoring (commit 29a43be), unchanged. This is dead or transitional code in a configuration context, not an active mock with drift concerns.Likely an incorrect or invalid review comment.
2ccfba5 to
897433f
Compare
# Conflicts: # examples/simple-single/src/gen2/index.ts
Summary by CodeRabbit
New Features
Performance
Improvements