-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: bump controller version + use controller presets #2430
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request includes updates to several files, primarily focusing on dependency version increments in Changes
Sequence Diagram(s)sequenceDiagram
participant Env as Environment Variable
participant Provider as Starknet Provider
participant Connector as ControllerConnector
Env->>Provider: Check env.VITE_PUBLIC_CHAIN
alt Mainnet
Provider->>Connector: Instantiate without policies
else Other Chains
Provider->>Connector: Instantiate with policies (signingPolicy, policies, vrfPolicy)
end
Connector->>Provider: Return controller instance
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 (
|
You are out of MentatBot reviews. Your usage will refresh December 16 at 08:00 AM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
client/src/hooks/context/starknet-provider.tsx (1)
23-46
: Refactor to reduce configuration duplicationThe controller instantiation contains duplicated configuration. Consider extracting common properties into a base configuration object.
const controller = + const baseConfig = { + rpc: env.VITE_PUBLIC_NODE_URL, + namespace, + slot, + preset, + tokens: { + erc20: ["0x0124aeb495b947201f5fac96fd1138e326ad86195b98df6dec9009158a533b49"], + }, + colorMode, + }; + env.VITE_PUBLIC_CHAIN === "mainnet" - ? new ControllerConnector({ - rpc: env.VITE_PUBLIC_NODE_URL, - namespace, - slot, - preset, - tokens: { - erc20: ["0x0124aeb495b947201f5fac96fd1138e326ad86195b98df6dec9009158a533b49"], - }, - colorMode, - }) - : new ControllerConnector({ - rpc: env.VITE_PUBLIC_NODE_URL, - namespace, - slot, - preset, - policies: [...signingPolicy, ...policies, vrfPolicy], - theme, - tokens: { - erc20: ["0x0124aeb495b947201f5fac96fd1138e326ad86195b98df6dec9009158a533b49"], - }, - colorMode, - }); + ? new ControllerConnector(baseConfig) + : new ControllerConnector({ + ...baseConfig, + policies: [...signingPolicy, ...policies, vrfPolicy], + theme, + });Consider moving the hardcoded token address to a constant or environment variable
The ERC20 token address is hardcoded. Consider moving it to a constant or environment variable for better maintainability.
+const ERC20_TOKEN_ADDRESS = "0x0124aeb495b947201f5fac96fd1138e326ad86195b98df6dec9009158a533b49"; + const baseConfig = { // ... tokens: { - erc20: ["0x0124aeb495b947201f5fac96fd1138e326ad86195b98df6dec9009158a533b49"], + erc20: [ERC20_TOKEN_ADDRESS], }, // ... };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
client/package.json
(2 hunks)client/src/hooks/context/starknet-provider.tsx
(2 hunks)package.json
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🔇 Additional comments (2)
client/src/hooks/context/starknet-provider.tsx (1)
11-11
: 🛠️ Refactor suggestion
Consider removing the redundant theme
constant
The newly added preset
constant appears to serve the same purpose as the existing theme
constant, both set to "eternum". Since theme
is only used in the non-mainnet configuration while preset
is used in both configurations, this suggests a transition from theme
to preset
.
client/package.json (1)
20-21
: Verify compatibility with the updated cartridge packages
The version updates from 0.5.4 to 0.5.5 for both @Cartridge packages should be verified for:
- Breaking changes
- Latest stable versions
- Compatibility between the packages
✅ Verification successful
Let me try to gather more information about these packages by checking the repository for any documentation or changelog files.
Let me try one more search with corrected commands to find any relevant information.
Based on the search results, I can now provide a final response:
Version updates for cartridge packages are appropriate
The version updates to 0.5.5 for both @cartridge/connector and @cartridge/controller are verified to be:
- The latest stable versions available on npm
- Compatible with each other, as evidenced by their paired usage in pnpm-lock.yaml
- A minor version bump (0.5.4 -> 0.5.5) suggesting no breaking changes
- Already in use together in other parts of the monorepo (as seen in landing package)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for latest versions and changelogs
for pkg in "@cartridge/connector" "@cartridge/controller"; do
echo "Checking $pkg..."
# Get latest version from npm
npm view "$pkg" version
# Get changelog if available
npm view "$pkg" changelog
done
Length of output: 449
Script:
#!/bin/bash
# Search for changelog or documentation files related to cartridge packages
fd -i -g 'changelog*' -g 'cartridge*' -g 'packages*' -g 'readme*'
# Check package-lock.json for dependency details
if [ -f package-lock.json ]; then
echo "Checking package-lock.json for cartridge dependencies..."
jq '.dependencies | with_entries(select(.key | contains("@cartridge")))' package-lock.json
fi
# Look for any documentation mentioning cartridge versions
rg -i "cartridge.*0\.5\.[45]" --type md --type txt
Length of output: 359
Script:
#!/bin/bash
# Search for files with proper fd syntax
fd -e md -e txt -e json
# Check if package-lock.json exists in client directory
if [ -f client/package-lock.json ]; then
echo "Checking client/package-lock.json for cartridge dependencies..."
jq '.dependencies | with_entries(select(.key | contains("@cartridge")))' client/package-lock.json
fi
# Look for any documentation mentioning cartridge packages
rg "@cartridge/(connector|controller)" -A 2 -B 2
Length of output: 9032
Failed to generate code suggestions for PR |
Summary by CodeRabbit
New Features
@cartridge/connector
and@cartridge/controller
.preset
for theControllerConnector
.ControllerConnector
based on the environment.Chores
package.json
.