cli: integrate runtime config serialization in rolldown to avoid bundling apibara.config #156
Conversation
- Added a new function to serialize runtime configuration for use in plugins. - Updated the appConfig plugin to utilize the serialized configuration and handle undefined cases. - Adjusted logger initialization to prevent bundling issues with custom loggers.
📝 WalkthroughWalkthroughThis pull request introduces a new JSON file containing prerelease metadata for the "apibara" package and adds a new dependency ( Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as CLI Command
participant A as createApibara
participant R as RuntimeConfig Resolver
participant E as Environment (APIBARA_RUNTIME_CONFIG)
participant P as RollDown Plugin
U->>C: Invoke command with preset option
C->>A: Call createApibara(preset)
A->>R: Resolve runtime configuration with preset handling
R-->>A: Return merged runtime config or error
A->>E: Serialize and store runtime config to env variable
P->>E: Retrieve runtime config for plugin replacement
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 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
CodeRabbit Configuration File (
|
![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 (4)
packages/cli/src/rolldown/config.ts (1)
93-109: Well-structured serialization function with proper error handling.The
getSerializedRuntimeConfigfunction correctly serializes only the necessary configuration properties and includes appropriate error handling with a clear error message. This helps ensure that only serializable values are used in the runtime configuration.Two small suggestions:
- Consider adding more specific error information by including the original error in the thrown Error message
- Consider using a more concise JSON.stringify (without formatting) for production builds
function getSerializedRuntimeConfig(apibara: Apibara) { try { return JSON.stringify( { runtimeConfig: apibara.options.runtimeConfig, preset: apibara.options.preset, presets: apibara.options.presets, }, - null, - 2, + null ); } catch (error) { throw new Error( - "Failed to serialize runtime config, please only use serializable values for runtime config", + `Failed to serialize runtime config, please only use serializable values for runtime config: ${error}` ); } }packages/cli/src/runtime/internal/app.ts (1)
65-80: Well-documented temporary disabling of custom logger support.The thorough comments explaining why custom logger support is temporarily disabled are excellent. They clearly articulate how this change relates to the PR objective of preventing unwanted bundling while still providing a roadmap for future improvements.
In the future, you might want to consider alternatives for custom loggers that don't require bundling the entire configuration object, such as:
- A plugin-based approach where loggers are registered separately
- A factory function pattern that accepts the serialized config at runtime
packages/cli/src/core/config/resolvers/runtime-config.resolver.ts (2)
4-5: Validateoptions.runtimeConfigbefore spreading
Ifoptions.runtimeConfigisundefinedor not an object, this spread operation could potentially throw an error. Consider defaulting to an empty object for safer merging.- let runtimeConfig: Record<string, unknown> = { ...options.runtimeConfig }; + let runtimeConfig: Record<string, unknown> = { ...(options.runtimeConfig ?? {}) };
14-16: Check for property existence within
You can improve readability by verifying property existence with theinoperator, which avoids referencing a potentially non-existent index.- if (presets[preset] === undefined) { + if (!(preset in presets)) { throw new Error(`Specified preset "${preset}" but it was not defined`); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
change/apibara-3c1e3504-7ac7-41b7-bb7f-02c09deb8cf9.json(1 hunks)packages/cli/package.json(1 hunks)packages/cli/src/cli/commands/dev.ts(1 hunks)packages/cli/src/cli/commands/start.ts(1 hunks)packages/cli/src/core/config/resolvers/runtime-config.resolver.ts(1 hunks)packages/cli/src/rolldown/config.ts(2 hunks)packages/cli/src/rolldown/plugins/config.ts(1 hunks)packages/cli/src/runtime/internal/app.ts(1 hunks)packages/cli/src/types/virtual/config.d.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (15)
packages/cli/package.json (1)
94-94: New dependency added for runtime config serialization.The
@rollup/plugin-replacedependency has been added to support the runtime configuration serialization in rolldown, which aligns with the PR objective of avoiding bundling apibara.config.change/apibara-3c1e3504-7ac7-41b7-bb7f-02c09deb8cf9.json (1)
1-7: Change file looks good for documenting the feature.This change file correctly documents the feature addition and marks it as a prerelease, which is appropriate for integrating runtime configuration serialization into rolldown.
packages/cli/src/cli/commands/dev.ts (2)
59-59: Preset integration looks good.The change adds support for the preset parameter to be passed to the createApibara function, which aligns with the PR objective of integrating runtime config serialization.
114-114: Verify preset propagation through CLI.The preset argument is correctly passed to the child process when starting indexers. This ensures that the preset configuration is consistently applied throughout the application lifecycle.
packages/cli/src/cli/commands/start.ts (4)
20-23: Added preset CLI argument.The preset argument is properly defined with appropriate type and description, allowing users to specify a preset configuration when starting an indexer.
31-31: Preset integration in createApibara.The preset parameter is correctly passed to the createApibara function, which enables runtime configuration based on the specified preset.
35-35: Log message includes preset information.Good addition of the preset information in the log message, which improves visibility of which preset is being used when starting an indexer.
55-55: Preset propagation to child process.The preset argument is properly passed to the child process, ensuring that the preset configuration is maintained throughout the application lifecycle.
packages/cli/src/types/virtual/config.d.ts (1)
3-6: Good type narrowing for config serialization.The change from using the full
ApibaraConfigtype to a more specificPicktype with only the necessary properties (runtimeConfig,preset, andpresets) aligns perfectly with the PR objective of avoiding bundling the entire apibara.config. This focused approach will reduce the bundle size and ensure only the essential configuration properties are serialized at runtime.packages/cli/src/rolldown/config.ts (2)
3-3: Import added for the new plugin.The
@rollup/plugin-replaceimport is correctly added to support runtime config serialization.
79-86: Good implementation of the config serialization strategy.The addition of the
replaceplugin withprocess.env.APIBARA_CONFIGreplacement is a clean way to inject the serialized runtime configuration during the build process. This approach avoids bundling the entire apibara.config file as intended.packages/cli/src/rolldown/plugins/config.ts (1)
8-14: Effective approach to integrate environment-based config.The change from file-based configuration to an environment variable approach aligns with the PR objective. The error handling for when
APIBARA_CONFIGis undefined is a good defensive programming practice.This change works in conjunction with the
@rollup/plugin-replaceimplementation in the rolldown config to effectively inject the serialized configuration at build time.packages/cli/src/runtime/internal/app.ts (1)
59-59: Changed variable declaration to constant.Changing from
lettoconstfor the reporter is appropriate since the custom logger functionality is being disabled, meaning the variable no longer needs to be reassignable.packages/cli/src/core/config/resolvers/runtime-config.resolver.ts (2)
7-12: Good practice in error reporting
This clear error message ensures users know how to fix a missingpresetsdefinition whenpresetis specified.
18-21: Confirm the intended overwrite direction
Merging the preset’sruntimeConfiglast means the preset overrides user-specified values. If you intended user-specified values to override the preset, reverse the merge order.
packages/cli/src/core/config/resolvers/runtime-config.resolver.ts
Outdated
Show resolved
Hide resolved
fracek
left a comment
fracek
left a comment
There was a problem hiding this comment.
I think we just need to support serializing/deserializing bigint and we're good.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
examples/cli/indexers/1-evm.indexer.ts (1)
18-21: Consider adding defensive programming for configuration values.The code assumes that
evmandstartingBlockalways exist in the runtime configuration. Consider adding default values or validation to handle cases where these properties might be missing.const { connectionString, - evm: { startingBlock }, + evm: { startingBlock } = { startingBlock: 215_30_000n }, } = runtimeConfig;Or with more comprehensive error handling:
const { connectionString, - evm: { startingBlock }, + evm, } = runtimeConfig; + + if (!evm || typeof evm.startingBlock === 'undefined') { + throw new Error('Missing required evm.startingBlock in runtime configuration'); + } + + const { startingBlock } = evm;packages/cli/src/hooks/useRuntimeConfig.ts (1)
5-5: Consider adding fallback error handling for invalid JSON.If
process.env.APIBARA_RUNTIME_CONFIGis not valid JSON,JSON.parsewill throw. Handling or logging parse errors here might improve resilience and debugging.packages/cli/src/utils/helper.ts (1)
1-7: Support negative BigInts in the reviver.Currently, the regex only matches positive integers ending with “n”. If negative BigInt strings (e.g.,
-42n) are needed, consider updating the regex pattern:- typeof value === "string" && value.match(/^\d+n$/) + typeof value === "string" && value.match(/^-?\d+n$/)packages/cli/src/rolldown/plugins/config.ts (3)
10-12: Improve error handling for invalid configuration.The current error handling only checks if the config is undefined or empty. Consider adding additional validation to handle cases where the environment variable exists but contains invalid JSON.
if (serializedConfig === undefined || serializedConfig === "") { throw new Error("APIBARA_CONFIG is not defined"); } +try { + JSON.parse(serializedConfig); +} catch (error) { + throw new Error("APIBARA_CONFIG contains invalid JSON: " + error.message); +}
14-20: Enhance BigInt deserialization to support negative numbers.The current regex pattern only matches positive BigInt values. Update it to also handle negative values.
function deserialize(str) { return JSON.parse(str, (_, value) => - typeof value === "string" && value.match(/^\\d+n$/) + typeof value === "string" && value.match(/^-?\\d+n$/) ? BigInt(value.slice(0, -1)) : value, ); }
22-22: Consider adding error handling for the deserialization process.It's a good practice to wrap the deserialization in a try-catch block to provide clearer error messages when parsing fails.
-export const config = deserialize(serializedConfig); +try { + export const config = deserialize(serializedConfig); +} catch (error) { + throw new Error(`Failed to deserialize configuration: ${error.message}`); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
examples/cli/apibara.config.ts(1 hunks)examples/cli/indexers/1-evm.indexer.ts(2 hunks)examples/cli/indexers/2-starknet.indexer.ts(2 hunks)examples/cli/test/ethereum.test.ts(1 hunks)examples/cli/test/starknet.test.ts(1 hunks)packages/cli/src/core/config/resolvers/runtime-config.resolver.ts(1 hunks)packages/cli/src/hooks/useRuntimeConfig.ts(1 hunks)packages/cli/src/rolldown/config.ts(3 hunks)packages/cli/src/rolldown/plugins/config.ts(1 hunks)packages/cli/src/utils/helper.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cli/src/rolldown/config.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (12)
examples/cli/apibara.config.ts (1)
5-10: Good addition of structured configuration for EVM and StarkNet environmentsThe new configuration objects for both EVM and StarkNet with their respective starting block numbers are well-structured and improve configurability. Using bigint literals with underscore separators (
215_30_000nand10_30_000n) improves the readability of these large numbers.examples/cli/test/starknet.test.ts (1)
14-18: Well-structured test configurationThe updated
createIndexercall properly incorporates the new configuration parameters. Note that the test is using a different starting block value (800_000n) than the one in the main configuration (10_30_000n), which is appropriate for this test since it's only examining blocks 800_000 through 800_005.examples/cli/indexers/2-starknet.indexer.ts (2)
13-16: Good refactoring to use configured starting blockThe destructuring of the runtime configuration now properly extracts the nested
startingBlockfrom thestarknetobject. This is a better approach than hardcoding values.
28-28: Clean implementation using the configured starting blockUsing the dynamically configured
startingBlockvalue here improves flexibility and maintainability.examples/cli/test/ethereum.test.ts (1)
14-18: Well-structured test configuration with consistent patternThe test properly includes both EVM and StarkNet configuration parameters, following the same pattern as the StarkNet test. The test uses a different starting block value for testing purposes, which is appropriate since it examines only a small block range (10_000_000 to 10_000_005).
examples/cli/indexers/1-evm.indexer.ts (2)
18-21: Good refactoring to use configuration instead of hardcoded values.This change enhances configurability by extracting the
startingBlockfrom the nestedevmobject inruntimeConfiginstead of using a hardcoded value. This is a good practice that aligns with the PR objective of integrating runtime config serialization.
33-33: LGTM! Good use of the configured value.Using the destructured
startingBlockfrom configuration instead of a hardcoded value improves flexibility and maintainability.packages/cli/src/hooks/useRuntimeConfig.ts (1)
2-2: LGTM for the new import.The introduction of
deserializefrom../utils/helperis a clean way to centralize parsing logic.packages/cli/src/utils/helper.ts (1)
9-15: Validate unified handling of BigInts.The serializer logic for BigInt is consistent, but ensure you also handle negative values equivalently (e.g.,
-42n). This approach maintains parity with the proposed regex update indeserialize.packages/cli/src/core/config/resolvers/runtime-config.resolver.ts (2)
2-23: Robust preset merging logic.Merging the specified preset's
runtimeConfiginto the existingruntimeConfigis well-structured. Throwing clear errors for undefined or missing presets ensures the user is aware of configuration mistakes. No specific concerns here.
24-24: Minimize reliance onprocess.envfor local config.Storing configuration directly in
process.envmay cause side effects in multi-step scripts or runtime re-entry. Consider returning the computed config instead of mutating environment variables.packages/cli/src/rolldown/plugins/config.ts (1)
8-8:✅ Verification successful
Review the environment variable replacement pattern.
The template literal syntax
\process.env.APIBARA_CONFIG`` suggests this string will be processed by a build tool to replace it with the actual environment variable value. Make sure the string replacement is working as expected with your rollup configuration.
🏁 Script executed:
#!/bin/bash # Check if @rollup/plugin-replace is correctly configured in your build setup grep -r "@rollup/plugin-replace" packages/cli/Length of output: 194
Environment Variable Replacement Config Looks Correct
The build setup includes the
@rollup/plugin-replacedependency (as seen inpackages/cli/package.json) and its import inpackages/cli/src/rolldown/config.tsverifies it’s available for processing. The literalconst serializedConfig = `process.env.APIBARA_CONFIG`;in
packages/cli/src/rolldown/plugins/config.tswill be replaced at build time if your rollup configuration correctly targets that string. Please ensure through a full build/test cycle that the expected runtime value is injected as intended.
2 participants
No description provided.