Ditch support to mb wallet #541

rubenmarcus · 2024-09-19T10:20:57Z

User description

Bitte Wallet will be the standard now.

PR Type

Enhancement, Tests

Description

Added onlyBitteWallet property to BitteWalletContextProvider and replaced all references from onlyMbWallet to onlyBitteWallet.
Removed the entire Mintbase wallet context implementation and its associated tests.
Removed exports and setup functions related to Mintbase wallet.

Changes walkthrough 📝

Relevant files

Enhancement

BitteWalletContext.tsx `Update BitteWallet context to replace Mintbase wallet references` packages/react/src/BitteWalletContext.tsx Added `onlyBitteWallet` property to context provider. Replaced references from `onlyMbWallet` to `onlyBitteWallet`. Updated wallet setup function to use `setupBitteWallet`.	+8/-7
MintbaseWalletContext.tsx `Remove Mintbase wallet context implementation` packages/react/src/MintbaseWalletContext.tsx Removed Mintbase wallet context implementation.	+0/-201
index.ts `Remove MintbaseWalletContext export` packages/react/src/index.ts Removed export for MintbaseWalletContext.	+0/-1
bitte-wallet.ts `Update BitteWallet setup to replace Mintbase wallet references` packages/react/src/wallet/bitte-wallet.ts Replaced references from `onlyMbWallet` to `onlyBitteWallet`.	+2/-2
index.ts `Remove Mintbase wallet setup export` packages/wallet/src/index.ts Removed export for `setupMintbaseWallet`.	+0/-1
mintbase-wallet.ts `Remove Mintbase wallet implementation` packages/wallet/src/mintbase-wallet.ts Removed Mintbase wallet implementation.	+0/-272
setup.ts `Remove Mintbase wallet setup function` packages/wallet/src/setup.ts Removed Mintbase wallet setup function.	+0/-54

Tests

MintbaseWalletContext.test.tsx `Remove Mintbase wallet context tests` packages/react/src/MintbaseWalletContext.test.tsx Removed Mintbase wallet context tests.	+0/-195

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

mintbase-codium-pr-agent · 2024-09-19T10:23:29Z

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Key issues to review Logic Change The logic for determining `isOnlyBitteWallet` in `setupBitteWallet` function has been modified. Ensure that this change aligns with the intended functionality and does not introduce any regressions or unexpected behaviors. Possible Bug The `onlyBitteWallet` parameter in `setupBitteWalletSelector` function defaults to `false`, which might not reflect the actual intent if `onlyBitteWallet` should default to `true` when not specified.

mintbase-codium-pr-agent · 2024-09-19T10:25:35Z

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Ensure parameters are correctly typed to prevent runtime errors Ensure that the `additionalWallets` parameter is always an array when passed to `setupBitteWalletSelector` to avoid potential runtime errors. packages/react/src/BitteWalletContext.tsx [85] -isOnlyBitteWallet ? { additionalWallets } : undefined, +isOnlyBitteWallet ? { additionalWallets: additionalWallets \|\| [] } : undefined, Suggestion importance[1-10]: 9 Why: Ensuring that `additionalWallets` is always an array when passed to `setupBitteWalletSelector` prevents potential runtime errors, which is crucial for robust code.	9
Best practice	Initialize function parameters with default values for clarity and safety Consider initializing `onlyBitteWallet` with a default value in the function parameters to ensure it always has a predictable value, enhancing readability and reducing potential bugs. packages/react/src/BitteWalletContext.tsx [77] -const setupBitteWallet = async (): Promise<WalletSelectorComponents> => { +const setupBitteWallet = async (onlyBitteWallet = false): Promise<WalletSelectorComponents> => { Suggestion importance[1-10]: 8 Why: Initializing `onlyBitteWallet` with a default value enhances readability and reduces potential bugs by ensuring it always has a predictable value.	8
Best practice	Simplify method calls by using destructuring for imported module methods Use destructuring for `BitteWalletAuth` methods at the beginning of the component to simplify the method calls inside your component. packages/react/src/BitteWalletContext.tsx [75] -const { setupBitteWalletSelector, registerWalletAccountsSubscriber, connectWalletSelector, pollForWalletConnection, disconnectFromWalletSelector, signMessage } = BitteWalletAuth; +const { setupBitteWalletSelector } = BitteWalletAuth; Suggestion importance[1-10]: 5 Why: While using destructuring can simplify method calls, the existing code is already clear and the improvement is minor.	5
Maintainability	Refactor complex conditions into a separate function for better readability Refactor the conditional logic inside `setupBitteWallet` to a separate function to improve code readability and maintainability. packages/react/src/BitteWalletContext.tsx [78] -const isOnlyBitteWallet = !!onlyBitteWallet \|\| !!(additionalWallets && additionalWallets.length > 0); +const isOnlyBitteWallet = checkOnlyBitteWallet(onlyBitteWallet, additionalWallets); +function checkOnlyBitteWallet(onlyBitteWallet, additionalWallets) { + return !!onlyBitteWallet \|\| !!(additionalWallets && additionalWallets.length > 0); +} + Suggestion importance[1-10]: 7 Why: Refactoring the conditional logic into a separate function improves code readability and maintainability, making it easier to understand and modify.	7

microchipgnu

LGTM! 🥳

bh2smith

A bunch of tests are deleted that shouldn't be.
I propose removing the redundant "Bitte" from all the naming.

bh2smith · 2024-09-19T10:43:37Z

packages/data/src/api/nearPrice/nearPrice.test.ts

@@ -1,52 +0,0 @@
-import { nearPrice } from './nearPrice';


deleting a test seems unrelated and unwise.

bh2smith · 2024-09-19T10:44:51Z

packages/react/README.md

@@ -72,15 +72,15 @@ the default way of interacting with Mintbase Wallet is using the MintbaseWalletC

 ```typescript
 import "@near-wallet-selector/modal-ui/styles.css";
-import { MintbaseWalletContextProvider } from  '@mintbase-js/react'


Seems like this should just be called WalletContextProvider - independent of the name. If it were, then we wouldn't have had to change anything here.

bh2smith · 2024-09-19T10:45:04Z

packages/react/package.json

@@ -6,8 +6,7 @@
  "scripts": {
    "build": "tsc",
    "watch": "tsc && tsc --watch & jest --watch --coverage",
-    "lint": "eslint . --fix --ext ts --ext tsx",
-    "test": "jest --coverage"


bh2smith · 2024-09-19T10:48:08Z

packages/react/src/BitteWalletContext.tsx

+  const setupBitteWallet = async (): Promise<WalletSelectorComponents> => {
+    const isOnlyBitteWallet = !!onlyBitteWallet || !!(additionalWallets && additionalWallets.length > 0);

    return await setupBitteWalletSelector(
      callbackUrl,
-      isOnlyMbWallet,
+      isOnlyBitteWallet,
      selectedNetwork,
      selectedContract,
-      isOnlyMbWallet ? { additionalWallets } : undefined,
+      isOnlyBitteWallet ? { additionalWallets } : undefined,
      successUrl, failureUrl,
    );
  };


I think we could live without having the wallet name everywhere. Like why not just setupWallet? People know its the bitte wallet from the import.

bh2smith · 2024-09-19T10:51:19Z

packages/react/src/wallet/bitte-wallet.ts

    network?,
    contractAddress?,
    options?: { additionalWallets?: Array<WalletModuleFactory> },
    successUrl?: string,
    failureUrl?: string,
  ): Promise<WalletSelectorComponents> => {

-    if (onlyMbWallet === false) {
+    if (onlyBitteWallet === false) {


Not related to this Pr, but this entire if block is unnecessary and could probably be replaced with much more compact similar code as in react/WalletContext

bh2smith · 2024-09-19T10:51:58Z

packages/react/src/MintbaseWalletContext.tsx

@@ -1,201 +0,0 @@
-import React, {


no delete tests.

bh2smith · 2024-09-19T10:52:16Z

packages/react/src/MintbaseWalletContext.test.tsx

@@ -1,195 +0,0 @@
-/**


no delete tests.

bh2smith · 2024-09-19T11:03:46Z

This shouldn't have been merged.

rubenmarcus added 2 commits September 19, 2024 11:19

ditch support to mintbase wallet

152ae6e

remove mintbase wallet

dab476b

rubenmarcus requested review from GAllen98, bh2smith, marcelokunze, tifrel, SurgeCode, sainthiago, microchipgnu, nategeier and Jxnis as code owners September 19, 2024 10:20

mintbase-codium-pr-agent bot added enhancement New feature or request Tests Review effort [1-5]: 3 labels Sep 19, 2024

rubenmarcus added 3 commits September 19, 2024 11:29

remove mintbase wallet

9399170

testing

fdef042

testing

b8da081

github-actions bot temporarily deployed to production September 19, 2024 10:38 Inactive

microchipgnu approved these changes Sep 19, 2024

View reviewed changes

sainthiago approved these changes Sep 19, 2024

View reviewed changes

rubenmarcus merged commit 4af87a9 into beta Sep 19, 2024
2 checks passed

rubenmarcus deleted the ditch-support-mb-wallet branch September 19, 2024 10:46

bh2smith requested changes Sep 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ditch support to mb wallet #541

Ditch support to mb wallet #541

rubenmarcus commented Sep 19, 2024 •

edited by mintbase-codium-pr-agent bot

Loading

mintbase-codium-pr-agent bot commented Sep 19, 2024

mintbase-codium-pr-agent bot commented Sep 19, 2024

microchipgnu left a comment

bh2smith left a comment

bh2smith Sep 19, 2024

bh2smith Sep 19, 2024

bh2smith Sep 19, 2024

bh2smith Sep 19, 2024

bh2smith Sep 19, 2024

bh2smith Sep 19, 2024

bh2smith Sep 19, 2024

bh2smith commented Sep 19, 2024

Ditch support to mb wallet #541

Ditch support to mb wallet #541

Conversation

rubenmarcus commented Sep 19, 2024 • edited by mintbase-codium-pr-agent bot Loading

User description

PR Type

Description

Changes walkthrough 📝

mintbase-codium-pr-agent bot commented Sep 19, 2024

PR Reviewer Guide 🔍

mintbase-codium-pr-agent bot commented Sep 19, 2024

PR Code Suggestions ✨

microchipgnu left a comment

Choose a reason for hiding this comment

bh2smith left a comment

Choose a reason for hiding this comment

bh2smith Sep 19, 2024

Choose a reason for hiding this comment

bh2smith Sep 19, 2024

Choose a reason for hiding this comment

bh2smith Sep 19, 2024

Choose a reason for hiding this comment

bh2smith Sep 19, 2024

Choose a reason for hiding this comment

bh2smith Sep 19, 2024

Choose a reason for hiding this comment

bh2smith Sep 19, 2024

Choose a reason for hiding this comment

bh2smith Sep 19, 2024

Choose a reason for hiding this comment

bh2smith commented Sep 19, 2024

rubenmarcus commented Sep 19, 2024 •

edited by mintbase-codium-pr-agent bot

Loading