-
Notifications
You must be signed in to change notification settings - Fork 557
Revert "brige embed (#7222)" #7345
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
Conversation
This reverts commit 35a909a.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis change removes the entire BridgeEmbed 2.0 UI, state machine, hooks, and related documentation from the codebase, including all bridge payment orchestration, step execution, error handling, and Storybook stories. It also updates various types and components to remove references to bridge-specific logic, simplifies payment and token handling, and removes unused files, tests, and documentation. Some UI and type definitions are refactored for consistency with the new structure. Changes
Sequence Diagram(s)Omitted due to the extensive and varied nature of the deletions and refactors; no single new feature or control flow is introduced that warrants a sequence diagram. Suggested labels
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
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: 4
🔭 Outside diff range comments (5)
packages/thirdweb/src/react/core/hooks/wallets/useAutoConnectCore.test.tsx (1)
111-153
:console.info
spy is never restored & relying on console side-effects weakens the test
infoSpy
is created (lines 112-113) but never restored.
• This leavesconsole.info
mocked for the remainder of the test suite, potentially breaking unrelated tests that rely on real logging.- The test asserts timeout behaviour by calling
console.info
inside the callback and then spying on it.
• This couples the test to an implementation detail (that the callback logs to the console) rather than to the contract “onTimeout
is invoked”.
• A much cleaner pattern is to pass avi.fn()
asonTimeout
and assert that this mock was called.@@ -const infoSpy = vi.spyOn(console, "info").mockImplementation(() => {}); +const onTimeout = vi.fn();@@ - onTimeout: () => console.info("TIMEOUTTED"), + onTimeout,@@ - expect(infoSpy).toHaveBeenCalled(); - expect(infoSpy).toHaveBeenCalledWith("TIMEOUTTED"); - warnSpy.mockRestore(); + expect(onTimeout).toHaveBeenCalled(); + warnSpy.mockRestore(); + infoSpy.mockRestore(); // only needed if the spy is kept for other reasonsThis removes the unnecessary console spying and guarantees the spy cleanup.
If you prefer to keep the console approach, at minimum addinfoSpy.mockRestore()
after the assertions.packages/thirdweb/src/react/web/ui/components/ChainName.tsx (1)
30-38
:shorterChainName
can return an empty string; guard & trim onceEdge case:
"Mainnet Chain"
→""
, causing an empty<Text>
node.
Also, splitting on" "
leaves empty tokens for multiple spaces.-function shorterChainName(name: string) { - const split = name.split(" "); - const wordsToRemove = new Set(["mainnet", "testnet", "chain"]); - return split - .filter((s) => { - return !wordsToRemove.has(s.toLowerCase()); - }) - .join(" "); +function shorterChainName(name: string) { + const wordsToRemove = new Set(["mainnet", "testnet", "chain"]); + const shorter = name + .split(/\s+/) // collapse multiple spaces + .filter(Boolean) // eliminate empty tokens + .filter((w) => !wordsToRemove.has(w.toLowerCase())); + + // Fallback to original name if everything was stripped + return shorter.length ? shorter.join(" ") : name; }This keeps UX intact while still providing the abbreviated variant.
packages/thirdweb/src/react/core/hooks/others/useChainQuery.ts (1)
138-144
: InconsistentqueryKey
strategy can break cache sharing
getQueryOptions
now uses["chain", chain]
, while the helper hooks above (useChainName
,useChainIconUrl
, …) still query with["chain", chain?.id]
.
React-Query will therefore create two separate cache entries for the same chain, triggering duplicate network requests and stale data.- queryKey: ["chain", chain], + // Keep it consistent with the rest of the file + queryKey: ["chain", chain?.id],…or update every other occurrence (lines 18, 41, 70, 93, 119) to also pass the full
chain
object – but do it everywhere, not just here.packages/thirdweb/src/pay/utils/commonTypes.ts (1)
22-23
: Capitalisation change breaks theFiatProvider
literal type
FiatProviders
was switched to uppercase literals, but many call-sites (e.g.buyWithFiat/getQuote.ts
→case "stripe":
) still emit lowercase strings.
This will now violate theFiatProvider
type and fail compilation / runtime checks.Options:
-export const FiatProviders = ["COINBASE", "STRIPE", "TRANSAK"] as const; +// keep previous casing to avoid a breaking change +export const FiatProviders = ["coinbase", "stripe", "transak"] as const;…or update every downstream comparison/return value to uppercase.
apps/dashboard/src/components/buttons/MismatchButton.tsx (1)
294-324
: Re-adding the null-safety you just removed may prevent runtime crashes
onPurchaseSuccess(info)
now assumesinfo
is always defined. If any upstream change calls the callback withundefined
(e.g. network error, user abort), the button explodes at runtime (Cannot read properties of undefined (reading 'type')
).Either keep optional chaining or early-return when
!info
.- onPurchaseSuccess(info) { - if ( - info.type === "crypto" && + onPurchaseSuccess(info) { + if (!info) { + return; + } + if ( + info.type === "crypto" &&Same safeguard applies to the
fiat
branch below.
🧹 Nitpick comments (17)
packages/thirdweb/src/wallets/connection/autoConnectCore.test.ts (1)
121-123
: Use a mock instead of relying onconsole.info
side-effectsUsing
console.info
purely to assert thatonTimeout
was invoked couples the test to a global side-effect and forces you to stub/restore another console method.
A cleaner pattern is to pass avi.fn()
spy directly and assert its invocation, keeping the test self-contained and avoiding unnecessary console stubbing.- onTimeout: () => console.info("TIMEOUTTED"), + onTimeout: infoSpy,You can then drop lines 105-106 & 141-142 that deal with
console.info
.AGENTS.md (1)
42-42
: Verify Jest configuration and add a test command example.The doc now states “Jest is pre-configured” but omits how to actually run the tests. Please confirm the Jest setup in CI and consider adding a sample command (e.g.,
pnpm test
ornpm test
) for contributors.- 42- Keep tests deterministic and side-effect free; Jest is pre-configured. + 42- Keep tests deterministic and side-effect free; Jest is pre-configured. + 43- Run tests with `pnpm test` or `npm test` to execute the Jest suite.packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/BuyScreen.tsx (1)
542-543
:fromToken
may beundefined
– confirm receiving component handles it
SwapScreenContent
now receivesfromToken
directly (previous cast removed). When the user lands on “buy-with-crypto” without having selected a source token yet,fromToken
can beundefined
. Make sure the prop type allows it or guard before passing:- fromToken={fromToken} + fromToken={fromToken ?? null}(or treat the zero-state inside
SwapScreenContent
).packages/thirdweb/src/pay/buyWithFiat/getQuote.ts (1)
294-297
: Add explicit handling for “COINBASE” for clarityThe switch now covers
STRIPE
andTRANSAK
, withCOINBASE
falling through thedefault
. It works, but an explicit case improves readability and prevents accidental regressions:- case "STRIPE": + case "STRIPE": return "stripe"; - case "TRANSAK": + case "TRANSAK": return "transak"; - default: // default to coinbase when undefined or any other value + case "COINBASE": + default: // undefined or anything else → coinbase return "coinbase";packages/thirdweb/src/react/web/ui/ConnectWallet/Details.test.tsx (2)
594-595
: Avoidconsole.log
in test helpers
setScreen={(scr) => console.log(scr)}
will spam CI output. Use a noop orvi.fn()
instead:- setScreen={(scr) => console.log(scr)} + setScreen(() => {})
619-620
: Same logging issue in second instanceApply the same quiet noop / mock here.
apps/dashboard/src/app/pay/components/client/PayPageEmbed.client.tsx (1)
66-77
: Remove residual null-checks or keep defensive coding?
result
is now treated as always defined (optional chaining removed), which aligns with the new non-optionalonPurchaseSuccess
contract.
However, other parts of the codebase (or 3rd-party integrators) that still passundefined
will now fail at runtime before TypeScript can warn (e.g. JS callers). Consider leaving a fast-fail guard:- switch (result.type) { + if (!result) { + console.error("onPurchaseSuccess called without a result object"); + return; + } + switch (result.type) {Up to you, but a 1-line guard avoids a potential hard crash while still surfacing the misuse.
packages/thirdweb/src/bridge/types/BridgeAction.ts (1)
1-1
: Dropping"fee"
is a breaking API change – double-check downstream usages
Action
previously allowed"fee"
. Removing it will make any existing bridge orchestration code that relied on that literal stop compiling. Please:
- Search the repo (and docs) for
"\"fee\""
usages.- Publish this removal in release notes / changelog.
If any runtime (JS) extensions emit
"fee"
, they will now fall through type-guards and potentially produce undefined behaviour.packages/thirdweb/src/react/core/hooks/transaction/useSendTransaction.ts (1)
72-87
:info
made mandatory – ensure all external consumers are updatedThe callback signature change is correct for safety, but it silently breaks any TypeScript projects pinned to an older minor version that still mark
info
optional.
Recommend bumping the package major or minor with a clear CHANGELOG note, and adding an overload that accepts the old signature (deprecated) for a smoother migration path.packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/WalletRow.tsx (1)
51-69
: Consider retaining monospaced styling for addressesThe monospace/weight props were dropped on the main address line (line 67).
For wallet addresses a monospace font markedly improves legibility and alignment with block-explorer UIs. If the intent was only to simplify styling, you can keep the lighter gap tweaks while still forcing a monospace font:- <Text size={props.textSize || "xs"} color="primaryText"> + <Text + size={props.textSize || "xs"} + color="primaryText" + style={{ fontFamily: "monospace", fontWeight: 500 }} + >packages/thirdweb/src/react/web/ui/components/buttons.tsx (1)
90-138
: Repeatedtransform: scale(1.01)
may clash with the:active
translateAll variants now set
transform: scale(1.01)
on:hover
, while:active
still appliestransform: translateY(1px)
.
Because both rules target the same CSS property, the:active
rule fully overrides the hover scale on click, causing a perceptible jump.Consider composing transforms instead:
-&:active { transform: translateY(1px); } +&:active { transform: scale(0.99) translateY(1px); }…or use a separate property (e.g.
box-shadow
) for the pressed effect.apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/(chainPage)/components/client/PayModal.tsx (1)
45-62
: Minor DRY clean-up for success trackingBoth blocks test the same
info.status.status !== "NOT_FOUND"
condition.
You can merge them to avoid duplicated logic:-if (info.type === "crypto" && info.status.status !== "NOT_FOUND") { +if (info.status.status !== "NOT_FOUND") { trackEvent({ category: "pay", action: "buy", label: "success", type: info.type, chainId: info.status.quote.toToken.chainId, tokenAddress: info.status.quote.toToken.tokenAddress, - amount: info.status.quote.toAmount, + amount: + info.type === "crypto" + ? info.status.quote.toAmount + : info.status.quote.estimatedToTokenAmount, }); }Not critical, but reduces branching and keeps the analytics payload in one spot.
packages/thirdweb/src/react/web/ui/components/TokenIcon.tsx (1)
30-40
:useMemo
adds negligible value here—consider inlining
tokenImage
is derived from two primitives and is cheap to compute; memoization doesn’t buy much and makes the dependency array easy to get wrong. Inline expression improves readability and removes one React hook call.- const tokenImage = useMemo(() => { - if ( - isNativeToken(props.token) || - props.token.address === NATIVE_TOKEN_ADDRESS - ) { - return chainIconQuery.url; - } - return props.token.icon; - }, [props.token, chainIconQuery.url]); + const tokenImage = + isNativeToken(props.token) || props.token.address === NATIVE_TOKEN_ADDRESS + ? chainIconQuery.url + : props.token.icon;apps/playground-web/src/app/connect/pay/embed/RightSection.tsx (1)
73-82
: Hard-coded placeholder URL couples theme & asset; expose as helper?The fallback placeholder is duplicated in a few places and embeds color logic directly in the component. Extracting a small
getPlaceholderImage(themeType)
utility (or passing it via props) centralises the knowledge of the URL format and prevents color mismatches if the theme palette changes.packages/thirdweb/src/react/web/ui/TransactionButton/DepositScreen.tsx (2)
167-176
: Minor style tweak introduces UX inconsistencyAdding
marginBottom
,borderRadius: spacing.md
, andbackgroundColor: theme.colors.tertiaryBg
makes this wallet row look different from the same component used elsewhere (e.g. Buy-flow wallet picker).Consider centralising the style (prop or styled-component) so the wallet row looks identical across screens; avoids visual drift over time.
226-228
: Monospace removal hurts address legibility
shortenAddress
output aligns better in a monospace font, especially when users compare multiple addresses.
Recommend re-addingfontFamily: "monospace"
for the address string.packages/thirdweb/src/react/web/ui/TransactionButton/TransactionModal.tsx (1)
116-129
: PassingconnectOptions={undefined}
drops custom wallet-UI overridesPreviously
BridgeOrchestrator
accepted and forwarded connect options.
If integrators relied on e.g.walletConnect.deepLink
, those props are now silently ignored. Consider forwardingprops.payOptions.connectOptions ?? undefined
instead.- connectOptions={undefined} + connectOptions={ + "connectOptions" in props.payOptions + ? (props.payOptions as any).connectOptions + : undefined + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (111)
.changeset/icy-eyes-show.md
(0 hunks)AGENTS.md
(1 hunks)apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/(chainPage)/components/client/PayModal.tsx
(2 hunks)apps/dashboard/src/app/pay/components/client/PayPageEmbed.client.tsx
(1 hunks)apps/dashboard/src/components/buttons/MismatchButton.tsx
(2 hunks)apps/playground-web/src/app/connect/pay/commerce/page.tsx
(1 hunks)apps/playground-web/src/app/connect/pay/components/types.ts
(0 hunks)apps/playground-web/src/app/connect/pay/embed/LeftSection.tsx
(0 hunks)apps/playground-web/src/app/connect/pay/embed/RightSection.tsx
(1 hunks)apps/playground-web/src/app/connect/pay/embed/page.tsx
(0 hunks)apps/playground-web/src/components/pay/direct-payment.tsx
(0 hunks)packages/thirdweb/knip.json
(1 hunks)packages/thirdweb/package.json
(0 hunks)packages/thirdweb/src/bridge/Routes.ts
(0 hunks)packages/thirdweb/src/bridge/Token.ts
(1 hunks)packages/thirdweb/src/bridge/types/BridgeAction.ts
(1 hunks)packages/thirdweb/src/bridge/types/Errors.ts
(0 hunks)packages/thirdweb/src/pay/buyWithFiat/getQuote.ts
(1 hunks)packages/thirdweb/src/pay/convert/cryptoToFiat.ts
(2 hunks)packages/thirdweb/src/pay/convert/fiatToCrypto.ts
(2 hunks)packages/thirdweb/src/pay/convert/get-token.ts
(1 hunks)packages/thirdweb/src/pay/utils/commonTypes.ts
(1 hunks)packages/thirdweb/src/react/PRODUCT.md
(0 hunks)packages/thirdweb/src/react/TASK_LIST.md
(0 hunks)packages/thirdweb/src/react/TECH_SPEC.md
(0 hunks)packages/thirdweb/src/react/components.md
(0 hunks)packages/thirdweb/src/react/core/adapters/.keep
(0 hunks)packages/thirdweb/src/react/core/adapters/WindowAdapter.ts
(0 hunks)packages/thirdweb/src/react/core/errors/.keep
(0 hunks)packages/thirdweb/src/react/core/errors/mapBridgeError.test.ts
(0 hunks)packages/thirdweb/src/react/core/errors/mapBridgeError.ts
(0 hunks)packages/thirdweb/src/react/core/hooks/connection/ConnectButtonProps.ts
(4 hunks)packages/thirdweb/src/react/core/hooks/others/useChainQuery.ts
(1 hunks)packages/thirdweb/src/react/core/hooks/pay/useBuyWithFiatQuotesForProviders.ts
(0 hunks)packages/thirdweb/src/react/core/hooks/transaction/useSendTransaction.ts
(1 hunks)packages/thirdweb/src/react/core/hooks/useBridgeError.test.ts
(0 hunks)packages/thirdweb/src/react/core/hooks/useBridgeError.ts
(0 hunks)packages/thirdweb/src/react/core/hooks/useBridgePrepare.test.ts
(0 hunks)packages/thirdweb/src/react/core/hooks/useBridgePrepare.ts
(0 hunks)packages/thirdweb/src/react/core/hooks/useBridgeQuote.ts
(0 hunks)packages/thirdweb/src/react/core/hooks/useBridgeRoutes.test.ts
(0 hunks)packages/thirdweb/src/react/core/hooks/useBridgeRoutes.ts
(0 hunks)packages/thirdweb/src/react/core/hooks/usePaymentMethods.test.ts
(0 hunks)packages/thirdweb/src/react/core/hooks/usePaymentMethods.ts
(0 hunks)packages/thirdweb/src/react/core/hooks/useStepExecutor.test.ts
(0 hunks)packages/thirdweb/src/react/core/hooks/useStepExecutor.ts
(0 hunks)packages/thirdweb/src/react/core/hooks/useTransactionDetails.ts
(0 hunks)packages/thirdweb/src/react/core/hooks/wallets/useAutoConnectCore.test.tsx
(1 hunks)packages/thirdweb/src/react/core/machines/.keep
(0 hunks)packages/thirdweb/src/react/core/machines/paymentMachine.test.ts
(0 hunks)packages/thirdweb/src/react/core/machines/paymentMachine.ts
(0 hunks)packages/thirdweb/src/react/core/types/.keep
(0 hunks)packages/thirdweb/src/react/core/utils/wallet.test.ts
(0 hunks)packages/thirdweb/src/react/native/flows/.keep
(0 hunks)packages/thirdweb/src/react/web/adapters/WindowAdapter.ts
(0 hunks)packages/thirdweb/src/react/web/adapters/adapters.test.ts
(0 hunks)packages/thirdweb/src/react/web/flows/.keep
(0 hunks)packages/thirdweb/src/react/web/hooks/transaction/useSendTransaction.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/BridgeOrchestrator.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/DirectPayment.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/ErrorBanner.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/QuoteLoader.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/StepRunner.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/TransactionPayment.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/UnsupportedTokenScreen.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/common/TokenAndChain.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/common/TokenBalanceRow.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/common/WithHeader.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/payment-details/PaymentDetails.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/payment-details/PaymentOverview.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/payment-selection/FiatProviderSelection.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/payment-selection/TokenSelection.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/payment-selection/WalletFiatSelection.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/payment-success/PaymentReceipt.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/Details.test.tsx
(2 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/WalletSelector.tsx
(1 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/constants.ts
(1 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/icons/CreditCardIcon.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/BuyScreen.tsx
(2 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/DirectPaymentModeScreen.tsx
(1 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/fiat/currencies.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/main/useUISelectionStates.ts
(2 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/FiatValue.tsx
(1 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/StepConnector.tsx
(0 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/WalletRow.tsx
(2 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/screens/formatTokenBalance.ts
(0 hunks)packages/thirdweb/src/react/web/ui/PayEmbed.tsx
(4 hunks)packages/thirdweb/src/react/web/ui/TransactionButton/DepositScreen.tsx
(3 hunks)packages/thirdweb/src/react/web/ui/TransactionButton/ExecutingScreen.tsx
(4 hunks)packages/thirdweb/src/react/web/ui/TransactionButton/TransactionModal.tsx
(2 hunks)packages/thirdweb/src/react/web/ui/components/ChainName.tsx
(2 hunks)packages/thirdweb/src/react/web/ui/components/TokenIcon.tsx
(2 hunks)packages/thirdweb/src/react/web/ui/components/buttons.tsx
(4 hunks)packages/thirdweb/src/stories/Bridge/BridgeOrchestrator.stories.tsx
(0 hunks)packages/thirdweb/src/stories/Bridge/DirectPayment.stories.tsx
(0 hunks)packages/thirdweb/src/stories/Bridge/ErrorBanner.stories.tsx
(0 hunks)packages/thirdweb/src/stories/Bridge/FundWallet.stories.tsx
(0 hunks)packages/thirdweb/src/stories/Bridge/PaymentDetails.stories.tsx
(0 hunks)packages/thirdweb/src/stories/Bridge/PaymentSelection.stories.tsx
(0 hunks)packages/thirdweb/src/stories/Bridge/StepRunner.stories.tsx
(0 hunks)packages/thirdweb/src/stories/Bridge/SuccessScreen.stories.tsx
(0 hunks)packages/thirdweb/src/stories/Bridge/TransactionPayment.stories.tsx
(0 hunks)packages/thirdweb/src/stories/Bridge/UnsupportedTokenScreen.stories.tsx
(0 hunks)packages/thirdweb/src/stories/Bridge/fixtures.ts
(0 hunks)packages/thirdweb/src/stories/TokenBalanceRow.stories.tsx
(0 hunks)packages/thirdweb/src/stories/WalletRow.stories.tsx
(0 hunks)packages/thirdweb/src/stories/utils.tsx
(0 hunks)packages/thirdweb/src/wallets/connection/autoConnectCore.test.ts
(1 hunks)
💤 Files with no reviewable changes (77)
- packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/StepConnector.tsx
- apps/playground-web/src/app/connect/pay/embed/page.tsx
- apps/playground-web/src/components/pay/direct-payment.tsx
- packages/thirdweb/src/react/core/errors/.keep
- packages/thirdweb/src/bridge/types/Errors.ts
- packages/thirdweb/src/react/native/flows/.keep
- packages/thirdweb/src/react/core/adapters/.keep
- packages/thirdweb/package.json
- .changeset/icy-eyes-show.md
- packages/thirdweb/src/react/core/machines/.keep
- packages/thirdweb/src/react/core/types/.keep
- packages/thirdweb/src/react/web/hooks/transaction/useSendTransaction.tsx
- packages/thirdweb/src/react/web/flows/.keep
- apps/playground-web/src/app/connect/pay/components/types.ts
- packages/thirdweb/src/react/core/adapters/WindowAdapter.ts
- apps/playground-web/src/app/connect/pay/embed/LeftSection.tsx
- packages/thirdweb/src/react/web/adapters/adapters.test.ts
- packages/thirdweb/src/react/core/hooks/useBridgePrepare.test.ts
- packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/fiat/currencies.tsx
- packages/thirdweb/src/react/web/ui/Bridge/common/TokenBalanceRow.tsx
- packages/thirdweb/src/react/web/ui/ConnectWallet/icons/CreditCardIcon.tsx
- packages/thirdweb/src/stories/Bridge/UnsupportedTokenScreen.stories.tsx
- packages/thirdweb/src/react/core/errors/mapBridgeError.ts
- packages/thirdweb/src/stories/Bridge/PaymentSelection.stories.tsx
- packages/thirdweb/src/react/core/hooks/useBridgeError.ts
- packages/thirdweb/src/react/web/ui/Bridge/QuoteLoader.tsx
- packages/thirdweb/src/react/core/hooks/useStepExecutor.test.ts
- packages/thirdweb/src/react/web/ui/Bridge/DirectPayment.tsx
- packages/thirdweb/src/react/web/ui/Bridge/common/WithHeader.tsx
- packages/thirdweb/src/bridge/Routes.ts
- packages/thirdweb/src/stories/utils.tsx
- packages/thirdweb/src/react/core/errors/mapBridgeError.test.ts
- packages/thirdweb/src/react/web/ui/Bridge/UnsupportedTokenScreen.tsx
- packages/thirdweb/src/react/core/hooks/useBridgeRoutes.test.ts
- packages/thirdweb/src/stories/Bridge/ErrorBanner.stories.tsx
- packages/thirdweb/src/stories/Bridge/TransactionPayment.stories.tsx
- packages/thirdweb/src/react/core/utils/wallet.test.ts
- packages/thirdweb/src/react/components.md
- packages/thirdweb/src/react/core/hooks/useBridgeQuote.ts
- packages/thirdweb/src/stories/TokenBalanceRow.stories.tsx
- packages/thirdweb/src/react/web/ui/ConnectWallet/screens/formatTokenBalance.ts
- packages/thirdweb/src/stories/WalletRow.stories.tsx
- packages/thirdweb/src/stories/Bridge/StepRunner.stories.tsx
- packages/thirdweb/src/react/core/hooks/useBridgeError.test.ts
- packages/thirdweb/src/react/web/ui/Bridge/payment-selection/FiatProviderSelection.tsx
- packages/thirdweb/src/react/core/machines/paymentMachine.test.ts
- packages/thirdweb/src/react/web/ui/Bridge/payment-details/PaymentDetails.tsx
- packages/thirdweb/src/react/web/ui/Bridge/payment-success/SuccessScreen.tsx
- packages/thirdweb/src/react/core/hooks/useBridgeRoutes.ts
- packages/thirdweb/src/react/PRODUCT.md
- packages/thirdweb/src/react/web/ui/Bridge/TransactionPayment.tsx
- packages/thirdweb/src/stories/Bridge/FundWallet.stories.tsx
- packages/thirdweb/src/react/core/hooks/useTransactionDetails.ts
- packages/thirdweb/src/react/core/hooks/usePaymentMethods.test.ts
- packages/thirdweb/src/react/core/hooks/usePaymentMethods.ts
- packages/thirdweb/src/react/TECH_SPEC.md
- packages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsx
- packages/thirdweb/src/stories/Bridge/BridgeOrchestrator.stories.tsx
- packages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsx
- packages/thirdweb/src/react/web/ui/Bridge/payment-selection/WalletFiatSelection.tsx
- packages/thirdweb/src/stories/Bridge/DirectPayment.stories.tsx
- packages/thirdweb/src/react/core/hooks/pay/useBuyWithFiatQuotesForProviders.ts
- packages/thirdweb/src/react/web/ui/Bridge/StepRunner.tsx
- packages/thirdweb/src/stories/Bridge/PaymentDetails.stories.tsx
- packages/thirdweb/src/react/web/ui/Bridge/payment-selection/TokenSelection.tsx
- packages/thirdweb/src/react/web/adapters/WindowAdapter.ts
- packages/thirdweb/src/react/web/ui/Bridge/payment-details/PaymentOverview.tsx
- packages/thirdweb/src/react/core/hooks/useBridgePrepare.ts
- packages/thirdweb/src/stories/Bridge/SuccessScreen.stories.tsx
- packages/thirdweb/src/react/web/ui/Bridge/BridgeOrchestrator.tsx
- packages/thirdweb/src/react/core/hooks/useStepExecutor.ts
- packages/thirdweb/src/stories/Bridge/fixtures.ts
- packages/thirdweb/src/react/web/ui/Bridge/common/TokenAndChain.tsx
- packages/thirdweb/src/react/core/machines/paymentMachine.ts
- packages/thirdweb/src/react/web/ui/Bridge/ErrorBanner.tsx
- packages/thirdweb/src/react/TASK_LIST.md
- packages/thirdweb/src/react/web/ui/Bridge/payment-success/PaymentReceipt.tsx
🧰 Additional context used
🧬 Code Graph Analysis (5)
packages/thirdweb/src/pay/convert/cryptoToFiat.ts (1)
packages/thirdweb/src/pay/convert/get-token.ts (1)
getTokenPrice
(5-24)
packages/thirdweb/src/react/web/ui/components/TokenIcon.tsx (1)
packages/thirdweb/src/react/core/hooks/others/useChainQuery.ts (1)
useChainIconUrl
(37-59)
packages/thirdweb/src/pay/convert/fiatToCrypto.ts (1)
packages/thirdweb/src/pay/convert/get-token.ts (1)
getTokenPrice
(5-24)
packages/thirdweb/src/react/web/ui/TransactionButton/ExecutingScreen.tsx (1)
packages/thirdweb/src/react/web/ui/components/buttons.tsx (2)
Button
(19-144)ButtonLink
(146-146)
packages/thirdweb/src/react/core/hooks/connection/ConnectButtonProps.ts (2)
packages/thirdweb/src/bridge/index.ts (1)
Chain
(12-12)packages/thirdweb/src/exports/react.ts (1)
TokenInfo
(30-30)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Unit Tests
- GitHub Check: Size
- GitHub Check: Lint Packages
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (30)
packages/thirdweb/src/react/web/ui/ConnectWallet/WalletSelector.tsx (1)
702-703
: Approve visual tweak to ShowAllWalletsIcon size.Updating from
iconSize.lg
toiconSize.xl
aligns with the new design emphasis and maintains consistency with the design system’s extra-large sizing. No further changes needed here.packages/thirdweb/knip.json (1)
7-7
: Verify updated entry list
Ensure that the added"src/transaction/actions/send-batch-transaction.ts"
file exists and should be tracked by knip, and confirm that the"src/react/core/hooks/useBridgeRoutes.ts"
entry was removed as intended.#!/bin/bash # Confirm the new entry file exists fd send-batch-transaction.ts # Ensure no lingering references to useBridgeRoutes rg -l "useBridgeRoutes" .packages/thirdweb/src/react/web/ui/components/ChainName.tsx (2)
21-24
: Loss ofcolor
/style
overrides may be a breaking change
<Text>
previously accepted forwarded styling props; callers that relied on custom colors or inline styles will silently break. Verify all usages of<ChainName>
across the app and docs before merging.
11-16
: ```shell
#!/bin/bashSearch for the ChainName component definition and any usage of 'client' in it
rg -n "export const ChainName" packages/thirdweb/src/react/web/ui/components/ChainName.tsx
rg -n "client" packages/thirdweb/src/react/web/ui/components/ChainName.tsx</details> <details> <summary>packages/thirdweb/src/react/web/ui/ConnectWallet/constants.ts (1)</summary> `8-8`: **Verify downstream impact of shrinking the compact-modal width** `modalMaxWidthCompact` drops from `400px` to `360px`. Check the more densely-packed screens (e.g. wallet lists, error states) on small break-points – a 10% reduction can push long labels/warnings to two lines or clip buttons. If the new width is intentional, no action needed. Otherwise, consider keeping 400 px or auditing individual layouts before release. </details> <details> <summary>apps/playground-web/src/app/connect/pay/commerce/page.tsx (1)</summary> `66-68`: **Sample code still compiles – looks good** Switching to `{ name, image }` mirrors the metadata type simplification, and the trailing comma is fine inside a TSX string literal. </details> <details> <summary>packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/BuyScreen.tsx (1)</summary> `27-27`: **Import path sanity-check** `SupportedTokens` now comes from `core/utils/defaultTokens`. Make sure the re-export exists after the bridge files were removed; otherwise the build will break silently with an unresolved module. </details> <details> <summary>packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/main/useUISelectionStates.ts (1)</summary> `70-74`: **Initial state may still be `undefined`** Same issue as above: the ternary chain can still yield `undefined` before falling back to `NATIVE_TOKEN` if *all* other branches are falsy. Verify this always resolves to a valid token. </details> <details> <summary>packages/thirdweb/src/pay/convert/cryptoToFiat.ts (1)</summary> `76-82`: ```shell #!/bin/bash set -e # Find Token.* files under bridge directory and display their contents echo "Searching for Token files in packages/thirdweb/src/bridge..." find packages/thirdweb/src/bridge -maxdepth 1 -type f \( -iname "Token.ts" -o -iname "Token.js" \) -print -exec sed -n '1,200p' {} \;
packages/thirdweb/src/react/web/ui/components/TokenIcon.tsx (1)
7-7
: Import switch looks goodSwitching to
useChainIconUrl
mirrors the new hook name and keeps concerns local. No issues spotted.packages/thirdweb/src/pay/convert/get-token.ts (1)
5-18
: Add an explicit return type and fail fast when price is missing
getTokenPrice
currently relies on type inference and silently returnsundefined
when the bridge API doesn’t have a price for the token.
- Explicitly annotating the return type improves DX and prevents
implicit any
/looseunknown
leaks.- Failing fast (throw or reject) makes downstream math (
Number(price)
, comparisons, etc.) safer than propagatingundefined
.-export async function getTokenPrice( - client: ThirdwebClient, - tokenAddress: string, - chainId: number, -) { +export async function getTokenPrice( + client: ThirdwebClient, + tokenAddress: string, + chainId: number, +): Promise<number> { return withCache( async () => { const result = await tokens({ @@ - return result[0]?.priceUsd; + const price = result[0]?.priceUsd; + if (price === undefined) { + throw new Error( + `Price for token ${tokenAddress} on chain ${chainId} not found`, + ); + } + return price; },Down-stream converters (
convertCryptoToFiat
,convertFiatToCrypto
) will then operate on a guaranteednumber
.
[ suggest_essential_refactor ]packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/DirectPaymentModeScreen.tsx (1)
91-109
: ```shell
#!/bin/bashShow the PaymentInfo type definition
sed -n '1,200p' packages/thirdweb/src/react/core/hooks/connection/ConnectButtonProps.ts
Locate and display the TokenInfo type definition
grep -R "export type TokenInfo" -n packages/thirdweb/src/react/core/utils/defaultTokens.ts
sed -n '1,200p' packages/thirdweb/src/react/core/utils/defaultTokens.ts</details> <details> <summary>packages/thirdweb/src/react/web/ui/TransactionButton/DepositScreen.tsx (1)</summary> `289-290`: **Radius change – verify clickable hit-area** Increasing to `radius.lg` is fine, but double-check the resulting CSS doesn’t clip the inside QR code or overflow effects on hover. </details> <details> <summary>packages/thirdweb/src/react/web/ui/TransactionButton/TransactionModal.tsx (1)</summary> `16-17`: **`LazyBuyScreen` import path only resolved in web bundle** Just a heads-up: if the same modal is reused in React-Native/Expo packages, the direct web path may break metro bundler resolution. </details> <details> <summary>packages/thirdweb/src/bridge/Token.ts (1)</summary> `161-161`: ```shell #!/bin/bash # Re-scan for any lingering `.add(` calls in TS/JS files using glob patterns rg --pretty '\.add\(' --glob '*.{ts,tsx,js}'
packages/thirdweb/src/react/web/ui/TransactionButton/ExecutingScreen.tsx (5)
1-1
: LGTM!The icon import change aligns with the simplified success state UI.
14-14
: LGTM!Adding
ButtonLink
import is appropriate for the explorer link implementation.
69-76
: LGTM!The simplified success state UI maintains the visual feedback while reducing complexity.
78-78
: LGTM!The increased vertical spacing improves the visual hierarchy of the UI.
Also applies to: 90-90
100-102
: LGTM!Placing the primary "Done" action first follows UX best practices.
packages/thirdweb/src/react/web/ui/PayEmbed.tsx (7)
3-3
: LGTM!The updated imports reflect the simplified architecture using connection management hooks.
Also applies to: 12-24
311-321
: LGTM!The simplified state management and centralized connection handling improve maintainability.
323-340
: LGTM!The effects properly handle chain definitions and active wallet management with correct dependencies.
342-346
: LGTM!Safe metadata extraction with proper type checking.
347-360
: LGTM!Clean loading state implementation with proper styling.
361-410
: LGTM!The simplified payment flow with clear screen transitions improves code clarity and maintainability.
207-208
: Minor formatting changepackages/thirdweb/src/react/core/hooks/connection/ConnectButtonProps.ts (3)
28-61
: LGTM!Using the full
TokenInfo
type improves type safety while maintaining backward compatibility.
81-81
: LGTM!The type changes improve type safety by:
- Using full
TokenInfo
type instead of partial- Making the
onPurchaseSuccess
callback parameter required, enforcing proper handlingAlso applies to: 117-132
163-163
: LGTM!Consistent use of
TokenInfo
type across all payment-related interfaces.
setToToken(prefillBuy.token); | ||
} |
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.
Type-safety regression in setToToken
call
setToToken(prefillBuy.token)
is executed unguarded.
If prefillBuy.token
is undefined
, the state setter receives undefined
while the hook’s state is declared as ERC20OrNativeToken
(non-nullable), producing a runtime type hole.
Quick guard:
-if (prefillBuy?.token) {
- setToToken(prefillBuy.token);
-}
+if (prefillBuy?.token) {
+ setToToken(prefillBuy.token);
+}
(or widen the state type to include undefined
).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/main/useUISelectionStates.ts
at lines 52-53, the call to setToToken(prefillBuy.token) is unguarded and may
pass undefined to a state declared as non-nullable ERC20OrNativeToken, causing a
runtime type error. Fix this by adding a guard to ensure prefillBuy.token is
defined before calling setToToken, or alternatively, update the state type to
allow undefined if that fits the logic.
${formatNumber(cryptoToFiatQuery.data.result, 2).toFixed(2)} | ||
</Text> |
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.
Potential runtime error: formatNumber(...).toFixed
formatNumber
typically returns a string (naming + previous usages).
Calling .toFixed()
on a string will throw. If formatNumber
already returns a formatted string, just drop the extra call:
- ${formatNumber(cryptoToFiatQuery.data.result, 2).toFixed(2)}
+ ${formatNumber(cryptoToFiatQuery.data.result, 2)}
If it does return a number, keep toFixed
and remove formatNumber
. Please reconcile and add a unit test.
📝 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.
${formatNumber(cryptoToFiatQuery.data.result, 2).toFixed(2)} | |
</Text> | |
${formatNumber(cryptoToFiatQuery.data.result, 2)} | |
</Text> |
🤖 Prompt for AI Agents
In
packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/FiatValue.tsx
around lines 46 to 47, the code calls toFixed() on the result of formatNumber,
which likely returns a string, causing a runtime error. Verify the return type
of formatNumber; if it returns a string, remove the toFixed() call and use
formatNumber directly. If it returns a number, remove formatNumber and apply
toFixed(2) directly on the number. Update the code accordingly and add a unit
test to cover this formatting behavior.
const price = await getTokenPrice(client, to, chain.id); | ||
if (!price || price === 0) { | ||
throw new Error( | ||
`Error: Failed to fetch price for token ${to} on chainId: ${chain.id}`, | ||
); | ||
} | ||
return { result: fromAmount / token.priceUsd }; | ||
return { result: fromAmount / price }; |
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.
🛠️ Refactor suggestion
Explicitly coerce price
to a finite number before division
If getTokenPrice
ever yields a string or non-finite value, fromAmount / price
will explode with Infinity
or NaN
. Coercing & validating once keeps the math safe.
- const price = await getTokenPrice(client, to, chain.id);
+ const rawPrice = await getTokenPrice(client, to, chain.id);
+ const price =
+ typeof rawPrice === "string" ? Number.parseFloat(rawPrice) : rawPrice;
Consider replacing the current !price || price === 0
guard with
if (!Number.isFinite(price) || price === 0)
for robustness.
🤖 Prompt for AI Agents
In packages/thirdweb/src/pay/convert/fiatToCrypto.ts around lines 75 to 81, the
current check for price uses !price || price === 0, which does not guard against
non-finite values or strings. Update the condition to explicitly coerce price to
a number and check if it is finite using Number.isFinite(price) and also check
for zero. This ensures that the division fromAmount / price is safe and avoids
Infinity or NaN results.
<ButtonLink | ||
fullWidth | ||
onClick={() => { | ||
props.windowAdapter.open( | ||
formatExplorerTxUrl( | ||
chainExplorers.explorers[0]?.url ?? "", | ||
txHash, | ||
), | ||
); | ||
}} | ||
variant="outline" | ||
href={formatExplorerTxUrl( | ||
chainExplorers.explorers[0]?.url ?? "", | ||
txHash, | ||
)} | ||
target="_blank" | ||
as="a" | ||
gap="xs" | ||
color="primaryText" | ||
style={{ | ||
textDecoration: "none", | ||
color: "inherit", | ||
}} | ||
> | ||
View on Explorer | ||
<ExternalLinkIcon width={iconSize.sm} height={iconSize.sm} /> | ||
</Button> | ||
<Spacer y="sm" /> | ||
</ButtonLink> |
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.
Add security attributes for external links
When using target="_blank"
, add rel="noopener noreferrer"
to prevent potential security vulnerabilities.
Apply this diff to add the security attributes:
<ButtonLink
fullWidth
variant="outline"
href={formatExplorerTxUrl(
chainExplorers.explorers[0]?.url ?? "",
txHash,
)}
target="_blank"
+ rel="noopener noreferrer"
as="a"
gap="xs"
style={{
textDecoration: "none",
color: "inherit",
}}
>
📝 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.
<ButtonLink | |
fullWidth | |
onClick={() => { | |
props.windowAdapter.open( | |
formatExplorerTxUrl( | |
chainExplorers.explorers[0]?.url ?? "", | |
txHash, | |
), | |
); | |
}} | |
variant="outline" | |
href={formatExplorerTxUrl( | |
chainExplorers.explorers[0]?.url ?? "", | |
txHash, | |
)} | |
target="_blank" | |
as="a" | |
gap="xs" | |
color="primaryText" | |
style={{ | |
textDecoration: "none", | |
color: "inherit", | |
}} | |
> | |
View on Explorer | |
<ExternalLinkIcon width={iconSize.sm} height={iconSize.sm} /> | |
</Button> | |
<Spacer y="sm" /> | |
</ButtonLink> | |
<ButtonLink | |
fullWidth | |
variant="outline" | |
href={formatExplorerTxUrl( | |
chainExplorers.explorers[0]?.url ?? "", | |
txHash, | |
)} | |
target="_blank" | |
rel="noopener noreferrer" | |
as="a" | |
gap="xs" | |
style={{ | |
textDecoration: "none", | |
color: "inherit", | |
}} | |
> | |
View on Explorer | |
<ExternalLinkIcon width={iconSize.sm} height={iconSize.sm} /> | |
</ButtonLink> |
🤖 Prompt for AI Agents
In packages/thirdweb/src/react/web/ui/TransactionButton/ExecutingScreen.tsx
between lines 106 and 123, the ButtonLink component uses target="_blank" for
opening external links but lacks the rel="noopener noreferrer" attribute. To fix
this, add rel="noopener noreferrer" to the ButtonLink component to enhance
security by preventing the new page from accessing the window.opener property
and to avoid potential phishing attacks.
size-limit report 📦
|
This reverts commit 35a909a.
PR-Codex overview
This PR focuses on significant deletions and updates in the
thirdweb
codebase, including the removal of various files related to UI components, hooks, and tests, as well as modifications in type definitions and UI behavior for payment processing.Detailed summary
Action
type inpackages/thirdweb/src/bridge/types/BridgeAction.ts
.onPurchaseSuccess
callback to requireinfo
parameter.modalMaxWidthCompact
value from "400px" to "360px".FiatProviders
.PayEmbed
to streamline purchase flow and integrateAutoConnect
.ExecutingTxScreen
.Summary by CodeRabbit
Removed Features
User Interface
Documentation
Style
Bug Fixes
Chores