feat: hasura setup script #59
Conversation
📝 WalkthroughWalkthroughThe pull request introduces a comprehensive Hasura configuration script project within the Key components of the project include a TypeScript script for configuring Hasura by tracking tables, relationships, and custom functions, as well as a new The project structure supports modularity, with separate files for types, services, and exceptions. A new Dockerfile and Docker Compose service have been added to facilitate deployment. Overall, the changes establish a standardized approach to managing Hasura metadata programmatically, enhancing type safety and usability. ✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
scripts/hasura/src/services/hasura.api.ts (1)
22-34: Suggest externalizing the Axios instance for better testability.Currently, the constructor configures a new Axios instance directly. It's often considered more flexible to inject the HTTP client rather than creating it internally, thereby facilitating unit testing and allowing you to replace or mock the transport layer.
scripts/hasura/src/configure-hasura.script.ts (1)
1-347: Consider adding a script entry in package.json.Since this is a CLI script, adding a dedicated script target (e.g.
"script:infra:configure-hasura") in yourpackage.jsoncan simplify usage and avoid manual invocation, aligning with your folder conventions for scripts.scripts/hasura/src/exceptions/hasuraApi.exception.ts (1)
1-6: Add JSDoc for clarity and maintainability.This custom exception class is straightforward, but it lacks documentation. Per the guidelines for TypeScript, please provide JSDoc comments explaining the purpose and usage context of this exception class to help future maintainers.
scripts/hasura/src/types/hasura.types.ts (4)
1-6: Consider runtime validation for environment-driven properties.The
HasuraConfigtype references environment-dependent properties (endpoint,adminSecret, etc.). Adding runtime type-checking (e.g., with Zod) ensures these values remain valid under real-world conditions, especially when users misconfigure environment variables.
8-15: Add JSDoc to clarify structure and usage.Consider adding a concise JSDoc comment describing how the
ManualConfigurationtype should be used, including guidelines or potential pitfalls when extending theTablesarray.
17-27: Ensure relationships are correctly represented.The
RelationshipConfigtype is comprehensive but could benefit from JSDoc clarifying permissible relationship structures and constraints. This helps new contributors avoid misconfigurations in advanced Hasura setups.
29-32: Document the purpose of the custom function type.Providing a short JSDoc for
CustomFunctionensures developers quickly understand how custom functions are tracked and used within the Hasura configuration script.scripts/hasura/README.md (1)
1-121: Align script usage with recommended naming conventions.The README is clear and detailed. To follow the coding guidelines for
scripts/**/*.ts, consider referencing a more structured command naming scheme, such asscript:infra:configureHasura, in yourpackage.json. This promotes consistency and helps other contributors quickly identify and run infrastructure scripts without confusion.🧰 Tools
🪛 LanguageTool
[misspelling] ~50-~50: Did you mean the verb “rolls”?
Context: ...ject_roles - rounds - pending_round_roles - round_roles - applications - ap...(ROLE_ROLL)
[duplication] ~52-~52: Possible typo: you repeated a word.
Context: ...pending_round_roles - round_roles - applications - applications_payouts - donations - legacy_projec...(ENGLISH_WORD_REPEAT_RULE)
📜 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 (13)
scripts/hasura/README.md(1 hunks)scripts/hasura/package.json(1 hunks)scripts/hasura/src/configure-hasura.script.ts(1 hunks)scripts/hasura/src/exceptions/hasuraApi.exception.ts(1 hunks)scripts/hasura/src/exceptions/index.ts(1 hunks)scripts/hasura/src/exceptions/network.exception.ts(1 hunks)scripts/hasura/src/internal.ts(1 hunks)scripts/hasura/src/services/hasura.api.ts(1 hunks)scripts/hasura/src/services/index.ts(1 hunks)scripts/hasura/src/types/hasura.types.ts(1 hunks)scripts/hasura/src/types/index.ts(1 hunks)scripts/hasura/tsconfig.build.json(1 hunks)scripts/hasura/tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (7)
- scripts/hasura/src/services/index.ts
- scripts/hasura/src/types/index.ts
- scripts/hasura/tsconfig.json
- scripts/hasura/package.json
- scripts/hasura/src/exceptions/index.ts
- scripts/hasura/src/internal.ts
- scripts/hasura/tsconfig.build.json
🧰 Additional context used
📓 Path-based instructions (5)
scripts/hasura/src/exceptions/hasuraApi.exception.ts (2)
Pattern **/*.ts: Review TypeScript files for adherence to the following guidelines:
- Avoid over-abstraction; prioritize composition over inheritance.
- Use dependency injection and follow SOLID principles.
- Avoid any; use unknown when necessary.
- Use runtime type-checking for environment variables (e.g., Zod).
- Prevent circular dependencies with the internal module pattern.
- Libraries should have an external.ts file explicitly listing public exports.
- Use bigint as-is; cast to Number only when exposing values via APIs.
- Document all code with JSDoc.
- Encourage static async factory functions for constructors.
- Avoid overly nitpicky feedback beyond these best practices.
Pattern scripts/**/*.ts: Ensure scripts:
- Use process.cwd() for root references.
- Follow folder conventions (infra/ for infra scripts, utilities/ for utilities).
- Are organized in package.json with script:infra:{name} or script:util:{name}.
- Be concise and avoid overly nitpicky feedback outside of these best practices.
scripts/hasura/src/configure-hasura.script.ts (2)
Pattern **/*.ts: Review TypeScript files for adherence to the following guidelines:
- Avoid over-abstraction; prioritize composition over inheritance.
- Use dependency injection and follow SOLID principles.
- Avoid any; use unknown when necessary.
- Use runtime type-checking for environment variables (e.g., Zod).
- Prevent circular dependencies with the internal module pattern.
- Libraries should have an external.ts file explicitly listing public exports.
- Use bigint as-is; cast to Number only when exposing values via APIs.
- Document all code with JSDoc.
- Encourage static async factory functions for constructors.
- Avoid overly nitpicky feedback beyond these best practices.
Pattern scripts/**/*.ts: Ensure scripts:
- Use process.cwd() for root references.
- Follow folder conventions (infra/ for infra scripts, utilities/ for utilities).
- Are organized in package.json with script:infra:{name} or script:util:{name}.
- Be concise and avoid overly nitpicky feedback outside of these best practices.
scripts/hasura/src/types/hasura.types.ts (2)
Pattern **/*.ts: Review TypeScript files for adherence to the following guidelines:
- Avoid over-abstraction; prioritize composition over inheritance.
- Use dependency injection and follow SOLID principles.
- Avoid any; use unknown when necessary.
- Use runtime type-checking for environment variables (e.g., Zod).
- Prevent circular dependencies with the internal module pattern.
- Libraries should have an external.ts file explicitly listing public exports.
- Use bigint as-is; cast to Number only when exposing values via APIs.
- Document all code with JSDoc.
- Encourage static async factory functions for constructors.
- Avoid overly nitpicky feedback beyond these best practices.
Pattern scripts/**/*.ts: Ensure scripts:
- Use process.cwd() for root references.
- Follow folder conventions (infra/ for infra scripts, utilities/ for utilities).
- Are organized in package.json with script:infra:{name} or script:util:{name}.
- Be concise and avoid overly nitpicky feedback outside of these best practices.
scripts/hasura/src/exceptions/network.exception.ts (2)
Pattern **/*.ts: Review TypeScript files for adherence to the following guidelines:
- Avoid over-abstraction; prioritize composition over inheritance.
- Use dependency injection and follow SOLID principles.
- Avoid any; use unknown when necessary.
- Use runtime type-checking for environment variables (e.g., Zod).
- Prevent circular dependencies with the internal module pattern.
- Libraries should have an external.ts file explicitly listing public exports.
- Use bigint as-is; cast to Number only when exposing values via APIs.
- Document all code with JSDoc.
- Encourage static async factory functions for constructors.
- Avoid overly nitpicky feedback beyond these best practices.
Pattern scripts/**/*.ts: Ensure scripts:
- Use process.cwd() for root references.
- Follow folder conventions (infra/ for infra scripts, utilities/ for utilities).
- Are organized in package.json with script:infra:{name} or script:util:{name}.
- Be concise and avoid overly nitpicky feedback outside of these best practices.
scripts/hasura/src/services/hasura.api.ts (3)
Pattern **/*.ts: Review TypeScript files for adherence to the following guidelines:
- Avoid over-abstraction; prioritize composition over inheritance.
- Use dependency injection and follow SOLID principles.
- Avoid any; use unknown when necessary.
- Use runtime type-checking for environment variables (e.g., Zod).
- Prevent circular dependencies with the internal module pattern.
- Libraries should have an external.ts file explicitly listing public exports.
- Use bigint as-is; cast to Number only when exposing values via APIs.
- Document all code with JSDoc.
- Encourage static async factory functions for constructors.
- Avoid overly nitpicky feedback beyond these best practices.
Pattern **/services/**/*.ts: Review service files with the following considerations:
- A Service encapsulates a broader business workflow and might orchestrate various components or interact with multiple data sources or APIs.
- Ensure proper composability: Services should use Providers for data/resource access and apply business/orchestration logic.
- Validate naming conventions for aggregating services (e.g., AggregatorService or domain-specific names like MetricsService).
- path: "/providers//*.ts"
instructions: |
Review provider files for the following:
- Providers should supply narrowly scoped data/resources.
- Ensure classes interacting with metadata sources (e.g., GitHub, JSON files, IPFS) implement the IMetadataProvider interface and follow naming conventions (e.g., GithubProvider, JsonFileProvider).
- Be concise and avoid overly nitpicky feedback outside of these best practices.
Pattern scripts/**/*.ts: Ensure scripts:
- Use process.cwd() for root references.
- Follow folder conventions (infra/ for infra scripts, utilities/ for utilities).
- Are organized in package.json with script:infra:{name} or script:util:{name}.
- Be concise and avoid overly nitpicky feedback outside of these best practices.
🪛 LanguageTool
scripts/hasura/README.md
[misspelling] ~50-~50: Did you mean the verb “rolls”?
Context: ...ject_roles - rounds - pending_round_roles - round_roles - applications - ap...
(ROLE_ROLL)
[duplication] ~52-~52: Possible typo: you repeated a word.
Context: ...pending_round_roles - round_roles - applications - applications_payouts - donations - legacy_projec...
(ENGLISH_WORD_REPEAT_RULE)
🔇 Additional comments (8)
scripts/hasura/src/services/hasura.api.ts (7)
36-53: Well-documented method.The
clearMetadatamethod is clearly documented and appears functionally correct for its intended purpose.
55-79: Good use of typed table names.Using
Tables[number]to enforce valid table names helps maintain type safety when tracking tables.
81-101: Array relationship creation logic is consistent.The logic for creating array relationships is straightforward, and the manual configuration is clearly structured.
103-123: Object relationship creation is clear.The method follows the same well-defined approach as the array relationship, keeping consistency across relationships.
125-156: Configurable fetch limit is a nice touch.Allowing a user-defined fetch limit for select permissions is a practical design choice.
158-182: Custom function tracking approach is correct.This method nicely abstracts the functionality of adding custom functions with minimal overhead.
191-213: Comprehensive error handling.Distinguishing between network errors and server errors enhances debugging. Throwing specialized exceptions is a SOLID approach.
scripts/hasura/src/exceptions/network.exception.ts (1)
1-6: Straightforward custom error helps clarify network issues.Raising a distinct
NetworkExceptionfacilitates more precise error handling and debugging.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
scripts/hasura/vitest.config.ts (1)
16-22: Optional: Use process.cwd() for path resolutionConsider referencing the project’s root directory via
process.cwd()for clarity and consistency with your script guidelines, although this is not strictly necessary here.scripts/hasura/Dockerfile (1)
1-24: Consider a multi-stage build to reduce image sizeYour current Docker setup is functional. For enhanced efficiency, you could adopt multi-stage builds to keep your final image minimal.
package.json (1)
10-10: Naming consistency for script commandsYour script name
api:configureis perfectly fine. If you want uniformity with the guidelines that suggestscript:infra:{name}orscript:util:{name}, you could rename it accordingly.docker-compose.yaml (1)
65-67: Consider environment variable validation.While the environment variables are correctly set, consider adding validation in the Dockerfile or entrypoint script to ensure these required variables are properly set before the service starts.
Example validation in entrypoint script:
#!/bin/bash required_vars=("HASURA_ENDPOINT" "HASURA_ADMIN_SECRET" "HASURA_SCHEMA") for var in "${required_vars[@]}"; do if [ -z "${!var}" ]; then echo "Error: Required environment variable $var is not set" exit 1 fi done
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docker-compose.yaml(1 hunks)package.json(1 hunks)scripts/hasura/Dockerfile(1 hunks)scripts/hasura/README.md(1 hunks)scripts/hasura/package.json(1 hunks)scripts/hasura/vitest.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/hasura/package.json
🧰 Additional context used
📓 Path-based instructions (1)
scripts/hasura/vitest.config.ts (2)
Pattern **/*.ts: Review TypeScript files for adherence to the following guidelines:
- Avoid over-abstraction; prioritize composition over inheritance.
- Use dependency injection and follow SOLID principles.
- Avoid any; use unknown when necessary.
- Use runtime type-checking for environment variables (e.g., Zod).
- Prevent circular dependencies with the internal module pattern.
- Libraries should have an external.ts file explicitly listing public exports.
- Use bigint as-is; cast to Number only when exposing values via APIs.
- Document all code with JSDoc.
- Encourage static async factory functions for constructors.
- Avoid overly nitpicky feedback beyond these best practices.
Pattern scripts/**/*.ts: Ensure scripts:
- Use process.cwd() for root references.
- Follow folder conventions (infra/ for infra scripts, utilities/ for utilities).
- Are organized in package.json with script:infra:{name} or script:util:{name}.
- Be concise and avoid overly nitpicky feedback outside of these best practices.
🪛 LanguageTool
scripts/hasura/README.md
[misspelling] ~50-~50: Did you mean the verb “rolls”?
Context: ...ject_roles - rounds - pending_round_roles - round_roles - applications - ap...
(ROLE_ROLL)
[duplication] ~52-~52: Possible typo: you repeated a word.
Context: ...pending_round_roles - round_roles - applications - applications_payouts - donations - legacy_projec...
(ENGLISH_WORD_REPEAT_RULE)
⏰ Context from checks skipped due to timeout of 300000ms (1)
- GitHub Check: lint / Run Linters
🔇 Additional comments (4)
scripts/hasura/vitest.config.ts (1)
5-15: Leverage script guidelines in test configurationThe Vitest test configuration here is correct and adheres to typical Node testing conventions.
scripts/hasura/README.md (1)
1-121: Documentation looks thoroughThe README provides clear instructions and usage details. The static analysis flag about “roles” vs. “rolls” is a false positive, so no change is needed. Great job on the detailed guide!
🧰 Tools
🪛 LanguageTool
[misspelling] ~50-~50: Did you mean the verb “rolls”?
Context: ...ject_roles - rounds - pending_round_roles - round_roles - applications - ap...(ROLE_ROLL)
[duplication] ~52-~52: Possible typo: you repeated a word.
Context: ...pending_round_roles - round_roles - applications - applications_payouts - donations - legacy_projec...(ENGLISH_WORD_REPEAT_RULE)
docker-compose.yaml (2)
58-72: LGTM! Well-structured service configuration.The new
datalayer-graphql-configservice is properly configured with appropriate dependencies, networking, and environment variables. Good practices observed:
- Uses health check condition for dependency
- No exposed ports (security best practice)
- Proper internal network communication
66-66: Verify the admin secret handling.The service is using the same admin secret as the main Hasura instance. While this is necessary for configuration, ensure that:
- The configuration service only runs during setup
- The admin secret is properly secured in production
✅ Verification successful
Admin secret handling is properly implemented ✅
- The configuration service runs only during setup (no restart policy)
- The admin secret is properly secured:
- Configured via environment variables
- No hardcoded secrets in CI/CD
- Production environment files are not committed to the repository
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if admin secret is properly secured in CI/CD if [ -f ".github/workflows/deploy.yml" ]; then echo "Checking CI/CD configuration for secure secret handling..." rg -i "DATALAYER_HASURA_ADMIN_SECRET" .github/workflows/ fi # Check if service is configured to run only during setup rg -i "restart:" -A 1 docker-composeLength of output: 133
Script:
#!/bin/bash # Check docker-compose configuration echo "=== Docker Compose Configuration ===" cat docker-compose.yaml # Check for CI/CD files echo -e "\n=== CI/CD Configuration Files ===" fd -g '*.yml' .github/workflows/ # Check for environment files echo -e "\n=== Environment Files ===" fd -g '.env*'Length of output: 5518
Script:
#!/bin/bash # Check CI/CD files for secret handling echo "=== CI/CD Secret Handling ===" rg -i "DATALAYER_HASURA_ADMIN_SECRET" .github/workflows/ # Check environment files echo -e "\n=== Environment Files Content ===" fd -H -g '.env*' # Check workflow files for secure practices echo -e "\n=== Workflow Files Secret Handling ===" for file in $(fd -g '*.yml' .github/workflows/); do echo "=== $file ===" grep -i "secret" "$file" doneLength of output: 1361
| source: "default", | ||
| column_mapping: { | ||
| project_id: "id", | ||
| chain_id: "chain_id", | ||
| }, | ||
| }, | ||
| }, | ||
| source: "default", | ||
| }, | ||
| // Round roles object relationships | ||
| { | ||
| name: "round", | ||
| table: { | ||
| name: "round_roles", | ||
| schema: "public", | ||
| }, | ||
| using: { | ||
| manual_configuration: { | ||
| remote_table: { | ||
| name: "rounds", | ||
| schema: "public", | ||
| }, | ||
| source: "default", | ||
| column_mapping: { | ||
| round_id: "id", | ||
| chain_id: "chain_id", | ||
| }, | ||
| }, | ||
| }, | ||
| source: "default", | ||
| }, | ||
| // Applications object relationships | ||
| { | ||
| name: "project", | ||
| table: { | ||
| name: "applications", | ||
| schema: "public", | ||
| }, | ||
| using: { | ||
| manual_configuration: { | ||
| remote_table: { | ||
| name: "projects", | ||
| schema: "public", | ||
| }, | ||
| source: "default", | ||
| column_mapping: { | ||
| project_id: "id", | ||
| chain_id: "chain_id", | ||
| }, | ||
| }, | ||
| }, | ||
| source: "default", | ||
| }, | ||
| { | ||
| name: "round", | ||
| table: { | ||
| name: "applications", | ||
| schema: "public", | ||
| }, | ||
| using: { | ||
| manual_configuration: { | ||
| remote_table: { | ||
| name: "rounds", | ||
| schema: "public", | ||
| }, | ||
| source: "default", | ||
| column_mapping: { | ||
| round_id: "id", | ||
| chain_id: "chain_id", | ||
| }, | ||
| }, | ||
| }, | ||
| source: "default", | ||
| }, | ||
| // Applications payouts object relationships | ||
| { | ||
| name: "application", | ||
| table: { | ||
| name: "applications_payouts", | ||
| schema: "public", | ||
| }, | ||
| using: { | ||
| manual_configuration: { | ||
| remote_table: { | ||
| name: "applications", | ||
| schema: "public", | ||
| }, | ||
| source: "default", | ||
| column_mapping: { | ||
| application_id: "id", | ||
| chain_id: "chain_id", | ||
| round_id: "round_id", | ||
| }, | ||
| }, | ||
| }, | ||
| source: "default", | ||
| }, | ||
| ]; | ||
|
|
||
| // Custom functions to track |
There was a problem hiding this comment.
should be a way to build this dynamically as hasura does in its UI interface. Lets dive a little more into what does hasura do to get tables and relationships.
There was a problem hiding this comment.
thanks for pointing this out, found that Hasura has an ep that returns suggested relationships 😄
unluckily is not present for custom functions but i think since there's just one, we're fine manually tracking it
a09fd68
There was a problem hiding this comment.
Maybe i missed it in the selection but should be suggestions for tables also 🤔
There was a problem hiding this comment.
there isn't a direct way, we could manage to make some hacky way of introspecting the database with plain SQL to get all tables for tracking them, but we don't want to track them all (like registry, caches, or kysely migrations)
here i think the safest way is the verbosity of explicitly say which tables we want to track
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
scripts/hasura/src/services/hasura.api.ts (4)
27-55: Add input validation and improve axios instance creation.While the constructor implementation is good, consider these improvements:
- Add runtime validation for configuration parameters
- Extract axios instance creation to a separate private method
Here's a suggested implementation:
constructor(config: HasuraConfig) { + // Validate config + if (!config.adminSecret) throw new Error('Admin secret is required'); + if (!config.endpoint) throw new Error('Endpoint is required'); + if (!config.schema) throw new Error('Schema is required'); + if (!config.fetchLimit || config.fetchLimit <= 0) throw new Error('Invalid fetch limit'); + this.adminSecret = config.adminSecret; this.endpoint = config.endpoint; this.schema = config.schema; this.fetchLimit = config.fetchLimit; - this.axiosInstance = axios.create({ - baseURL: this.endpoint, - headers: { - "Content-Type": "application/json", - "X-Hasura-Admin-Secret": this.adminSecret, - }, - }); + this.axiosInstance = this.createAxiosInstance(); } + +private createAxiosInstance(): AxiosInstance { + return axios.create({ + baseURL: this.endpoint, + headers: { + "Content-Type": "application/json", + "X-Hasura-Admin-Secret": this.adminSecret, + }, + }); +}
48-54: Add request timeout and retry logic.Consider adding:
- Request timeout configuration to prevent hanging requests
- Retry logic for transient network errors
this.axiosInstance = axios.create({ baseURL: this.endpoint, headers: { "Content-Type": "application/json", "X-Hasura-Admin-Secret": this.adminSecret, }, + timeout: 5000, // 5 seconds + // Add axios-retry package for retry logic });
281-303: Enhance error handling with specific error types and logging.Consider these improvements to the error handling:
- Create specific error types for different API errors (e.g.,
ValidationError,AuthenticationError)- Add detailed error logging for debugging
private handleError(err: unknown, operation: string): never { const error = err as Error | AxiosError; let errorMessage = `❌ Failed to ${operation}`; + + // Log error details for debugging + console.error('Operation failed:', { + operation, + error: error instanceof Error ? error.stack : error, + }); if (isAxiosError<{ error: string; code: string }>(error)) { if (!error.response) { throw new NetworkException(`Network error while ${operation}: ${error.message}`); } if (error.response.status >= 500) { throw new HasuraApiException( `Server error while ${operation}: ${error.response.data || error.message}`, ); } + // Add specific error types based on status codes + switch (error.response.status) { + case 401: + throw new AuthenticationError(errorMessage); + case 422: + throw new ValidationError(errorMessage); + default: + errorMessage += `: ${error.response.data.error}`; + } - errorMessage += `: ${error.response.data.error}`; } else { errorMessage += `: ${error.message}`; } throw new HasuraApiException(errorMessage); }
36-304: Overall excellent implementation with room for minor improvements.The
HasuraMetadataApiclass is well-structured and follows TypeScript best practices. The implementation is solid with proper error handling, type safety, and documentation.Consider the suggested improvements for enhanced robustness:
- Add retry mechanism for network errors
- Implement specific error types
- Add input validation in constructor
But these are optimizations on top of an already well-implemented service.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/hasura/src/configure-hasura.script.ts(1 hunks)scripts/hasura/src/services/hasura.api.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/hasura/src/configure-hasura.script.ts
🧰 Additional context used
📓 Path-based instructions (1)
scripts/hasura/src/services/hasura.api.ts (3)
Pattern **/*.ts: Review TypeScript files for adherence to the following guidelines:
- Avoid over-abstraction; prioritize composition over inheritance.
- Use dependency injection and follow SOLID principles.
- Avoid any; use unknown when necessary.
- Use runtime type-checking for environment variables (e.g., Zod).
- Prevent circular dependencies with the internal module pattern.
- Libraries should have an external.ts file explicitly listing public exports.
- Use bigint as-is; cast to Number only when exposing values via APIs.
- Document all code with JSDoc.
- Encourage static async factory functions for constructors.
- Avoid overly nitpicky feedback beyond these best practices.
Pattern **/services/**/*.ts: Review service files with the following considerations:
- A Service encapsulates a broader business workflow and might orchestrate various components or interact with multiple data sources or APIs.
- Ensure proper composability: Services should use Providers for data/resource access and apply business/orchestration logic.
- Validate naming conventions for aggregating services (e.g., AggregatorService or domain-specific names like MetricsService).
- path: "/providers//*.ts"
instructions: |
Review provider files for the following:
- Providers should supply narrowly scoped data/resources.
- Ensure classes interacting with metadata sources (e.g., GitHub, JSON files, IPFS) implement the IMetadataProvider interface and follow naming conventions (e.g., GithubProvider, JsonFileProvider).
- Be concise and avoid overly nitpicky feedback outside of these best practices.
Pattern scripts/**/*.ts: Ensure scripts:
- Use process.cwd() for root references.
- Follow folder conventions (infra/ for infra scripts, utilities/ for utilities).
- Are organized in package.json with script:infra:{name} or script:util:{name}.
- Be concise and avoid overly nitpicky feedback outside of these best practices.
🔇 Additional comments (1)
scripts/hasura/src/services/hasura.api.ts (1)
1-25: LGTM! Well-structured imports and type definitions.The code follows TypeScript best practices with:
- Proper use of the internal module pattern
- Well-defined types with clear structure
- Appropriate use of built-in and external dependencies
| column_mapping: Object.fromEntries( | ||
| relationship.from.columns.map((col, i) => [ | ||
| col, | ||
| relationship.to.columns[i]!, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove non-null assertion operator.
The non-null assertion operator (!) in relationship.to.columns[i]! should be replaced with proper runtime validation.
-relationship.to.columns[i]!,
+const toColumn = relationship.to.columns[i];
+if (!toColumn) throw new Error(`Missing column mapping at index ${i}`);
+toColumn,Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
the assert check that goes before guarantees that length of both arrays is the same, however TS stills complies that's why the non-null assertion is still in place
There was a problem hiding this comment.
@0xnigir1 You're absolutely right. Since the assert statement guarantees that both arrays have the same length at runtime, using the non-null assertion operator here is a valid approach to handle TypeScript's type inference limitation. I'll retract my suggestion.
✏️ Learnings added
Learnt from: 0xnigir1
PR: defi-wonderland/grants-stack-indexer-v2#59
File: scripts/hasura/src/services/hasura.api.ts:156-156
Timestamp: 2025-01-26T23:41:22.471Z
Learning: When arrays are guaranteed to have the same length through runtime checks (e.g., using `assert`), it's acceptable to use the non-null assertion operator (`!`) when accessing array elements by index, as TypeScript cannot infer that the index is within bounds even with the length check.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🤖 Linear
Closes GIT-243
Description
We create a script to automatically track tables, relationships and functions on Hasura
Checklist before requesting a review
Summary by CodeRabbit
New Documentation
New Package
@grants-stack-indexer/hasura-config-scriptspackageNew Features
Development Improvements