-
Notifications
You must be signed in to change notification settings - Fork 6
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
examples: add ts path alias and rollup typescript plugin #126
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces several changes primarily focused on updating the Rollup configuration for TypeScript support in the Changes
Possibly related PRs
Suggested reviewers
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
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 and nitpick comments (1)
examples/cli/apibara.config.ts (1)
1-1
: LGTM! Rollup TypeScript plugin integration looks goodThe integration of
@rollup/plugin-typescript
is correct, though we could enhance it with explicit configuration options.Consider adding TypeScript plugin options for better control:
rollupConfig: { - plugins: [typescript()], + plugins: [typescript({ + tsconfig: './tsconfig.json', + sourceMap: true, + compilerOptions: { + module: 'esnext', + } + })], },This configuration would:
- Explicitly specify the tsconfig path
- Enable source maps for better debugging
- Ensure proper module format output
Also applies to: 15-17
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
examples/cli/apibara.config.ts
(2 hunks)examples/cli/indexers/1-evm.indexer.ts
(1 hunks)examples/cli/indexers/2-starknet.indexer.ts
(1 hunks)examples/cli/package.json
(1 hunks)examples/cli/test/ethereum.test.ts
(1 hunks)examples/cli/test/helper.ts
(1 hunks)examples/cli/test/starknet.test.ts
(1 hunks)examples/cli/tsconfig.json
(1 hunks)examples/cli/vitest.config.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- examples/cli/indexers/2-starknet.indexer.ts
🔇 Additional comments (8)
examples/cli/vitest.config.ts (1)
1-1
: LGTM! Path alias support correctly configured for Vitest
The addition of vite-tsconfig-paths
plugin ensures proper resolution of TypeScript path aliases during testing.
Let's verify the path alias configuration:
Also applies to: 8-8
✅ Verification successful
Path alias configuration is correctly set up and actively used
The verification confirms:
tsconfig.json
has the correct path alias configuration mapping@/*
to./*
- Test files are actively using these path aliases (e.g.,
@/indexers
,@/lib/schema
) - The
vite-tsconfig-paths
plugin will properly resolve these aliases during testing
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if tsconfig.json has corresponding path configurations
rg -A 5 '"paths":\s*{' examples/cli/tsconfig.json
# Verify that path aliases are being used in test files
rg -l '@/' examples/cli/test/
Length of output: 267
Script:
#!/bin/bash
# Check tsconfig.json paths configuration with fixed regex
cat examples/cli/tsconfig.json | grep -A 5 "paths"
# Sample test files that use path aliases to verify the actual usage
head -n 20 examples/cli/test/starknet.test.ts
head -n 20 examples/cli/test/helper.ts
Length of output: 1159
examples/cli/test/helper.ts (1)
1-1
: LGTM! Consistent use of path alias
The change from relative to aliased import path aligns with the project's move towards using TypeScript path aliases.
Let's verify consistent usage of the path alias across the codebase:
✅ Verification successful
Path alias @/lib/schema is consistently used across the codebase
All imports of the schema module in the examples/cli directory are using the @/lib/schema path alias. No relative imports (../lib/schema) were found, confirming consistent usage across:
- examples/cli/test/starknet.test.ts
- examples/cli/indexers/2-starknet.indexer.ts
- examples/cli/indexers/1-evm.indexer.ts
- examples/cli/test/helper.ts
- examples/cli/test/ethereum.test.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining relative imports to lib/schema
rg -l '\.\./lib/schema' examples/cli/
# Verify consistent usage of @/lib/schema alias
rg -l '@/lib/schema' examples/cli/
Length of output: 255
Script:
#!/bin/bash
# Let's check the actual content of imports in these files to verify the consistency
rg -A 1 'import.*schema' examples/cli/test/starknet.test.ts examples/cli/indexers/2-starknet.indexer.ts examples/cli/indexers/1-evm.indexer.ts examples/cli/test/helper.ts examples/cli/test/ethereum.test.ts
Length of output: 1259
examples/cli/apibara.config.ts (1)
15-17
: Verify Rollup plugin compatibility
Let's ensure there are no conflicts with other Rollup plugins and that path aliases are properly resolved.
✅ Verification successful
Path aliases are properly configured and no plugin conflicts detected
The TypeScript plugin setup is correct and compatible with the project's requirements:
- The project uses
@rollup/plugin-typescript
which supports path aliases through tsconfig - Path aliases are correctly configured in tsconfig.json with
@/*
mapping - No conflicting Rollup plugins are present in the dependencies
- Path aliases are being used correctly in the indexer files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other Rollup plugins that might conflict
rg -l 'rollup.*plugin' examples/cli/
# Verify that path aliases are used in files processed by Rollup
find examples/cli/indexers -type f -name "*.ts" -exec rg -l '@/' {} \;
Length of output: 252
Script:
#!/bin/bash
# Check package.json for rollup plugins and dependencies
cat examples/cli/package.json | jq '.dependencies, .devDependencies'
# Check the actual usage of path aliases in the indexer files
rg -A 5 '@/' examples/cli/indexers/
# Check tsconfig for path alias configuration
fd tsconfig.json --exec cat {}
Length of output: 5538
examples/cli/tsconfig.json (1)
14-17
: LGTM! Path alias configuration is properly set up.
The TypeScript path alias configuration is correctly implemented with:
baseUrl
set to "." for proper path resolution- Path alias "@/" mapping to "./" for absolute imports
Let's verify the path alias usage across the codebase:
✅ Verification successful
Path alias configuration is working correctly and being used consistently
The verification shows that:
- The
@/
path alias is actively used in theexamples/cli
directory, with 11 imports following the configured pattern - All imports using
@/
are properly resolving to local files within the project - The relative imports (
../
) found are in different packages and not in theexamples/cli
directory where the path alias is configured
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent usage of the new path alias
# Look for import statements using the @/ prefix
rg "^import.*from\s+['\"]@/" --type ts
# Look for any potential relative imports that could be using the alias
rg "^import.*from\s+['\"]\.\./" --type ts
Length of output: 5174
examples/cli/package.json (1)
21-21
: Verify dependency version compatibility and security.
The dependency changes look good, replacing rollup-plugin-tsconfig-paths
with more maintained alternatives:
@rollup/plugin-typescript
for Rollup TypeScript supportvite-tsconfig-paths
for test environment path resolution
Let's check for any known issues with these versions:
Also applies to: 26-26
✅ Verification successful
Let me verify the TypeScript and Rollup versions to ensure compatibility with the peer dependencies.
Let me check for Rollup in the dependencies as well, since it might be in devDependencies.
Let me check if Rollup is a dependency of any workspace packages since this is using workspace dependencies.
Dependencies are compatible and secure
The verification shows:
- No security vulnerabilities found for
@rollup/plugin-typescript
- TypeScript v5.6.2 satisfies the peer dependency requirement (>=3.7.0)
- Rollup v4.18.1 satisfies the peer dependency requirement (^2.14.0||^3.0.0||^4.0.0)
- All version constraints are properly specified with
^
for minor updates vite-tsconfig-paths
has a flexible peer dependency on vite (*
)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for compatibility issues and security advisories
# Check for security advisories
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "@rollup/plugin-typescript") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}
'
# Check peer dependencies
npm info @rollup/[email protected] peerDependencies
npm info [email protected] peerDependencies
Length of output: 511
Script:
#!/bin/bash
# Check TypeScript and Rollup versions in package.json
rg -U '"typescript":\s*"[^"]*"|"rollup":\s*"[^"]*"' examples/cli/package.json
Length of output: 107
Script:
#!/bin/bash
# Check all dependencies sections for Rollup
cat examples/cli/package.json
Length of output: 1080
Script:
#!/bin/bash
# Check for rollup in workspace packages
fd package.json --type f --exec rg -U '"rollup":\s*"[^"]*"' {}
Length of output: 91
examples/cli/indexers/1-evm.indexer.ts (1)
1-2
: LGTM! Import paths updated to use the new path alias.
The import statements have been properly updated to use the "@/" path alias, improving code maintainability:
- import { db } from "../lib/db";
- import { ethereumUsdcTransfers } from "../lib/schema";
+ import { db } from "@/lib/db";
+ import { ethereumUsdcTransfers } from "@/lib/schema";
examples/cli/test/ethereum.test.ts (1)
1-3
: LGTM! Verify TypeScript path alias configuration.
The transition to absolute imports using the @/
path alias improves code maintainability. The changes align with the PR objectives.
Let's verify the path alias configuration and its consistent usage:
✅ Verification successful
Path alias configuration is properly set up
The TypeScript path alias configuration is correctly implemented:
- The
tsconfig.json
in examples/cli has the proper path configuration with"@/*": ["./*"]
- Path resolution is handled by
vite-tsconfig-paths
plugin in the test setup - All imports using
@/
prefix in the codebase follow a consistent pattern
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify TypeScript path alias configuration and usage
# Check tsconfig.json for path alias configuration
echo "Checking tsconfig.json for path alias configuration..."
cat tsconfig.json | jq '.compilerOptions.paths'
# Check for any remaining relative imports that could be converted to path aliases
echo "Checking for remaining relative imports in the codebase..."
rg -g '*.{ts,tsx}' -l '\.\./(?!\.)'
# Verify Rollup configuration for path alias handling
echo "Checking Rollup configuration..."
cat examples/cli/apibara.config.ts
Length of output: 1028
Script:
#!/bin/bash
# Let's try a different approach to verify the configuration and usage
# Check for any tsconfig files in the repository
echo "Looking for tsconfig files..."
fd tsconfig
# Check package.json for path alias related configurations
echo "Checking package.json..."
cat package.json
# Check for imports using @ prefix
echo "Checking usage of @ imports..."
rg -g '*.{ts,tsx}' '^import.*from.*"@/'
Length of output: 2777
Script:
#!/bin/bash
# Let's check the specific tsconfig.json in the examples/cli directory since that's where our file is located
echo "Checking examples/cli/tsconfig.json..."
cat examples/cli/tsconfig.json
# Also check if there's any path alias configuration in the build setup
echo "Checking for build configuration files in examples/cli..."
fd -t f -e js -e ts -e json "build|vite|rollup|webpack" examples/cli/
Length of output: 829
Script:
#!/bin/bash
# Let's check the vitest configuration as it might contain path alias resolution
echo "Checking vitest.config.ts for path alias configuration..."
cat examples/cli/vitest.config.ts
# Let's also verify if all @ imports in the directory follow the same pattern
echo "Checking all import patterns in examples/cli..."
rg -g '*.{ts,tsx}' '^import' examples/cli/
Length of output: 5332
examples/cli/test/starknet.test.ts (1)
1-3
: LGTM! Import paths standardized consistently.
The changes maintain consistency with other test files while improving import structure using path aliases.
Let's verify that the test functionality remains intact:
✅ Verification successful
Import changes are safe and test functionality is preserved
The test file is fully functional with:
- Working imports from correct locations (
@/indexers/2-starknet.indexer
and@/lib/schema
) - Complete test implementation with snapshot assertions
- Proper test setup using vitest
- Matching imported module paths (verified schema.ts and indexer files exist)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify test snapshots and configurations
# Check if test snapshots exist and are up to date
echo "Checking for existing test snapshots..."
find examples/cli/test -name '*.snap'
# Verify test configuration
echo "Checking vitest configuration..."
cat vitest.config.ts
# Check for any test-related TODO or FIXME comments
echo "Checking for test-related TODOs..."
rg -g '*.{ts,tsx}' 'TODO|FIXME' examples/cli/test/
Length of output: 407
Script:
#!/bin/bash
# Let's verify the test functionality by checking:
# 1. The actual test file content
# 2. Related test files
# 3. The imported modules
# Check the test file content
echo "=== Content of starknet.test.ts ==="
cat examples/cli/test/starknet.test.ts
# Check for any jest/vitest config files
echo -e "\n=== Looking for test config files ==="
fd -g '*test*config*' -g '*jest*config*' -g '*vitest*config*'
# Check the imported modules
echo -e "\n=== Checking imported modules ==="
echo "1. Checking starknet indexer:"
fd -g '*2-starknet.indexer*'
echo -e "\n2. Checking schema files:"
fd -g '*schema*'
Length of output: 5795
import { db } from "@/lib/db"; | ||
import { ethereumUsdcTransfers } from "@/lib/schema"; |
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.
Can you move local imports at the bottom? I like to always have global imports at the top, followed by local import (with alias), followed by local import with relative path.
import { db } from "@/lib/db"; | ||
import { starknetUsdcTransfers } from "@/lib/schema"; |
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.
Same here.
examples/cli/test/ethereum.test.ts
Outdated
import { createIndexer } from "@/indexers/1-evm.indexer"; | ||
import { ethereumUsdcTransfers } from "@/lib/schema"; | ||
import * as schema from "@/lib/schema"; |
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.
Same here.
examples/cli/test/helper.ts
Outdated
@@ -1,6 +1,6 @@ | |||
import type * as schema from "@/lib/schema"; |
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.
Same here.
examples/cli/test/starknet.test.ts
Outdated
import { createIndexer } from "@/indexers/2-starknet.indexer"; | ||
import { starknetUsdcTransfers } from "@/lib/schema"; | ||
import * as schema from "@/lib/schema"; |
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.
Same here.
Perfect thanks!! |
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: 1
🧹 Outside diff range and nitpick comments (6)
examples/cli/lib/db.ts (1)
Line range hint
8-10
: Add error handling for database connectionThe pool creation lacks error handling which could lead to silent failures.
Consider adding error handling:
const pool = new pg.Pool({ connectionString, }); +pool.on('error', (err) => { + console.error('Unexpected error on idle client', err); + process.exit(-1); +});examples/cli/lib/schema.ts (2)
Line range hint
5-13
: Consider abstracting common table structureBoth tables have identical structure. Consider creating a reusable factory function.
Here's a suggested refactor:
+const createTransferTable = (name: string) => pgIndexerTable(name, { + number: bigint("number", { mode: "number" }), + hash: text("hash"), +}); + -export const starknetUsdcTransfers = pgIndexerTable("starknet_usdc_transfers", { - number: bigint("number", { mode: "number" }), - hash: text("hash"), -}); +export const starknetUsdcTransfers = createTransferTable("starknet_usdc_transfers"); -export const ethereumUsdcTransfers = pgIndexerTable("ethereum_usdc_transfers", { - number: bigint("number", { mode: "number" }), - hash: text("hash"), -}); +export const ethereumUsdcTransfers = createTransferTable("ethereum_usdc_transfers");
Line range hint
5-13
: Consider adding indexes for query performanceThe tables lack indexes which could impact query performance.
Consider adding indexes for frequently queried fields:
export const starknetUsdcTransfers = pgIndexerTable("starknet_usdc_transfers", { number: bigint("number", { mode: "number" }), hash: text("hash"), +}, (table) => ({ + numberIdx: index("starknet_number_idx").on(table.number), + hashIdx: index("starknet_hash_idx").on(table.hash), }));examples/cli/indexers/3-starknet-factory.indexer.ts (3)
Line range hint
13-15
: Consider extracting configuration constantsThe indexer contains hardcoded values that should be configurable.
Consider moving these to a configuration file:
+// config.ts +export const FACTORY_ADDRESS = "0x00dad44c139a476c7a17fc8141e6db680e9abc9f56fe249a105094c44382c2fd"; +export const STARTING_BLOCK = 800_000n; - address: "0x00dad44c139a476c7a17fc8141e6db680e9abc9f56fe249a105094c44382c2fd", + address: FACTORY_ADDRESS,
Line range hint
41-45
: Consider using a logging library for colored outputDirect ANSI color codes in logs can cause issues in environments that don't support them.
Consider using a logging library like
chalk
or creating a utility function:+const formatAddress = (address: string) => { + return process.stdout.isTTY + ? `\x1b[35m${address}\x1b[0m` + : address; +}; -logger.log("Factory: PairAddress : ", `\x1b[35m${pairAddress}\x1b[0m`); +logger.log("Factory: PairAddress : ", formatAddress(pairAddress));
Line range hint
55-64
: Add error handling for event processingThe transform function lacks error handling for event processing.
Consider adding try-catch blocks:
async transform({ block, endCursor }) { const logger = useLogger(); const { events } = block; logger.log("Transforming... : ", endCursor?.orderKey); for (const event of events) { + try { logger.log( "Event Address : ", shortAddress(event.address), "| Txn hash :", shortAddress(event.transactionHash), ); + } catch (error) { + logger.error(`Failed to process event: ${error.message}`); + // Decide whether to continue or throw based on requirements + } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
examples/cli/apibara.config.ts
(2 hunks)examples/cli/indexers/1-evm.indexer.ts
(1 hunks)examples/cli/indexers/2-starknet.indexer.ts
(1 hunks)examples/cli/indexers/3-starknet-factory.indexer.ts
(1 hunks)examples/cli/lib/db.ts
(1 hunks)examples/cli/lib/schema.ts
(1 hunks)examples/cli/package.json
(1 hunks)examples/cli/test/ethereum.test.ts
(1 hunks)examples/cli/test/helper.ts
(1 hunks)examples/cli/test/starknet.test.ts
(1 hunks)examples/cli/tsconfig.json
(1 hunks)examples/cli/vitest.config.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- examples/cli/vitest.config.ts
- examples/cli/apibara.config.ts
- examples/cli/test/starknet.test.ts
- examples/cli/test/helper.ts
- examples/cli/test/ethereum.test.ts
- examples/cli/tsconfig.json
- examples/cli/indexers/1-evm.indexer.ts
- examples/cli/package.json
- examples/cli/indexers/2-starknet.indexer.ts
@@ -1,5 +1,6 @@ | |||
import { type NodePgDatabase, drizzle } from "drizzle-orm/node-postgres"; | |||
import pg from "pg"; | |||
|
|||
import * as schema from "./schema"; | |||
|
|||
const connectionString = "postgres://postgres:postgres@localhost:5432/postgres"; |
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.
🛠️ Refactor suggestion
Consider using environment variables for database configuration
The connection string is hardcoded which could pose security risks and make deployment across different environments difficult.
Consider using environment variables:
-const connectionString = "postgres://postgres:postgres@localhost:5432/postgres";
+const connectionString = process.env.DATABASE_URL || "postgres://postgres:postgres@localhost:5432/postgres";
📝 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 connectionString = "postgres://postgres:postgres@localhost:5432/postgres"; | |
const connectionString = process.env.DATABASE_URL || "postgres://postgres:postgres@localhost:5432/postgres"; |
updates the cli example to showcase how to use rollup plugins in
apibara.config.ts
under scenarios where you have custom path alias in tsconfig.