Make HyperIndex work without pnpm workspace #578
Conversation
|
Warning Rate limit exceeded@DZakh has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 15 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
WalkthroughThis update centralizes version validation and retrieval logic, modifies how dependencies and workspace configurations are managed, and removes several package manager configuration files. It introduces a new method for handling the "envio" version, updates template code and generated files, and adjusts extraction logic to skip Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant SystemConfig
participant Templates
User->>CLI: Run codegen/init command
CLI->>SystemConfig: get_envio_version()
SystemConfig-->>CLI: Returns validated envio version string
CLI->>Templates: Pass envio_version to ProjectTemplate
Templates-->>CLI: Generate files with envio_version
CLI->>CLI: Run pnpm install in generated and project_root
Suggested labels
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (1)
codegenerator/cli/src/config_parsing/system_config.rs (1)
392-398: Consider making the local development path construction more robust.The current implementation assumes the executable is always 3 directories deep from
cli/npm/envio. This could break if the directory structure changes or if the executable is in a different location (as your comment suggests with "integration_tests").Consider:
- Using a more robust path resolution method
- Checking if the constructed path actually exists before returning it
- Handling the different executable locations mentioned in the comment
- match env::current_exe() { - // This should be something like "file:~/envio/hyperindex/codegenerator/target/debug/envio" or "file:.../target/debug/integration_tests" - Ok(exe_path) => Ok(format!( - "file:{}/../../../cli/npm/envio", - exe_path.to_string_lossy() - )), - Err(e) => Err(anyhow!("failed to get current exe path: {e}")), - } + match env::current_exe() { + Ok(exe_path) => { + // Try to find the cli/npm/envio directory relative to the executable + let mut current_dir = exe_path.parent().ok_or_else(|| anyhow!("executable has no parent directory"))?; + + // Go up the directory tree looking for cli/npm/envio + for _ in 0..5 { // Limit search depth + let potential_path = current_dir.join("cli/npm/envio"); + if potential_path.exists() { + return Ok(format!("file:{}", potential_path.to_string_lossy())); + } + current_dir = match current_dir.parent() { + Some(parent) => parent, + None => break, + }; + } + + // Fallback to the original logic if not found + Ok(format!( + "file:{}/../../../cli/npm/envio", + exe_path.to_string_lossy() + )) + } + Err(e) => Err(anyhow!("failed to get current exe path: {e}")), + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
codegenerator/cli/src/commands.rs(2 hunks)codegenerator/cli/src/config_parsing/system_config.rs(3 hunks)codegenerator/cli/src/executor/init.rs(3 hunks)codegenerator/cli/src/hbs_templating/codegen_templates.rs(4 hunks)codegenerator/cli/templates/dynamic/codegen/package.json.hbs(2 hunks)codegenerator/cli/templates/static/blank_template/javascript/.npmrc(0 hunks)codegenerator/cli/templates/static/blank_template/rescript/.npmrc(0 hunks)codegenerator/cli/templates/static/blank_template/rescript/rescript.json(1 hunks)codegenerator/cli/templates/static/blank_template/shared/pnpm-workspace.yaml(0 hunks)codegenerator/cli/templates/static/blank_template/typescript/.npmrc(0 hunks)codegenerator/cli/templates/static/erc20_template/javascript/.npmrc(0 hunks)codegenerator/cli/templates/static/erc20_template/rescript/.npmrc(0 hunks)codegenerator/cli/templates/static/erc20_template/rescript/rescript.json(1 hunks)codegenerator/cli/templates/static/erc20_template/shared/pnpm-workspace.yaml(0 hunks)codegenerator/cli/templates/static/erc20_template/typescript/.npmrc(0 hunks)codegenerator/cli/templates/static/greeter_template/javascript/.npmrc(0 hunks)codegenerator/cli/templates/static/greeter_template/rescript/.npmrc(0 hunks)codegenerator/cli/templates/static/greeter_template/rescript/rescript.json(1 hunks)codegenerator/cli/templates/static/greeter_template/shared/pnpm-workspace.yaml(0 hunks)codegenerator/cli/templates/static/greeter_template/typescript/.npmrc(0 hunks)codegenerator/cli/templates/static/greeteronfuel_template/javascript/.npmrc(0 hunks)codegenerator/cli/templates/static/greeteronfuel_template/rescript/.npmrc(0 hunks)codegenerator/cli/templates/static/greeteronfuel_template/rescript/rescript.json(1 hunks)codegenerator/cli/templates/static/greeteronfuel_template/shared/pnpm-workspace.yaml(0 hunks)codegenerator/cli/templates/static/greeteronfuel_template/typescript/.npmrc(0 hunks)codegenerator/integration_tests/tests/test_indexers/dynamic_contracts/pnpm-workspace.yaml(0 hunks)codegenerator/integration_tests/tests/test_indexers/test_exits/pnpm-workspace.yaml(0 hunks)codegenerator/integration_tests/tests/test_indexers/wildcard-uni-factory/pnpm-workspace.yaml(0 hunks)scenarios/erc20_multichain_factory/.npmrc(0 hunks)scenarios/erc20_multichain_factory/pnpm-workspace.yaml(0 hunks)scenarios/fuel_test/.npmrc(0 hunks)scenarios/fuel_test/pnpm-workspace.yaml(0 hunks)scenarios/test_codegen/.npmrc(0 hunks)scenarios/test_codegen/pnpm-workspace.yaml(0 hunks)
💤 Files with no reviewable changes (25)
- codegenerator/integration_tests/tests/test_indexers/test_exits/pnpm-workspace.yaml
- scenarios/erc20_multichain_factory/pnpm-workspace.yaml
- codegenerator/cli/templates/static/greeteronfuel_template/shared/pnpm-workspace.yaml
- codegenerator/cli/templates/static/erc20_template/rescript/.npmrc
- scenarios/test_codegen/pnpm-workspace.yaml
- codegenerator/integration_tests/tests/test_indexers/dynamic_contracts/pnpm-workspace.yaml
- codegenerator/cli/templates/static/greeter_template/shared/pnpm-workspace.yaml
- codegenerator/cli/templates/static/greeteronfuel_template/javascript/.npmrc
- scenarios/test_codegen/.npmrc
- scenarios/fuel_test/pnpm-workspace.yaml
- codegenerator/cli/templates/static/greeter_template/rescript/.npmrc
- codegenerator/cli/templates/static/blank_template/rescript/.npmrc
- codegenerator/cli/templates/static/blank_template/shared/pnpm-workspace.yaml
- codegenerator/cli/templates/static/blank_template/javascript/.npmrc
- codegenerator/cli/templates/static/erc20_template/shared/pnpm-workspace.yaml
- codegenerator/cli/templates/static/greeteronfuel_template/rescript/.npmrc
- codegenerator/cli/templates/static/greeter_template/javascript/.npmrc
- scenarios/erc20_multichain_factory/.npmrc
- codegenerator/cli/templates/static/blank_template/typescript/.npmrc
- codegenerator/integration_tests/tests/test_indexers/wildcard-uni-factory/pnpm-workspace.yaml
- codegenerator/cli/templates/static/greeter_template/typescript/.npmrc
- codegenerator/cli/templates/static/erc20_template/javascript/.npmrc
- codegenerator/cli/templates/static/greeteronfuel_template/typescript/.npmrc
- scenarios/fuel_test/.npmrc
- codegenerator/cli/templates/static/erc20_template/typescript/.npmrc
🧰 Additional context used
🧬 Code Graph Analysis (1)
codegenerator/cli/src/hbs_templating/codegen_templates.rs (1)
codegenerator/cli/src/config_parsing/system_config.rs (1)
get_envio_version(383-400)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build_and_test
🔇 Additional comments (15)
codegenerator/cli/templates/static/greeteronfuel_template/rescript/rescript.json (1)
22-22: LGTM! Clean removal of pinned dependencies.The removal of the "pinned-dependencies" field aligns with the PR objective to centralize version management and eliminate pnpm workspace requirements while preserving the necessary ReScript dependencies.
codegenerator/cli/templates/static/blank_template/rescript/rescript.json (1)
22-22: Consistent dependency configuration cleanup.This change is consistent with the removal of "pinned-dependencies" across all ReScript templates, maintaining only the necessary "bs-dependencies" for proper functionality.
codegenerator/cli/templates/static/erc20_template/rescript/rescript.json (1)
22-22: Dependency configuration standardized.The removal of "pinned-dependencies" follows the same pattern as other templates, ensuring consistent dependency management across all ReScript project templates.
codegenerator/cli/templates/static/greeter_template/rescript/rescript.json (1)
22-22: Final template updated consistently.This completes the uniform removal of "pinned-dependencies" across all ReScript templates, ensuring standardized dependency management while preserving required build dependencies.
codegenerator/cli/templates/dynamic/codegen/package.json.hbs (2)
6-6: Appropriately marks generated packages as private.Adding
"private": trueprevents accidental publishing of generated packages, which is the correct approach for code generation templates.
59-59: Centralizes envio version management.The templated envio dependency
"envio": "{{envio_version}}"aligns with the PR's goal to centralize version validation and management, replacing the previous pinned dependency approach with dynamic version injection.codegenerator/cli/src/executor/init.rs (1)
9-12: Clean refactoring to centralize version validation logic.Good job moving the version validation logic to a centralized location in
system_config.rs. This follows the DRY principle and improves maintainability.Also applies to: 232-232
codegenerator/cli/src/commands.rs (2)
119-119: More descriptive progress message.Good improvement - the new message "Generating HyperIndex code..." is more specific than the generic "Building code".
92-108:✅ Verification successful
Verify the two-step pnpm installation approach.
The new installation process first installs in the
generateddirectory with--no-lockfile, then in the project root. This approach should work without pnpm workspaces, but please verify that:
- The
enviodependency (dynamically added to package.json as mentioned in the PR summary) is correctly resolved in both directories- Dependencies are properly shared between the generated and root directories without workspace configuration
🏁 Script executed:
#!/bin/bash # Description: Check for package.json files and their envio dependency configuration # Find all package.json files in the codebase fd -t f "package.json" | head -20 # Check if any package.json.hbs template includes envio dependency fd -t f "package.json.hbs" | xargs grep -l "envio" || echo "No package.json.hbs files with envio dependency found" # Look for the dynamic envio version injection mentioned in PR summary rg -A 3 -B 3 "envio.*version" --type handlebars || rg -A 3 -B 3 "envio.*version" -g "*.hbs"Length of output: 2413
🏁 Script executed:
#!/bin/bash # Show the context around the generated optional dependency in the shared package.json template rg -C3 "{{relative_path_from_root_to_generated}}" -g 'codegenerator/cli/templates/dynamic/init_templates/shared/package.json.hbs'Length of output: 245
🏁 Script executed:
#!/bin/bash # Properly escape braces or use fixed string search to show context for the generated optional dependency rg -F -C3 '{{relative_path_from_root_to_generated}}' -g 'codegenerator/cli/templates/dynamic/init_templates/shared/package.json.hbs'Length of output: 826
pnpm installation and envio dependency handling verified
The two-step install correctly handles the dynamically injected
enviodependency and the generated directory link:
- Both
codegenerator/cli/templates/dynamic/codegen/package.json.hbsand.../init_templates/shared/package.json.hbsinclude
"envio": "{{envio_version}}".- The shared template also adds the generated folder as an optional dependency via
"generated": "{{relative_path_from_root_to_generated}}".With
pnpm install --no-lockfileingeneratedandpnpm installin the root,envioand the localgeneratedpackage will be resolved as intended. No further changes are required.codegenerator/cli/src/config_parsing/system_config.rs (2)
374-381: Version validation logic looks good.The regex pattern correctly validates semantic versions with optional RC suffix and main branch versions. The implementation is clean and well-focused.
1974-2007: Comprehensive unit tests for version validation.Excellent test coverage! The tests cover all important cases including:
- Standard semantic versions
- Versions with RC suffix
- Main branch development versions
- Various invalid formats
codegenerator/cli/src/hbs_templating/codegen_templates.rs (4)
16-17: LGTM: Import addition is correctThe import of
get_envio_versionfromsystem_configis properly added and aligns with the function usage later in the file.
1245-1245: LGTM: Struct field addition is appropriateThe
envio_version: Stringfield addition toProjectTemplatestruct correctly supports the centralized version management approach described in the PR objectives.
1351-1351: LGTM: Correct field initializationThe initialization of
envio_versionusingget_envio_version()?properly calls the centralized function and handles potential errors with the?operator.
841-847:❌ Incorrect review comment
Verify template variable syntax and scope
The string interpolation on line 841 uses
${{Path.relativePathToRootFromGenerated}}which appears to be a template variable, but this syntax is unconventional and potentially problematic:
- The
${{...}}syntax doesn't match standard Handlebars template syntax ({{...}})- It's unclear where
Path.relativePathToRootFromGeneratedis defined or how it gets populated- Mixing template variables with Rust string formatting could lead to confusion
🏁 Script executed:
#!/bin/bash # Search for Path.relativePathToRootFromGenerated usage and definition rg -A 3 -B 3 "Path\.relativePathToRootFromGenerated" # Search for similar template variable patterns rg -A 2 -B 2 '\$\{\{.*\}\}' # Look for template processing of this generated code rg -A 5 "transpileAbi.*require"Length of output: 4315
Ignore incorrect template variable warning
The
${{Path.relativePathToRootFromGenerated}}syntax is intentional: the leading$starts a JavaScript template‐literal placeholder and the doubled curly braces are a Handlebars variable. At runtime the Rustformat!builds this HBS template string, then Handlebars replaces{{Path.relativePathToRootFromGenerated}}with the correct path. You can see the same pattern in:
- codegenerator/cli/templates/dynamic/codegen/src/RegisterHandlers.res.hbs
- codegenerator/cli/templates/static/codegen/src/bindings/Dotenv.res
switch require.resolve(`../../${Path.relativePathToRootFromGenerated}/.env`) {- codegenerator/cli/src/hbs_templating/codegen_templates.rs (line 841)
No change needed—this is by design.
Likely an incorrect or invalid review comment.
| println!("Clean build directory"); | ||
| let exit2 = rescript::clean(&project_paths.generated) | ||
| .await | ||
| .context("Failed running rescript clean")?; | ||
| if !exit2.success() { | ||
| return Ok(exit2); | ||
| } |
There was a problem hiding this comment.
I can't remember what case we were worried about stale build 🤔, I assume it's when a file gets removed from generated (but still exists from a stale generated version)
|
If we can just fix the failing cargo tests. Looks like its an expected fail now |
|
Yes, I've tested several cases, including the upgrade flow from the existing version, and everything appears to be in order. |
What about when a file gets removed from generated from earlier to later version? I think the generated folder gets purged but I can't remember. |
Yes, the generated folder is purged on a version change. |
|
Otherwise it's upserted |
Summary by CodeRabbit
New Features
Improvements
.gitkeepfiles to avoid unnecessary files in new projects.Chores
.npmrcandpnpm-workspace.yamlfiles from templates and scenarios, streamlining workspace and package manager configurations.