Moved subgraph config to env variables (except codegen) #155
Conversation
…d node and yarn version
WalkthroughThis pull request introduces several changes aimed at enhancing the project's configuration and environment management. Key updates include the addition of Yarn-related entries in the Changes
Possibly related PRs
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
warning eslint-config-next > @next/eslint-plugin-next > [email protected]: Glob versions prior to v9 are no longer supported 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 (
|
✅ Deploy Preview for proof-of-humanity-v2 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
src/config/subgraph.ts (1)
Line range hint
20-24: Add error handling for GraphQL client initializationThe GraphQL clients are created immediately without any error handling. Consider wrapping the client creation in a try-catch block or implementing lazy initialization.
Here's a suggested implementation:
-export const sdk = { - [mainnet.id]: getSdk(new GraphQLClient(subgraph_url[mainnet.id])), - [gnosis.id]: getSdk(new GraphQLClient(subgraph_url[gnosis.id])), - [sepolia.id]: getSdk(new GraphQLClient(subgraph_url[sepolia.id])), - [gnosisChiado.id]: getSdk(new GraphQLClient(subgraph_url[gnosisChiado.id])), -}; +export const sdk = Object.fromEntries( + [mainnet.id, gnosis.id, sepolia.id, gnosisChiado.id].map(chainId => { + try { + return [chainId, getSdk(new GraphQLClient(subgraph_url[chainId]))]; + } catch (error) { + console.error(`Failed to initialize GraphQL client for chain ${chainId}:`, error); + throw error; + } + }) +) as Record<SupportedChainId, sdkReturnType>;next.config.js (1)
Line range hint
39-45: Security & Quality Concern: Build errors being ignoredThe configuration is currently set to ignore both ESLint and TypeScript errors during builds. This can lead to:
- Potential bugs reaching production
- Type-safety guarantees being bypassed
- Code quality issues being overlooked
Consider:
- Addressing and fixing existing errors
- Setting up pre-commit hooks to catch issues early
- Making the build process stricter for production deployments
Would you like assistance in setting up a more robust build configuration that maintains both development velocity and code quality?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
.yarn/releases/yarn-1.22.22.cjsis excluded by!**/.yarn/**yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
.gitignore(1 hunks).yarnrc(1 hunks)codegen.yml(1 hunks)env.d.ts(1 hunks)next.config.js(1 hunks)package.json(1 hunks)src/config/subgraph.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .yarnrc
🔇 Additional comments (6)
codegen.yml (1)
2-2: Verify the new GraphQL service endpoint
The schema URL has changed significantly:
- Different organization ID: from 64099 to 61738
- Different project name: from 'proof-of-humanity-mainnet' to 'poh-origin-mainnet'
- Using 'latest' version tag which could lead to inconsistent builds
Let's verify the endpoint and its accessibility:
✅ Verification successful
GraphQL endpoint change appears valid and properly configured
The endpoint change is legitimate and well-integrated:
- The new endpoint is accessible and returns a valid GraphQL schema with PoH-specific types
- The endpoint URL is only referenced in
codegen.ymland used for code generation - Runtime GraphQL queries use environment variables (
MAINNET_SUBGRAPH_URL) for flexibility - The project's dependencies include proper GraphQL tooling and code generation setup
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the GraphQL endpoint accessibility and schema
# Expected results: 200 OK response and valid GraphQL schema
# Test endpoint accessibility
curl -s -o /dev/null -w "%{http_code}" "https://api.studio.thegraph.com/query/61738/poh-origin-mainnet/version/latest"
# Fetch and validate schema
curl -X POST -H "Content-Type: application/json" \
-d '{"query":"query { __schema { types { name } } }"}' \
"https://api.studio.thegraph.com/query/61738/poh-origin-mainnet/version/latest"
# Search for any references to the old endpoint
rg -g '!codegen.yml' "64099/proof-of-humanity-mainnet"
Length of output: 3063
Script:
#!/bin/bash
# Let's check for any references to both old and new endpoints in the codebase
# and look for any configuration or documentation files that might provide context
# Search for any references to both old and new subgraph names
rg -l "proof-of-humanity-mainnet|poh-origin-mainnet"
# Search for GraphQL related configuration files
fd -e graphql -e gql -e config.js -e config.ts
# Check package.json and other common config files for graph-related dependencies
fd -e json -e yaml -e yml | xargs rg -l "graph|graphql"
# Look for documentation about the GraphQL endpoint
fd "README|CONTRIBUTING|DEVELOPMENT" | xargs rg -l "graph|graphql"
Length of output: 1234
Script:
#!/bin/bash
# Let's examine the subgraph configuration and its usage
# Check the subgraph configuration
cat src/config/subgraph.ts
# Look for any version pinning or endpoint configuration
cat package.json
# Check if there are any migration scripts or documentation about the endpoint change
rg -l "61738"
# Examine the GraphQL codegen configuration in detail
cat codegen.yml
Length of output: 4062
env.d.ts (1)
8-11: LGTM! The environment variable declarations look good.
The new subgraph URL environment variables follow consistent naming patterns and maintain parity with existing RPC configurations across all networks.
Let's verify the usage of these new environment variables:
✅ Verification successful
Environment variables are properly used and no hardcoded subgraph URLs found
The verification shows that:
- All new environment variables are correctly used in
next.config.jsfor client-side access - The variables are properly mapped to network IDs in
src/config/subgraph.ts - No hardcoded subgraph URLs were found in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the new environment variables are properly used and that no hardcoded subgraph URLs remain
# Check for any remaining hardcoded subgraph URLs
rg -i "subgraph.*\.(io|com|org)" --type ts --type js
# Check the usage of the new environment variables
rg "process\.env\.(MAINNET|GNOSIS|CHIADO|SEPOLIA)_SUBGRAPH_URL" --type ts --type js
Length of output: 730
.gitignore (2)
39-39: LGTM: Proper section separation
The empty line provides clear visual separation between different ignore rule sections.
40-47: LGTM: Standard Yarn 2+ Zero-Install configuration
The Yarn ignore rules follow the official best practices for Yarn 2+ (Berry) with Zero-Install configuration. This setup will:
- Improve CI/CD reliability by committing the Yarn release
- Ensure consistent tooling across the team
- Support proper editor integration via SDKs
Let's verify the Yarn 2+ migration is complete:
src/config/subgraph.ts (1)
Line range hint 1-24: Verify environment variable configuration
Let's verify that all required environment variables are properly configured in the project.
✅ Verification successful
Environment variables are properly configured
The verification shows that all required subgraph URL environment variables are properly configured in both env.d.ts (type definitions) and next.config.js (runtime configuration). All four required variables (MAINNET_SUBGRAPH_URL, GNOSIS_SUBGRAPH_URL, SEPOLIA_SUBGRAPH_URL, and CHIADO_SUBGRAPH_URL) are present and correctly typed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for environment variable configuration in relevant files
echo "Checking .env.example file..."
if [ -f ".env.example" ]; then
echo "Contents of .env.example:"
grep -E "MAINNET_SUBGRAPH_URL|GNOSIS_SUBGRAPH_URL|SEPOLIA_SUBGRAPH_URL|CHIADO_SUBGRAPH_URL" .env.example || echo "No subgraph URLs found in .env.example"
fi
echo -e "\nChecking env.d.ts file..."
if [ -f "env.d.ts" ]; then
echo "Environment variable types in env.d.ts:"
grep -E "MAINNET_SUBGRAPH_URL|GNOSIS_SUBGRAPH_URL|SEPOLIA_SUBGRAPH_URL|CHIADO_SUBGRAPH_URL" env.d.ts || echo "No subgraph URL types found in env.d.ts"
fi
echo -e "\nChecking next.config.js file..."
if [ -f "next.config.js" ]; then
echo "Environment variable configuration in next.config.js:"
grep -E "MAINNET_SUBGRAPH_URL|GNOSIS_SUBGRAPH_URL|SEPOLIA_SUBGRAPH_URL|CHIADO_SUBGRAPH_URL" next.config.js || echo "No subgraph URLs found in next.config.js"
fi
Length of output: 1086
package.json (1)
86-90: LGTM: Version pinning implementation
The explicit version pinning for Yarn and the package manager specification with SHA hash ensures reproducible builds. The Yarn version 1.22.22 is the latest stable version in the 1.x series.
Let's verify the Yarn version and SHA:
| @@ -1,6 +1,5 @@ | |||
| overwrite: true | |||
| #schema: "https://api.studio.thegraph.com/query/64099/proof-of-humanity-sepolia/version/latest" | |||
| schema: "https://api.studio.thegraph.com/query/64099/proof-of-humanity-mainnet/version/latest" | |||
| schema: "https://api.studio.thegraph.com/query/61738/poh-origin-mainnet/version/latest" | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Schema URL should use environment variables
The schema URL is still hardcoded despite the PR's objective to move subgraph configuration to environment variables. This could make it difficult to switch between different environments (mainnet, testnet, etc.).
Consider using the environment variables that were added in this PR:
-schema: "https://api.studio.thegraph.com/query/61738/poh-origin-mainnet/version/latest"
+schema: ${MAINNET_SUBGRAPH_URL}📝 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.
| schema: "https://api.studio.thegraph.com/query/61738/poh-origin-mainnet/version/latest" | |
| schema: ${MAINNET_SUBGRAPH_URL} |
| [mainnet.id]: process.env.MAINNET_SUBGRAPH_URL, | ||
| [gnosis.id]: process.env.GNOSIS_SUBGRAPH_URL, | ||
| [sepolia.id]: process.env.SEPOLIA_SUBGRAPH_URL, | ||
| [gnosisChiado.id]: process.env.CHIADO_SUBGRAPH_URL, |
There was a problem hiding this comment.
Add validation for environment variables
The direct use of environment variables without validation could lead to runtime errors if any of these variables are undefined. Consider adding validation and providing meaningful error messages.
Here's a suggested implementation:
export const subgraph_url = {
- [mainnet.id]: process.env.MAINNET_SUBGRAPH_URL,
- [gnosis.id]: process.env.GNOSIS_SUBGRAPH_URL,
- [sepolia.id]: process.env.SEPOLIA_SUBGRAPH_URL,
- [gnosisChiado.id]: process.env.CHIADO_SUBGRAPH_URL,
+ [mainnet.id]: validateSubgraphUrl('MAINNET_SUBGRAPH_URL', process.env.MAINNET_SUBGRAPH_URL),
+ [gnosis.id]: validateSubgraphUrl('GNOSIS_SUBGRAPH_URL', process.env.GNOSIS_SUBGRAPH_URL),
+ [sepolia.id]: validateSubgraphUrl('SEPOLIA_SUBGRAPH_URL', process.env.SEPOLIA_SUBGRAPH_URL),
+ [gnosisChiado.id]: validateSubgraphUrl('CHIADO_SUBGRAPH_URL', process.env.CHIADO_SUBGRAPH_URL),
};
function validateSubgraphUrl(name: string, url: string | undefined): string {
if (!url) {
throw new Error(`${name} environment variable is not defined`);
}
try {
new URL(url);
return url;
} catch {
throw new Error(`${name} environment variable contains an invalid URL: ${url}`);
}
}Committable suggestion skipped: line range outside the PR's diff.
| MAINNET_SUBGRAPH_URL: process.env.MAINNET_SUBGRAPH_URL, | ||
| GNOSIS_SUBGRAPH_URL: process.env.GNOSIS_SUBGRAPH_URL, | ||
| CHIADO_SUBGRAPH_URL: process.env.CHIADO_SUBGRAPH_URL, | ||
| SEPOLIA_SUBGRAPH_URL: process.env.SEPOLIA_SUBGRAPH_URL, |
There was a problem hiding this comment.
💡 Codebase verification
Environment variables need documentation
The newly added subgraph URL environment variables (MAINNET_SUBGRAPH_URL, GNOSIS_SUBGRAPH_URL, CHIADO_SUBGRAPH_URL, SEPOLIA_SUBGRAPH_URL) lack documentation. Please:
- Create a
.env.examplefile listing these variables with sample values - Document the variables in the README.md or a dedicated environment setup guide
- Include any specific requirements or format expectations for the URLs
🔗 Analysis chain
LGTM! Verify environment variable documentation.
The addition of network-specific subgraph URL environment variables aligns with the PR objective and follows good practices for configuration management.
Let's verify the environment variables are properly documented:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for environment variable documentation
# Look for .env.example, README, or documentation files that should list these variables
# Search for documentation files
echo "Searching for environment documentation files..."
fd -e md -e example --exec grep -l "SUBGRAPH_URL" {} \;
fd ".env" --exec grep -l "SUBGRAPH_URL" {} \;
Length of output: 212
Script:
#!/bin/bash
# Let's try a broader search to find any potential documentation
# First, let's check for .env files with different extensions
fd ".env"
# Then, let's look for any files containing "env" in their name or content
fd -t f -e md -e txt | xargs grep -l -i "env" 2>/dev/null
# Finally, let's check the README for any configuration documentation
if [ -f "README.md" ]; then
echo "=== README.md content ==="
cat README.md
fi
Length of output: 78
| "packageManager": "[email protected]+sha1.ac34549e6aa8e7ead463a7407e1c7390f61a6610", | ||
| "volta": { | ||
| "node": "18.20.5", |
There was a problem hiding this comment.
Fix invalid Node.js version
The specified Node.js version 18.20.5 does not exist. The latest LTS version in the v18 series is 18.19.0.
Apply this diff to fix the version:
- "node": "18.20.5",
+ "node": "18.19.0",📝 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.
| "node": "18.20.5", | |
| "node": "18.19.0", |
Also pinned node and yarn version
Summary by CodeRabbit
New Features
.yarnrcfile to specify the Yarn version in use.Bug Fixes
Chores
.gitignorefile to manage Yarn-related files effectively.package.json.