fix(indexer): resolve compilation and migration issues in local deployment#3345
fix(indexer): resolve compilation and migration issues in local deployment#3345yasyzb wants to merge 1 commit into
Conversation
|
Need an answer fast? Review this PR in Change Stack to ask focused questions about the PR or a changed range. 📝 WalkthroughWalkthroughUpdates indexer build and compile settings: the Docker image build now runs ChangesIndexer Build Configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
indexer/tsconfig.json (1)
16-16: ⚡ Quick winConsider documenting the specific type conflict and treating this as a temporary workaround.
While
skipLibCheck: trueis effective for unblocking builds when third-party declaration files conflict, it masks underlying type issues rather than resolving them. Consider:
- Adding a comment in this file documenting which specific dependency or type conflict necessitated this flag
- Creating a follow-up task to investigate and resolve the root cause
- Monitoring whether future dependency updates resolve the conflict naturally
This approach helps future maintainers understand the reasoning and provides a path toward removing the workaround.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@indexer/tsconfig.json` at line 16, Add a short comment near the "skipLibCheck": true setting in tsconfig.json documenting the exact type conflict or dependency that required this workaround (e.g., which package or declaration file caused the error), add a TODO or follow-up task reference (issue/PR or tracker ID) to investigate and fix the root cause, and include guidance to revisit/remove this flag once that investigation or dependency updates resolve the conflict; keep the comment concise and actionable so future maintainers can find and address the underlying type issue.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@indexer/tsconfig.json`:
- Line 16: Add a short comment near the "skipLibCheck": true setting in
tsconfig.json documenting the exact type conflict or dependency that required
this workaround (e.g., which package or declaration file caused the error), add
a TODO or follow-up task reference (issue/PR or tracker ID) to investigate and
fix the root cause, and include guidance to revisit/remove this flag once that
investigation or dependency updates resolve the conflict; keep the comment
concise and actionable so future maintainers can find and address the underlying
type issue.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71be8257-83c4-4454-87cf-3fd50af440b3
📒 Files selected for processing (2)
indexer/Dockerfile.postgres-package.localindexer/tsconfig.json
5e58e2a to
7df72e1
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
indexer/Dockerfile.postgres-package.local (1)
43-43: Migrations require devDependencies (ts-node + .ts files), so removing--productionis correct.
indexer/packages/postgres/knexfile.jsloadsts-node/register- Migrations in
indexer/packages/postgres/src/db/migrations/migration_files/are all TypeScript (*.ts)indexer/packages/postgres/package.jsonrunsknex migrate:latest, anddevDependenciesincludets-node/typescriptSo
RUN pnpm i --loglevel warn --frozen-lockfile --unsafe-perm(instead of using--production) matches what’s needed for the migration container. Consider adding a short Dockerfile comment explaining why devDependencies must be installed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@indexer/Dockerfile.postgres-package.local` at line 43, Keep the current installation line using devDependencies (RUN pnpm i --loglevel warn --frozen-lockfile --unsafe-perm) and add a short Dockerfile comment above it explaining that devDependencies (ts-node/typescript) are required for running Knex migrations because indexer/packages/postgres/knexfile.js loads ts-node/register and migrations in indexer/packages/postgres/src/db/migrations/migration_files/ are TypeScript (*.ts); reference that the package.json runs "knex migrate:latest" so the container must install devDependencies rather than using --production.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@indexer/Dockerfile.postgres-package.local`:
- Line 43: Keep the current installation line using devDependencies (RUN pnpm i
--loglevel warn --frozen-lockfile --unsafe-perm) and add a short Dockerfile
comment above it explaining that devDependencies (ts-node/typescript) are
required for running Knex migrations because
indexer/packages/postgres/knexfile.js loads ts-node/register and migrations in
indexer/packages/postgres/src/db/migrations/migration_files/ are TypeScript
(*.ts); reference that the package.json runs "knex migrate:latest" so the
container must install devDependencies rather than using --production.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7ad61f6e-e930-4528-9b0e-7b21ada359e8
📒 Files selected for processing (2)
indexer/Dockerfile.postgres-package.localindexer/tsconfig.json
✅ Files skipped from review due to trivial changes (1)
- indexer/tsconfig.json
Description
This Pull Request resolves two universal issues encountered when compiling and setting up the local Indexer environment.
1. Enable
skipLibCheckin tsconfig compiler options.d.ts) often cause the build to fail."skipLibCheck": trueunder"compilerOptions"inindexer/tsconfig.json. This instructs the TypeScript compiler to skip type checking of declaration files (.d.ts) from third-party libraries, which is standard practice to avoid dependency-level type mismatches.2. Remove
--productionflag from pnpm install inDockerfile.postgres-package.localindexer/Dockerfile.postgres-package.local,NODE_ENVis set todevelopment, yetpnpm installis executed with the--productionflag. This prevents devDependencies (such astypescriptandts-node) from being installed. Consequently, the container fails to execute database migrations becausets-nodecannot be found.--productionflag from the local deployment Dockerfile to ensure that devDependencies required for local migration runs are properly installed.Test Plan
docker compose -f indexer/docker-compose-local-deployment.yml up --buildpostgres-package-1builds successfully, compiles typescript schemas, and exits with code0after successfully completing migrations.Summary by CodeRabbit