fix: update get-starknet-core version #372
Conversation
WalkthroughThe pull request involves updating the version specification for the Changes
Possibly related issues
Possibly related PRs
Poem
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 (1)
pnpm-workspace.yaml (1)
7-7: Consider adding changeset for this updateThis version bump might affect consumers of your package. Consider adding a changeset to document the change and its impact.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pnpm-workspace.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: check
🔇 Additional comments (2)
pnpm-workspace.yaml (2)
7-9: Update documentation to reflect version changesSince this is a major version update, please ensure:
- Documentation is updated to reflect any breaking changes
- Examples are tested with the new version
- Migration steps are documented if needed
This aligns with your PR checklist items for documentation updates.
7-7: Verify compatibility and migration requirements for get-starknet-core v4The update from v3 to v4 represents a major version bump which may include breaking changes.
Let's verify the compatibility and migration requirements:
rsodre
force-pushed
the
get-starknet-core-version
branch
from
926ba0a to
df4b3e4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
packages/create-burner/src/connectors/dojoBurnerSWO.ts (2)
Line range hint
44-44: Remove @ts-ignore annotations by implementing proper types.Multiple @ts-ignore annotations suggest incomplete type safety. Consider implementing proper types for:
- enable() parameters
- event handlers (on/off methods)
This will improve type safety and maintainability.
Also applies to: 77-77, 84-84
Implement request method with minimum required functionality
The request method should handle at least the
wallet_requestAccountscall type and provide proper error handling for unsupported operations. Here's what needs to be implemented:async request(call: any) { switch (call.type) { case "wallet_requestAccounts": { return [this._account.address]; } default: throw new Error( `DojoBurnerStarknetWindowObject: request not implemented [${call.type}]` ); } }🔗 Analysis chain
Line range hint
39-41: Implement the request method according to StarknetWindowObject interface.The empty request method with @ts-ignore suggests incomplete implementation. This method is crucial for wallet functionality.
Let's verify the required implementation:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for request method implementations in similar files rg -A 10 "async request\(" --type typescript # Find StarknetWindowObject interface definition ast-grep --pattern 'interface StarknetWindowObject { $$$ request($$$) { $$$ } $$$ }'Length of output: 180
Script:
#!/bin/bash # Search for StarknetWindowObject interface definition and request method rg -t ts "interface StarknetWindowObject" -A 20 # Search for request method implementations rg -t ts "async request\(" -A 10 # Search in potential starknet-related files fd -e ts -e js starknet --exec rg -l "request.*\("Length of output: 2899
Script:
#!/bin/bash # Search for request call types rg -t ts "case.*wallet_" -A 2 # Search for any other files importing or using StarknetWindowObject rg -t ts "StarknetWindowObject"Length of output: 5115
🧹 Nitpick comments (1)
packages/create-burner/src/connectors/dojoPredeployedSWO.ts (1)
Line range hint
39-41: Consider extracting common wallet functionality.Both DojoPredeployedStarknetWindowObject and DojoBurnerStarknetWindowObject share similar patterns:
- Empty request implementation
- @ts-ignore annotations
- Similar event handling
Consider creating a base class to reduce code duplication.
Example approach:
abstract class BaseDojoWindowObject implements StarknetWindowObject { // Common properties and implementations abstract request(call: any): Promise<any>; // Other shared methods } class DojoBurnerStarknetWindowObject extends BaseDojoWindowObject { // Specific implementations } class DojoPredeployedStarknetWindowObject extends BaseDojoWindowObject { // Specific implementations }Also applies to: 44-44, 84-84, 91-91
📜 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 (7)
examples/example-vite-react-sql/package.json(1 hunks)packages/create-burner/package.json(1 hunks)packages/create-burner/src/connectors/burner.ts(3 hunks)packages/create-burner/src/connectors/dojoBurnerSWO.ts(2 hunks)packages/create-burner/src/connectors/dojoPredeployedSWO.ts(2 hunks)packages/react/package.json(1 hunks)pnpm-workspace.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pnpm-workspace.yaml
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: check
🔇 Additional comments (6)
packages/react/package.json (1)
44-44: Verify compatibility with peer dependencies after version update.The change to use "catalog:" for get-starknet-core aligns with workspace-level version management. However, since this is a major version update (v3 to v4), please ensure compatibility with the peer dependency 'starknet' which is also using "catalog:".
Run this script to check version compatibility:
✅ Verification successful
Dependencies are properly configured with workspace catalog
The workspace catalog ensures compatibility by managing get-starknet-core (^4.0.0) and starknet (6.21.0) versions centrally. The use of "catalog:" references maintains consistent versioning across the workspace.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check peer dependencies compatibility jq -r '.peerDependencies | to_entries[] | .key + ": " + .value' packages/react/package.json echo "Workspace versions:" cat pnpm-workspace.yamlLength of output: 391
packages/create-burner/package.json (1)
45-47: Verify version compatibility between @starknet-react/core and get-starknet-core.Both dependencies are now using "catalog:" which aligns with workspace-level version management. Since @starknet-react/core requires get-starknet-core ^4.0.0, this change supports the PR objective.
Run this script to verify version compatibility:
✅ Verification successful
Version compatibility between dependencies is correct
The change to use "catalog:" for both @starknet-react/core and get-starknet-core is safe as @starknet-react/core requires get-starknet-core ^4.0.0 as a peer dependency, which will be properly managed at the workspace level.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check dependencies in @starknet-react/core echo "Checking @starknet-react/core dependencies:" npm view @starknet-react/core dependencies --json | jq '."get-starknet-core"'Length of output: 181
Script:
#!/bin/bash # Check peer dependencies echo "Checking @starknet-react/core peer dependencies:" npm view @starknet-react/core peerDependencies --json echo -e "\nChecking local package files:" fd "package.json" --exec cat {} \; | jq -c 'select(.name == "@starknet-react/core" or .name == "get-starknet-core") | {name: .name, version: .version, dependencies, peerDependencies}'Length of output: 505
examples/example-vite-react-sql/package.json (1)
28-28: LGTM! TypeScript dependency added appropriately.Adding TypeScript as a devDependency is correct since the project uses TypeScript (as evidenced by the build script using tsc). The version ^5.6.2 is recent and stable.
packages/create-burner/src/connectors/dojoBurnerSWO.ts (1)
1-1: LGTM: Interface update aligns with get-starknet-core v4.The change from IStarknetWindowObject to StarknetWindowObject correctly implements the new interface from get-starknet-core v4.
Also applies to: 11-11
packages/create-burner/src/connectors/dojoPredeployedSWO.ts (1)
1-1: LGTM: Interface update matches dojoBurnerSWO.ts.The change to StarknetWindowObject is consistent with the other file.
Also applies to: 11-12
packages/create-burner/src/connectors/burner.ts (1)
2-2: LGTM! Type definition aligned with get-starknet-core.The update to use
StarknetWindowObject["icon"]ensures type consistency with the core library.Also applies to: 20-21
| async request(call: any) { | ||
| switch (call.type) { | ||
| case "wallet_requestAccounts": { | ||
| return [this._account.address]; | ||
| } | ||
| default: | ||
| throw new Error( | ||
| `BurnerConnector: request not implemented [${call.type}]` | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety and documentation of the request method.
Several improvements are recommended:
- Replace
anywith proper request type fromget-starknet-core. - Add JSDoc documentation explaining the method's purpose and parameters.
- Consider implementing other standard wallet request types.
Apply this diff to improve the implementation:
+ /**
+ * Handles wallet requests as per the Starknet wallet standard.
+ * @param {StarknetWindowObject['request']['parameters'][0]} call - The request parameters
+ * @returns {Promise<any>} The request result
+ */
- async request(call: any) {
+ async request(call: StarknetWindowObject['request']['parameters'][0]) {
switch (call.type) {
case "wallet_requestAccounts": {
return [this._account.address];
}
+ case "wallet_switchChain": {
+ // TODO: Implement chain switching
+ throw new Error("Chain switching not implemented");
+ }
default:
throw new Error(
- `BurnerConnector: request not implemented [${call.type}]`
+ `BurnerConnector: The request type "${call.type}" is not supported. ` +
+ `Supported types: wallet_requestAccounts`
);
}
}Committable suggestion skipped: line range outside the PR's diff.
💡 Codebase verification
Implement missing wallet standard methods
The BurnerConnector is missing several standard wallet methods that are implemented in the predeployed connector. At minimum, implement:
wallet_getPermissions: Required for permission managementwallet_requestChainId: Required for network identificationwallet_addInvokeTransaction: Required for transaction executionwallet_signTypedData: Required for message signing
🔗 Analysis chain
Verify implementation against Starknet wallet standard.
Let's verify if we're implementing all required wallet methods.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for wallet method implementations in get-starknet-core
rg -A 5 'wallet_[a-zA-Z]+' $(fd -t f -e ts -e js get-starknet-core)
# Search for other burner wallet implementations for comparison
rg -A 5 'class.*extends.*Connector.*{' $(fd -t f -e ts)
Length of output: 19212
Script:
#!/bin/bash
# Search for wallet interface definitions
ast-grep --pattern 'interface $_ {
$$$
wallet_$$$
$$$
}'
# Search for any type definitions related to wallet methods
rg -A 5 'type.*Wallet'
Length of output: 564
Script:
#!/bin/bash
# Find connector/provider interface files
fd -t f -e ts "(connector|provider|interface)"
# Search for interface definitions in those files
rg -A 10 "interface.*Connector|interface.*Provider|interface.*Wallet"
# Look for documentation files
fd -t f "README|SPEC|standard" -e md
Length of output: 3115
Script:
#!/bin/bash
# Search for Starknet-related files
fd -t f -e ts "starknet|wallet"
# Search for Starknet-related types and interfaces
rg -A 10 "type.*Starknet|interface.*Starknet|enum.*Starknet"
# Search for wallet method constants or types
rg -A 5 "const.*WALLET_|type.*WALLET_|enum.*Wallet"
Length of output: 889
Introduced changes
get-starknet-core: ^4.0.0This is the version used by @starknet-react/core v.3.6.2
Checklist
Summary by CodeRabbit
Summary by CodeRabbit
get-starknet-corepackage to version 4.0.0