Skip to content
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

fix: multiple chain setting #431

Merged
merged 9 commits into from
Oct 11, 2024
Merged

fix: multiple chain setting #431

merged 9 commits into from
Oct 11, 2024

Conversation

sanyu1225
Copy link
Contributor

Summary

  • remove #addToSwitchable function call this.#getBloctoProperties
  • add test
  • add loadSwitchableNetwork comment

Related Links

Asana: https://app.asana.com/0/1202921060741036/1208234504509478/f

Checklist

  • Pasted Asana link.
  • Tagged labels.

Prerequisite/Related Pull Requests

graph TD
    A[Client: new BloctoSDK] -->|Set| B[Constructor]
    B -->|Store| C[this._blocto.unloadedNetwork = switchableChains]
    D[Client: request 'eth_accounts'] -->|Call| E[EthereumProvider.request]
    E -->|Call| F[getBloctoProperties]
    F -->|Check| G{unloadedNetwork exists?}
    G -->|Yes| H[loadSwitchableNetwork]
    H -->|Iterate| I[#addToSwitchable]
    I -->|Originally called| J[getBloctoProperties]
    J -->|Caused loop| F
    G -->|No| K[Continue processing]
    
    subgraph "Root Cause"
    I -.->|Circular call| J
    end


Loading

@sanyu1225 sanyu1225 requested a review from mordochi September 13, 2024 08:21
Copy link

changeset-bot bot commented Sep 13, 2024

🦋 Changeset detected

Latest commit: 79dade9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@blocto/sdk Patch
@blocto/aptos-wallet-adapter-plugin Patch
@blocto/rainbowkit-connector Patch
@blocto/wagmi-connector Patch
@blocto/web3-react-connector Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

*
* @param networkList - List of networks to add
* @returns Promise<null>
*/
async loadSwitchableNetwork(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the annotations are left here without changing function to private. Or do you think you can change it directly to a private function? @mordochi

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我覺得可改可不改
不過如果要改的話版號可能就要生中版 不要用 patch

mordochi
mordochi previously approved these changes Sep 23, 2024
@@ -180,7 +180,6 @@ export default class EthereumProvider
chainId: `${number}`;
rpcUrls: string[];
}): Promise<void> {
await this.#getBloctoProperties();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] 發現 addToSwitchable 跟 getBloctoProperties 長得好像....看起來好重複😂

*
* @param networkList - List of networks to add
* @returns Promise<null>
*/
async loadSwitchableNetwork(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我覺得可改可不改
不過如果要改的話版號可能就要生中版 不要用 patch

@sanyu1225 sanyu1225 merged commit 8d074d4 into develop Oct 11, 2024
3 checks passed
@sanyu1225 sanyu1225 deleted the fix/multiple-chain-setting branch October 11, 2024 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants