feat: add react hook to use predeployed accounts #419
Conversation
WalkthroughThis pull request introduces a new React hook, Changes
Sequence Diagram(s)sequenceDiagram
participant Component as StarknetProvider
participant Hook as usePredeployedAccounts
participant Fetcher as predeployedAccounts
participant Config as StarknetConfig
Component->>Hook: Call usePredeployedAccounts(options)
Hook->>Fetcher: Request predeployed accounts with options
Fetcher-->>Hook: Return connectors data (with retry logic)
Hook-->>Component: Provide connectors array
Component->>Config: Pass connectors to StarknetConfig
Possibly related PRs
Poem
🪧 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: 1
🧹 Nitpick comments (8)
examples/example-vite-react-sdk/package.json (1)
48-49: Version Consistency Check for “vite-plugin-top-level-await”
In this file,"vite-plugin-top-level-await"is added todevDependencieswith version"^1.5.0", which differs from the"^1.4.4"used in the other examples. If this version bump is intentional (perhaps due to new features or improved compatibility in the React SDK), please ensure that it is documented and that all related tooling supports it. Otherwise, consider aligning the version across examples for consistency.packages/predeployed-connector/src/react/index.ts (3)
23-28: Addretriesto the dependency array to prevent stale value issues.The
useEffecthook uses theretriesvariable inside its callback function, butretriesis not included in the dependency array. This could lead to stale values being used and potentially cause unexpected behavior.- }, [connectors, options]); + }, [connectors, options, retries]);However, adding
retriesto the dependency array could lead to an infinite loop, since you're incrementing it inside the effect. Consider refactoring to use the functional update form ofsetRetries:- setRetries(retries + 1); + setRetries(prev => prev + 1);This way, you wouldn't need to include
retriesin the dependency array at all:- }, [connectors, options]); + }, [connectors, options]);🧰 Tools
🪛 Biome (1.9.4)
[error] 23-23: This hook does not specify all of its dependencies: retries
This dependency is not specified in the hook dependency list.
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
23-28: Consider enhancing error handling for failed account fetching.The current implementation only retries up to 5 times without providing feedback if all attempts fail. Consider adding explicit error handling to inform users when account fetching fails completely.
- predeployedAccounts(options).then(setConnectors); + predeployedAccounts(options) + .then(setConnectors) + .catch(error => { + console.error("Failed to fetch predeployed accounts:", error); + // Optionally add a state variable to track and expose the error + });🧰 Tools
🪛 Biome (1.9.4)
[error] 23-23: This hook does not specify all of its dependencies: retries
This dependency is not specified in the hook dependency list.
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
23-28: Optimize dependency handling for theoptionsobject.Including the
optionsobject in the dependency array might cause unnecessary effect runs if the object is recreated on each render, even if its contents haven't changed.Consider either:
- Memoizing the options object using
useMemo- Extracting primitive values from options and using those in the dependency array
- Using the
useDeepCompareEffectfrom a library likeuse-deep-compare-effect- useEffect(() => { + // Example using primitive values + const { rpc, id, name } = options; + useEffect(() => { if (connectors.length === 0 && retries < 5) { setRetries(prev => prev + 1); predeployedAccounts(options).then(setConnectors); } - }, [connectors, options]); + }, [connectors, rpc, id, name]);🧰 Tools
🪛 Biome (1.9.4)
[error] 23-23: This hook does not specify all of its dependencies: retries
This dependency is not specified in the hook dependency list.
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
examples/example-vite-kitchen-sink/src/components/starknet-provider.tsx (2)
9-16: Ensure consistent RPC URL sources.The component uses two different sources for RPC URLs:
dojoConfig.rpcUrlfor the predeployed accounts hookgetRpcUrl()for the JSON RPC providerThis inconsistency could lead to problems if these sources return different values.
Consider using the same source for both:
const { connectors } = usePredeployedAccounts({ rpc: dojoConfig.rpcUrl as string, id: "katana", name: "Katana", }); const provider = jsonRpcProvider({ - rpc: () => ({ nodeUrl: getRpcUrl() }), + rpc: () => ({ nodeUrl: dojoConfig.rpcUrl as string }), });Or alternatively:
+ const rpcUrl = getRpcUrl(); const { connectors } = usePredeployedAccounts({ - rpc: dojoConfig.rpcUrl as string, + rpc: rpcUrl, id: "katana", name: "Katana", }); const provider = jsonRpcProvider({ - rpc: () => ({ nodeUrl: getRpcUrl() }), + rpc: () => ({ nodeUrl: rpcUrl }), });
22-22: Simplify array usage by removing unnecessary spread operator.The spread operator in
[...connectors]is unnecessary sinceconnectorsis already an array that can be directly passed to theconnectorsprop.- connectors={[...connectors]} + connectors={connectors}examples/example-predeployed-connector/src/starknet-provider.tsx (1)
1-1: Useimport typefor type-only imports.Since
PropsWithChildrenis only used as a type and not as a value, it's better to useimport typeto ensure it gets removed during compilation.- import { type PropsWithChildren } from "react"; + import type { PropsWithChildren } from "react";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
examples/example-vite-react-sql/src/components/starknet-provider.tsx (1)
1-1: Useimport typefor type-only importsConsider using the
import typesyntax for thePropsWithChildrenimport since it's only used as a type annotation.-import { type PropsWithChildren } from "react"; +import type { PropsWithChildren } from "react";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
📜 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 (15)
.changeset/clean-tables-turn.md(1 hunks).gitignore(1 hunks)examples/example-predeployed-connector/src/starknet-provider.tsx(1 hunks)examples/example-vite-kitchen-sink/src/components/starknet-provider.tsx(2 hunks)examples/example-vite-react-app-recs/package.json(1 hunks)examples/example-vite-react-phaser-recs/package.json(1 hunks)examples/example-vite-react-sdk/package.json(1 hunks)examples/example-vite-react-sdk/src/starknet-provider.tsx(2 hunks)examples/example-vite-react-sql/src/components/starknet-provider.tsx(1 hunks)examples/example-vite-react-threejs-recs/package.json(1 hunks)examples/example-vite-svelte-recs/package.json(1 hunks)examples/example-vue-app-recs/package.json(1 hunks)packages/predeployed-connector/package.json(2 hunks)packages/predeployed-connector/src/react/index.ts(1 hunks)packages/predeployed-connector/tsup.config.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
examples/example-vite-react-sql/src/components/starknet-provider.tsx
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
examples/example-predeployed-connector/src/starknet-provider.tsx
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
packages/predeployed-connector/src/react/index.ts
[error] 23-23: This hook does not specify all of its dependencies: retries
This dependency is not specified in the hook dependency list.
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: check
- GitHub Check: build
🔇 Additional comments (15)
examples/example-vite-react-threejs-recs/package.json (1)
81-82: Correct categorization ofvite-plugin-top-level-awaitas a dev dependencyMoving
vite-plugin-top-level-awaitfrom dependencies to devDependencies is appropriate since it's a build-time plugin that isn't needed at runtime. This follows the best practice of only including runtime dependencies in thedependenciessection..changeset/clean-tables-turn.md (1)
1-5: Properly documented new React hook for predeployed accountsThe changeset correctly indicates this is a patch release for the predeployed-connector package. The feature description is clear and concise.
.gitignore (1)
10-10: Good addition to gitignore for Vite timestamp filesAdding this pattern will help keep the repository clean by ignoring Vite's temporary timestamp files. The double asterisk pattern properly covers timestamp files in any subdirectory.
packages/predeployed-connector/tsup.config.ts (1)
7-10: Correctly configured entry points including new React integrationThe configuration now properly exposes the React hook as a separate entry point, which is good for tree-shaking and allows consumers to import only what they need. This matches the feature described in the changeset.
examples/example-vue-app-recs/package.json (1)
32-33: Relocate “vite-plugin-top-level-await” to DevDependencies
The dependency"vite-plugin-top-level-await": "^1.4.4"is now placed in thedevDependenciessection, which is appropriate since it is used only during development/build time. Ensure that no production code relies on it at runtime.examples/example-vite-svelte-recs/package.json (1)
19-20: Ensure Consistency in Dependency Placement
The plugin"vite-plugin-top-level-await": "^1.4.4"has been correctly moved todevDependenciesalong with"vite": "^5.4.1". Confirm that this re-categorization does not affect build or runtime behavior in this Svelte example.examples/example-vite-react-phaser-recs/package.json (1)
58-59: Validate Updated DevDependencies for React-Phaser Example
The changes update"vite": "^6.0.11"and relocate"vite-plugin-top-level-await": "^1.4.4"to thedevDependenciessection. This is consistent with similar examples. Please verify that the tooling and build scripts (e.g., for TypeScript and ESLint) are compatible with these updated versions.examples/example-vite-react-app-recs/package.json (1)
45-46: Confirm Dependency Relocation for React App Example
The removal of"vite-plugin-top-level-await"from the runtime dependencies and its re-addition indevDependencieswith version"^1.4.4"is correctly implemented. It would be good to double-check that this change does not introduce any runtime issues in this React example.packages/predeployed-connector/package.json (1)
67-68: LGTM! Dependencies have been properly configured.The package correctly sets React and React DOM as peer dependencies while including their types as dev dependencies. This is the recommended approach for packages that provide React components or hooks.
examples/example-predeployed-connector/src/starknet-provider.tsx (1)
8-12: Good implementation of the predeployed accounts hook.The component now uses the
usePredeployedAccountshook which simplifies the code by removing manual state management and retries logic. This is a clean implementation that follows good React practices.examples/example-vite-react-sdk/src/starknet-provider.tsx (3)
3-5: Good refactoring with improved importsThe changes to the import statements appropriately integrate the new
usePredeployedAccountshook while maintaining a clean import structure.
8-12: Clean implementation of the new hookThe implementation of the
usePredeployedAccountshook is well-structured with clear parameters. This approach simplifies the component by abstracting away the complexity of managing predeployed accounts.
22-22: Simplified connector managementUsing the
connectorsdirectly from the hook return value eliminates the need for manual state management, making the code more maintainable and less error-prone.examples/example-vite-react-sql/src/components/starknet-provider.tsx (2)
5-5: Good addition of the new hook importThe import of
usePredeployedAccountsis correctly placed and follows the component's import organization pattern.
8-12: Clean implementation of the hook with consistent configurationThe implementation of
usePredeployedAccountswith configuration parameters is well-structured and consistent with other examples in the codebase. This refactoring removes the need for manual state management withuseStateanduseEffect, resulting in cleaner, more maintainable code.
| "./react": { | ||
| "import": "./dist/src/react.js", | ||
| "types": "./dist/src/react.d.ts" | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the export path for the React module.
The export path for the React module points to ./dist/src/react.js and ./dist/src/react.d.ts, but based on the file structure, the actual files might be at ./dist/src/react/index.js and ./dist/src/react/index.d.ts.
If the paths don't match, update the package.json exports accordingly:
"./react": {
- "import": "./dist/src/react.js",
- "types": "./dist/src/react.d.ts"
+ "import": "./dist/src/react/index.js",
+ "types": "./dist/src/react/index.d.ts"
}🏁 Script executed:
#!/bin/bash
# Check the actual build output path for the React module
ls -la ./packages/predeployed-connector/dist/src/ | grep -E "react|react.js|react.d.ts"
ls -la ./packages/predeployed-connector/dist/src/react/ 2>/dev/null | grep -E "index|index.js|index.d.ts"
# Verify build configuration
grep -r "src/react" ./packages/predeployed-connector/tsup.config.tsLength of output: 386
Action Required: Update Export Paths for React Module
It appears that the build output no longer contains the "src" folder under ./dist, so the paths in the package.json export should be updated to point to ./dist/react/index.js and ./dist/react/index.d.ts instead of the current paths. Based on the output from running the verification scripts:
- The directory
./packages/predeployed-connector/dist/src/doesn’t exist. - The tsup configuration maps
"src/react"to"src/react/index.ts", implying that the built files should be located at./dist/react/index.jsand./dist/react/index.d.ts.
Please update the export snippet in packages/predeployed-connector/package.json as follows:
"./react": {
- "import": "./dist/src/react.js",
- "types": "./dist/src/react.d.ts"
+ "import": "./dist/react/index.js",
+ "types": "./dist/react/index.d.ts"
}📝 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.
| "./react": { | |
| "import": "./dist/src/react.js", | |
| "types": "./dist/src/react.d.ts" | |
| }, | |
| "./react": { | |
| - "import": "./dist/src/react.js", | |
| - "types": "./dist/src/react.d.ts" | |
| + "import": "./dist/react/index.js", | |
| + "types": "./dist/react/index.d.ts" | |
| }, |
MartianGreed
force-pushed
the
fix/example-predeployed
branch
from
19d964f to
612da82
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/predeployed-connector/package.json (1)
24-27:⚠️ Potential issueUpdate export paths for the React module.
The export paths for the React module need to be updated based on the actual build output structure.
"./react": { - "import": "./dist/src/react.js", - "types": "./dist/src/react.d.ts" + "import": "./dist/react/index.js", + "types": "./dist/react/index.d.ts" }
🧹 Nitpick comments (2)
examples/example-vite-react-sql/src/components/starknet-provider.tsx (1)
1-1: Consider usingimport typefor type-only imports.The
PropsWithChildrenimport is only used as a type. Usingimport typeensures it's removed during compilation and avoids loading unnecessary modules.-import { type PropsWithChildren } from "react"; +import type { PropsWithChildren } from "react";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
packages/predeployed-connector/src/react/index.ts (1)
23-28: Improve error handling in the account fetching logic.The current implementation only has a retry mechanism but lacks proper error handling. If
predeployedAccountsfails, there's no feedback to the user.useEffect(() => { if (connectors.length === 0 && retries < 5) { setRetries(retries + 1); - predeployedAccounts(options).then(setConnectors); + predeployedAccounts(options) + .then(setConnectors) + .catch(error => { + console.error("Failed to fetch predeployed accounts:", error); + // Optionally set some error state here to show to the user + }); } }, [connectors, options, retries]);🧰 Tools
🪛 Biome (1.9.4)
[error] 23-23: This hook does not specify all of its dependencies: retries
This dependency is not specified in the hook dependency list.
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
.changeset/clean-tables-turn.md(1 hunks).gitignore(1 hunks)examples/example-predeployed-connector/src/starknet-provider.tsx(1 hunks)examples/example-vite-kitchen-sink/src/components/starknet-provider.tsx(2 hunks)examples/example-vite-react-app-recs/package.json(1 hunks)examples/example-vite-react-phaser-recs/package.json(1 hunks)examples/example-vite-react-sdk/package.json(1 hunks)examples/example-vite-react-sdk/src/starknet-provider.tsx(2 hunks)examples/example-vite-react-sql/src/components/starknet-provider.tsx(1 hunks)examples/example-vite-react-threejs-recs/package.json(1 hunks)examples/example-vite-svelte-recs/package.json(1 hunks)examples/example-vue-app-recs/package.json(1 hunks)packages/predeployed-connector/package.json(2 hunks)packages/predeployed-connector/src/react/index.ts(1 hunks)packages/predeployed-connector/tsup.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- .changeset/clean-tables-turn.md
- .gitignore
- packages/predeployed-connector/tsup.config.ts
- examples/example-vite-svelte-recs/package.json
- examples/example-vite-react-phaser-recs/package.json
- examples/example-vite-react-sdk/package.json
- examples/example-vue-app-recs/package.json
- examples/example-vite-react-threejs-recs/package.json
- examples/example-vite-react-sdk/src/starknet-provider.tsx
- examples/example-vite-kitchen-sink/src/components/starknet-provider.tsx
- examples/example-vite-react-app-recs/package.json
🧰 Additional context used
🪛 Biome (1.9.4)
examples/example-vite-react-sql/src/components/starknet-provider.tsx
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
packages/predeployed-connector/src/react/index.ts
[error] 23-23: This hook does not specify all of its dependencies: retries
This dependency is not specified in the hook dependency list.
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: check
🔇 Additional comments (3)
examples/example-vite-react-sql/src/components/starknet-provider.tsx (1)
5-5: LGTM - Great refactoring with the new hook!Using the
usePredeployedAccountshook simplifies the component by removing manual state management for predeployed accounts. This abstraction improves readability and maintainability.Also applies to: 8-12
packages/predeployed-connector/package.json (1)
33-34: LGTM - Good practice adding React as peer dependency.Adding React and React DOM as peer dependencies with their corresponding type definitions ensures proper compatibility and prevents duplicate React instances.
Also applies to: 45-46
examples/example-predeployed-connector/src/starknet-provider.tsx (1)
1-1: LGTM - Consistent implementation of the new hook.The refactoring to use
usePredeployedAccountshook is consistent with other components and removes manual state management, making the code cleaner and more maintainable.Also applies to: 5-5, 8-12
| export function usePredeployedAccounts( | ||
| options: PredeployedAccountsConnectorOptions | ||
| ) { | ||
| const [retries, setRetries] = useState<number>(0); | ||
| const [connectors, setConnectors] = useState< | ||
| PredeployedAccountsConnector[] | ||
| >([]); | ||
|
|
||
| useEffect(() => { | ||
| if (connectors.length === 0 && retries < 5) { | ||
| setRetries(retries + 1); | ||
| predeployedAccounts(options).then(setConnectors); | ||
| } | ||
| }, [connectors, options]); | ||
|
|
||
| return { connectors }; | ||
| } |
There was a problem hiding this comment.
Add missing dependency in useEffect dependency array.
The retries state variable is used inside the useEffect callback but is missing from the dependency array, which could lead to stale closures and unexpected behavior.
useEffect(() => {
if (connectors.length === 0 && retries < 5) {
setRetries(retries + 1);
predeployedAccounts(options).then(setConnectors);
}
- }, [connectors, options]);
+ }, [connectors, options, retries]);📝 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.
| export function usePredeployedAccounts( | |
| options: PredeployedAccountsConnectorOptions | |
| ) { | |
| const [retries, setRetries] = useState<number>(0); | |
| const [connectors, setConnectors] = useState< | |
| PredeployedAccountsConnector[] | |
| >([]); | |
| useEffect(() => { | |
| if (connectors.length === 0 && retries < 5) { | |
| setRetries(retries + 1); | |
| predeployedAccounts(options).then(setConnectors); | |
| } | |
| }, [connectors, options]); | |
| return { connectors }; | |
| } | |
| export function usePredeployedAccounts( | |
| options: PredeployedAccountsConnectorOptions | |
| ) { | |
| const [retries, setRetries] = useState<number>(0); | |
| const [connectors, setConnectors] = useState< | |
| PredeployedAccountsConnector[] | |
| >([]); | |
| useEffect(() => { | |
| if (connectors.length === 0 && retries < 5) { | |
| setRetries(retries + 1); | |
| predeployedAccounts(options).then(setConnectors); | |
| } | |
| }, [connectors, options, retries]); | |
| return { connectors }; | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 23-23: This hook does not specify all of its dependencies: retries
This dependency is not specified in the hook dependency list.
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
Closes #
Introduced changes
Checklist
Summary by CodeRabbit
New Features
Refactor
Chore