Conversation
|
Caution Review failedThe pull request is closed. WalkthroughA fixed Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
internal/generator/typescript/generator.go(2 hunks)
🔇 Additional comments (1)
internal/generator/typescript/generator.go (1)
119-119: Explicitly setting the default FCL computation limit is safeThe added
limit: 9999simply mirrors FCL’s default maximum computation steps for script queries (9999), so existing clients won’t see any behavior change. Explicitly including it in the generated code improves clarity—developers can still override the limit when higher resources are needed.
| arg({{.Name}}, {{getFCLType .TypeStr}}), | ||
| {{- end}} | ||
| ], | ||
| limit: 9999, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make the limit configurable and document its purpose.
The hardcoded limit: 9999 lacks flexibility and documentation. Consider:
- Make it configurable: Allow users to specify different limits for different scripts
- Add documentation: Explain what this limit controls (execution units, gas, etc.)
- Provide sensible defaults: Research appropriate default values for different script types
- limit: 9999,
+ limit: {{if $func.Limit}}{{$func.Limit}}{{else}}9999{{end}},Also consider adding a field to the TypeScriptFunction struct to support per-function limits:
type TypeScriptFunction struct {
Name string
Parameters []TypeScriptParameter
ReturnType string
Base64 string
Type string
Limit *int // Optional limit override
}🤖 Prompt for AI Agents
In internal/generator/typescript/generator.go at line 119, the limit value is
hardcoded to 9999, which reduces flexibility and lacks documentation. To fix
this, add a configurable limit field to the TypeScriptFunction struct to allow
per-function limit overrides, update the code to use this configurable limit
instead of the hardcoded value, and add comments explaining what the limit
controls (e.g., execution units or gas). Also, set a sensible default limit
value when none is provided.
| arg({{.Name}}, {{getFCLType .TypeStr}}), | ||
| {{- end}} | ||
| ], | ||
| limit: 9999, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider different limits for transactions vs queries.
Using the same hardcoded limit for both transactions and queries may not be optimal. Transactions typically have different resource requirements and cost implications.
Consider:
- Separate configuration: Use different default limits for transactions vs queries
- Research-based defaults: Investigate appropriate limits based on Flow network specifications
- Runtime configurability: Allow limits to be set at the service level
- limit: 9999,
+ limit: {{if $func.Limit}}{{$func.Limit}}{{else}}{{if eq $func.Type "transaction"}}15000{{else}}5000{{end}}{{end}},This example uses different defaults but still allows per-function overrides.
📝 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.
| limit: 9999, | |
| limit: {{if $func.Limit}}{{$func.Limit}}{{else}}{{if eq $func.Type "transaction"}}15000{{else}}5000{{end}}{{end}}, |
🤖 Prompt for AI Agents
In internal/generator/typescript/generator.go at line 135, the current hardcoded
limit of 9999 is used for both transactions and queries, which is not optimal.
Refactor the code to define separate default limits for transactions and
queries, ideally based on Flow network specifications. Implement these as
configurable parameters at the service level to allow overrides per function,
ensuring flexibility and better resource management.
fix: allow same version in npm publish workflow
2 participants
Summary by CodeRabbit