-
Notifications
You must be signed in to change notification settings - Fork 11
chore: fix apibara cli to work with .mjs file extensions and to show debug logs when debug is true #183
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
chore: fix apibara cli to work with .mjs file extensions and to show debug logs when debug is true #183
Conversation
📝 WalkthroughWalkthroughThis update introduces debug logging to the indexer, controlled by a debug flag, and extends support for Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant ConfigUtils
participant Indexer
participant Logger
User->>CLI: Run command with .mjs config/indexer
CLI->>ConfigUtils: Detect config/indexer file extension
ConfigUtils-->>CLI: Return detected extension (.js, .ts, or .mjs)
CLI->>Indexer: Start indexer with options (including debug flag)
Indexer->>Logger: Initialize logger plugin
Indexer->>Indexer: Set context.debug if debug enabled
Logger->>Logger: Set log level to debug if context.debug is true
Logger-->>Indexer: Logger ready with correct level
Indexer-->>User: Indexer runs, outputs debug logs if enabled
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
packages/cli/src/create/utils.ts (2)
40-43:⚠️ Potential issueUpdate error message to include "mjs" option
The error message should reflect that "mjs" is now a valid language option, matching the validation logic above.
- `Invalid language ${cyan("(--language | -l)")}: ${red(language)}. Options: ${blue("typescript, ts")} or ${yellow("javascript, js")} | default: ${cyan("typescript")}`, + `Invalid language ${cyan("(--language | -l)")}: ${red(language)}. Options: ${blue("typescript, ts")} or ${yellow("javascript, js, mjs")} | default: ${cyan("typescript")}`,
57-59:⚠️ Potential issueError message needs to include "mjs" option
Similar to the previous comment, this error message should mention "mjs" as a valid option.
- `Invalid language ${cyan("(--language | -l)")}: ${red(alias)}. Options: ${blue("typescript, ts")} or ${yellow("javascript, js")}`, + `Invalid language ${cyan("(--language | -l)")}: ${red(alias)}. Options: ${blue("typescript, ts")} or ${yellow("javascript, js, mjs")}`,
🧹 Nitpick comments (1)
packages/cli/src/rolldown/plugins/instrumentation.ts (1)
10-10: Extension detection logic is correct, consider extracting for readability.The nested ternary correctly handles
.ts,.mjs, and.jsextensions. However, for improved readability and maintainability, consider extracting this logic.Consider extracting the extension detection logic:
+function getFileExtension(configFile?: string): string { + if (configFile?.endsWith(".ts")) return "ts"; + if (configFile?.endsWith(".mjs")) return "mjs"; + return "js"; +} + export function instrumentation(apibara: Apibara) { const instrumentationPath = join( apibara.options._c12.cwd!, - `instrumentation.${apibara.options._c12.configFile?.endsWith(".ts") ? "ts" : apibara.options._c12.configFile?.endsWith(".mjs") ? "mjs" : "js"}`, + `instrumentation.${getFileExtension(apibara.options._c12.configFile)}`, );This makes the code more readable and easier to extend if additional extensions need to be supported in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
change/@apibara-indexer-d223a0d2-d422-40bd-a7b0-939c3b04ead1.json(1 hunks)change/apibara-b87e2be4-ca28-4711-b029-a215cfd726c8.json(1 hunks)packages/cli/src/core/build/dev.ts(1 hunks)packages/cli/src/core/scan.ts(1 hunks)packages/cli/src/create/add.ts(3 hunks)packages/cli/src/create/templates.ts(5 hunks)packages/cli/src/create/types.ts(2 hunks)packages/cli/src/create/utils.ts(4 hunks)packages/cli/src/rolldown/plugins/instrumentation.ts(1 hunks)packages/indexer/src/indexer.ts(1 hunks)packages/indexer/src/plugins/logger.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (15)
change/@apibara-indexer-d223a0d2-d422-40bd-a7b0-939c3b04ead1.json (1)
1-7: LGTM! Changelog entry accurately reflects debug logging enhancement.The changelog entry correctly documents the addition of debug logging functionality when debug mode is enabled, which aligns with the PR objectives.
packages/indexer/src/indexer.ts (1)
229-231: LGTM! Debug flag implementation is correct.The conditional check properly sets the debug flag on the context when the indexer's debug option is enabled. The placement after
useIndexerContext()and before middleware registration is appropriate for enabling debug behavior throughout the indexer lifecycle.packages/cli/src/create/types.ts (2)
19-19: LGTM! FileExtension type properly includes .mjs support.The new
FileExtensiontype correctly includes all three supported extensions ("js", "ts", "mjs"), which aligns with the PR objective to add .mjs file support.
31-31:✅ Verification successful
Verify backward compatibility of the extension property.
The addition of the
extensionproperty toIndexerOptionslooks good. However, ensure this property is handled appropriately in existing code that may not provide it.
🏁 Script executed:
#!/bin/bash # Description: Check how IndexerOptions is used throughout the codebase to ensure backward compatibility. # Expected: Find usages and verify they handle the new extension property. rg -A 3 -B 3 "IndexerOptions" --type tsLength of output: 4433
🏁 Script executed:
#!/bin/bash # Display contents of types.ts to verify IndexerOptions definition sed -n '1,200p' packages/cli/src/create/types.tsLength of output: 882
🏁 Script executed:
#!/bin/bash # Display the create/add.ts file to inspect how IndexerOptions and extension are handled sed -n '1,200p' packages/cli/src/create/add.tsLength of output: 5378
🏁 Script executed:
#!/bin/bash # Display lines 200-300 around the IndexerOptions assignment in create/add.ts sed -n '200,300p' packages/cli/src/create/add.tsLength of output: 1061
🏁 Script executed:
#!/bin/bash # Search for updateApibaraConfigFile definition to inspect parameter usage rg -n "export async function updateApibaraConfigFile" -n packages/cli/src/create/templates.tsLength of output: 147
🏁 Script executed:
#!/bin/bash # Locate definition of getApibaraConfigLanguage rg -n "getApibaraConfigLanguage" -n packages/cli/src/create/utils.tsLength of output: 129
🏁 Script executed:
#!/bin/bash # Inspect getApibaraConfigLanguage in utils.ts sed -n '190,260p' packages/cli/src/create/utils.ts echo # Inspect updateApibaraConfigFile in templates.ts sed -n '200,260p' packages/cli/src/create/templates.tsLength of output: 3543
Backward compatibility of
extensionproperty confirmedThe new
extensionfield is always derived fromgetApibaraConfigLanguage(cwd)inadd.tsand correctly propagated to every code path that consumesIndexerOptions. Specifically:
packages/cli/src/create/add.ts:
•const { language, extension } = getApibaraConfigLanguage(cwd);
• Passed intoupdateApibaraConfigFile,updatePackageJson,createIndexerFile, andcreateStorageRelatedFilespackages/cli/src/create/utils.ts:
•getApibaraConfigLanguagereliably returns bothlanguageandextensionbased on existing config filepackages/cli/src/create/templates.ts:
•updateApibaraConfigFile,updatePackageJson,createIndexerFile, andcreateStorageRelatedFilesall accept and use theextensionvalueNo code path omits handling of
extension.change/apibara-b87e2be4-ca28-4711-b029-a215cfd726c8.json (1)
1-7: LGTM! Changelog entry accurately documents .mjs extension fix.The changelog entry correctly describes the CLI fix for .mjs extension support, which directly addresses one of the key PR objectives.
packages/indexer/src/plugins/logger.ts (1)
24-27: LGTM! Debug logging implementation is correct.The conditional check properly enables debug level logging when the debug flag is set in the context. This implementation correctly applies the debug level after logger creation, regardless of whether a custom reporter is provided.
packages/cli/src/core/build/dev.ts (1)
106-106: LGTM! Correctly extends ignore pattern for .mjs configuration files.The glob pattern properly includes
.mjsextension alongside.tsand.js, ensuring thatapibara.config.mjsfiles are ignored by the watcher as intended.packages/cli/src/create/utils.ts (2)
185-195: LGTM! Correctly extends config detection for .mjs filesThe function now properly checks for all three config file variants.
197-216: Well-structured refactor to support file extensionsThe function now returns both language and extension, enabling proper handling of .mjs files throughout the codebase. The check order (mjs → js → ts) ensures newer module formats take precedence.
packages/cli/src/create/add.ts (2)
80-80: Good refactor to support dynamic file extensionsProperly destructures both
languageandextensionfrom the updatedgetApibaraConfigLanguagefunction.
98-104: Consistent use of dynamic extension throughoutAll file paths and success messages now correctly use the
extensionvariable, ensuring proper support for .js, .ts, and .mjs files.Also applies to: 208-209, 213-214, 221-222
packages/cli/src/core/scan.ts (1)
5-5: Correctly extends indexer scanning for .mjs filesThe scanner now recognizes
.indexer.mjsfiles alongside.indexer.tsand.indexer.js.packages/cli/src/create/templates.ts (3)
146-146: Good change for extension flexibility.The modification correctly uses the dynamic extension from options, enabling support for .mjs files as intended.
217-219: Properly updated to support dynamic extensions.The function signature and path construction correctly incorporate the extension parameter, maintaining consistency with the extension-agnostic approach.
268-274: Comprehensive extension handling for Drizzle storage files.All file references have been properly updated to use the dynamic
fileExtension. The implementation correctly handles schema files, drizzle config files, and user-facing messages.Also applies to: 286-286, 300-300, 354-354, 362-362, 371-371
|
Looks good, thank you! |
No description provided.