feat(widget): defer yield hydration for dashboard flows#529
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (13)
📝 WalkthroughWalkthroughThis PR removes legacy yield fallback data, introduces a YieldBase alias, composes provider-enriched yield summaries, hydrates opportunities from summaries, migrates streaming/catalog hooks to summaries, simplifies token selection UI and request/unstake flows, and updates tests, mocks, and CLI scripts. ChangesCore Type System and Yield Domain Refactoring
Yield Summary and Provider Fetching Infrastructure
Building Yield Opportunities from Summaries
Dashboard Yield Discovery and Selection Migration
UI Simplification: Remove Yield-Availability Gating
Request Builder and State Machine Simplification
Test Fixture Updates to Remove Legacy Fallback
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/widget/src/pages/details/earn-page/components/select-token-section/select-token-list-item.tsx (1)
28-45:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDon't close the modal before dashboard selection actually succeeds.
onTokenBalanceSelectnow has an async dashboard path that can return without dispatching when the picked token has no yield for the active category (packages/widget/src/pages/details/earn-page/state/earn-page-context.tsx, Lines 601-615), but this handler always closes the modal immediately. That turns enabled rows into silent no-ops. Either gate those rows again, or have the callback report success so the modal stays open when no matching yield exists.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/pages/details/earn-page/components/select-token-section/select-token-list-item.tsx` around lines 28 - 45, The click handler _onItemClick closes the modal immediately even though onTokenBalanceSelect has an async path that can silently no-op; make _onItemClick async, await the result of onTokenBalanceSelect(item) (or accept a boolean success return from onTokenBalanceSelect) and only call closeModal() when the callback indicates success (and handle/track errors accordingly), ensuring SelectModalItem only closes when a dashboard selection actually succeeded.packages/widget/src/hooks/api/use-multi-yields.ts (2)
90-106:⚠️ Potential issue | 🟠 Major | ⚡ Quick winResubscribe when the wallet filter inputs change.
multipleYieldSummaries$({ ...argsRef.current, ... })snapshotsnetwork,isConnected, andisLedgerLivewhen the effect runs, but those fields are not in the dependency list. A connect/disconnect or network switch will keep polling with the old filter context untilyieldIdsorvalidatorsConfigchanges, which can surface yields from the wrong network.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/hooks/api/use-multi-yields.ts` around lines 90 - 106, The effect subscribes to multipleYieldSummaries$ using a snapshot from argsRef.current but does not re-run when those snapshot fields change, so add the actual filter inputs (e.g. network, isConnected, isLedgerLive—or whatever keys inside argsRef.current that affect the observable) to the useEffect dependency list (or destructure them into stable local variables used in the effect) so that multipleYieldSummaries$ is re-created and resubscribed when the wallet/network/connect state changes; keep references to multipleYieldSummaries$, argsRef (or the destructured fields), yieldIds, validatorsConfig, and hashedKey so the unsubscribe/subscribe lifecycle (multiYieldsStore.send) behaves correctly.
56-70:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace the per-key batch instead of only upserting items.
Line 66 only inserts or updates summaries in the existing
Map. When a later poll returns fewer yields, or a filter change excludes some previous ones, those old entries never get removed, souseStreamYieldSummariescan keep exposing stale opportunities indefinitely. Overwrite the whole per-key map for each refresh, or emit an explicit reset before replaying the new batch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/hooks/api/use-multi-yields.ts` around lines 56 - 70, The "yield-opportunity" handler in multiYieldsStore currently mutates the existing per-key Map by upserting a single yieldDto (event.data.yieldDto) which leaves stale entries when a later poll returns fewer items; change the handler to replace the entire per-key Map for event.data.key with a fresh Map built from the incoming batch (e.g., event.data.yields) or, if the event payload must remain a single batch emitter, add/handle an explicit reset event before replaying yields—ensure you update the "yield-opportunity" handler to create a new perKeyMap (Map constructed from the new batch) and call newMap.set(event.data.key, perKeyMap) rather than mutating prev via prev.set(...).
🧹 Nitpick comments (3)
packages/widget/src/pages/details/earn-page/components/select-token-section/select-token.tsx (1)
3-3: ⚡ Quick winRemove
useMemofor this derived value under React Compiler rules.This
datacomputation does not appear to require semantic-stability identity; compute it inline.Proposed refactor
-import { useMemo } from "react"; @@ - const data = useMemo( - () => - selectedToken - .map((st) => { - const tokenBalances = - tokenBalancesData.map((v) => v.filtered).extract() ?? []; - - return { - st, - tokenBalances, - }; - }) - .extractNullable(), - [selectedToken, tokenBalancesData] - ); + const data = selectedToken + .map((st) => { + const tokenBalances = + tokenBalancesData.map((v) => v.filtered).extract() ?? []; + + return { + st, + tokenBalances, + }; + }) + .extractNullable();As per coding guidelines,
packages/widget/src/**/*.{ts,tsx}must not adduseMemo/useCallback/React.memoonly for render-performance optimization; plain values/functions are preferred unless semantic stability is required.Also applies to: 42-56
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/pages/details/earn-page/components/select-token-section/select-token.tsx` at line 3, The imported useMemo is unnecessary for the derived `data` value in select-token.tsx; remove the `useMemo` import and the useMemo wrapper around the `data` computation and instead compute `data` inline where it is used (the same logic now inside the `data` variable at lines ~42-56). Update any references to the `data` variable to use the new plain value; keep all other logic and signatures in the SelectToken component unchanged.Source: Coding guidelines
packages/widget/src/pages/details/earn-page/components/select-token-section/select-token-list-item.tsx (1)
22-23: ⚡ Quick winRemove
React.memofrom this row component.This component does not appear to need semantic identity stability, and the repo's React Compiler guidance says not to add manual memoization just for render performance.
As per coding guidelines,
packages/widget/src/**/*.{ts,tsx}: React Compiler is enabled. Do not adduseMemo,useCallback, orReact.memoonly for render-performance optimization; prefer plain values/functions. Use manual memoization only when required for semantic stability, such as an external API dependency or context value identity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/pages/details/earn-page/components/select-token-section/select-token-list-item.tsx` around lines 22 - 23, The component SelectTokenListItem is wrapped with React.memo but per project React Compiler guidelines it should not be manually memoized for render-performance; remove the memo wrapper and export the plain functional component (keep the existing Props, props names like item, isConnected, isSelected, onTokenBalanceSelect and the component implementation intact) so the file exports SelectTokenListItem as a regular function/constant instead of memo(…).Source: Coding guidelines
packages/widget/src/pages/details/earn-page/state/earn-page-context.tsx (1)
513-590: ⚡ Quick winDrop these new
useCallbackwrappers unless identity is part of the contract.
selectDashboardTokenYieldandgetDashboardTokenYieldstay inside this provider; nothing in the changed flow needs their identities to be stable. With React Compiler enabled, the extra memoization just adds dependency bookkeeping and more stale-closure surface.As per coding guidelines,
packages/widget/src/**/*.{ts,tsx}: React Compiler is enabled. Do not adduseMemo,useCallback, orReact.memoonly for render-performance optimization; prefer plain values/functions. Use manual memoization only when required for semantic stability, such as an external API dependency or context value identity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/pages/details/earn-page/state/earn-page-context.tsx` around lines 513 - 590, The two functions selectDashboardTokenYield and getDashboardTokenYield are unnecessarily wrapped with useCallback — remove the useCallback wrappers and export them as plain functions declared in the same provider scope (keep their existing names: selectDashboardTokenYield and getDashboardTokenYield) so they continue to close over dispatch, apiClient, queryClient, and isLedgerLive; delete the dependency arrays and surrounding useCallback imports/usages and leave the internal logic (including calls to dispatch, queryClient.fetchQuery, getYieldOpportunity/getYieldOpportunityFromSummary, and getDashboardCategoryYieldIdsForToken) unchanged so identity is no longer memoized but behavior remains identical.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/widget/src/hooks/api/use-yield-summaries.ts`:
- Around line 61-63: FetchByIdsWithProvidersArgs declares a chunkSize but
fetchYieldSummariesWithProvidersByIds (and the call path from line ~152) ignores
it, causing callers' chunking expectations to be lost; update
fetchYieldSummariesWithProvidersByIds to accept and forward chunkSize (or remove
chunkSize from FetchByIdsWithProvidersArgs and related types) so the API is
consistent—specifically either add chunkSize parameter handling in
fetchYieldSummariesWithProvidersByIds and any internal helper it calls, ensuring
it uses the passed chunkSize instead of a hardcoded value, or remove chunkSize
from FetchByIdsWithProvidersArgs and all call sites (including FetchByIdsArgs
references) so the property is not present.
---
Outside diff comments:
In `@packages/widget/src/hooks/api/use-multi-yields.ts`:
- Around line 90-106: The effect subscribes to multipleYieldSummaries$ using a
snapshot from argsRef.current but does not re-run when those snapshot fields
change, so add the actual filter inputs (e.g. network, isConnected,
isLedgerLive—or whatever keys inside argsRef.current that affect the observable)
to the useEffect dependency list (or destructure them into stable local
variables used in the effect) so that multipleYieldSummaries$ is re-created and
resubscribed when the wallet/network/connect state changes; keep references to
multipleYieldSummaries$, argsRef (or the destructured fields), yieldIds,
validatorsConfig, and hashedKey so the unsubscribe/subscribe lifecycle
(multiYieldsStore.send) behaves correctly.
- Around line 56-70: The "yield-opportunity" handler in multiYieldsStore
currently mutates the existing per-key Map by upserting a single yieldDto
(event.data.yieldDto) which leaves stale entries when a later poll returns fewer
items; change the handler to replace the entire per-key Map for event.data.key
with a fresh Map built from the incoming batch (e.g., event.data.yields) or, if
the event payload must remain a single batch emitter, add/handle an explicit
reset event before replaying yields—ensure you update the "yield-opportunity"
handler to create a new perKeyMap (Map constructed from the new batch) and call
newMap.set(event.data.key, perKeyMap) rather than mutating prev via
prev.set(...).
In
`@packages/widget/src/pages/details/earn-page/components/select-token-section/select-token-list-item.tsx`:
- Around line 28-45: The click handler _onItemClick closes the modal immediately
even though onTokenBalanceSelect has an async path that can silently no-op; make
_onItemClick async, await the result of onTokenBalanceSelect(item) (or accept a
boolean success return from onTokenBalanceSelect) and only call closeModal()
when the callback indicates success (and handle/track errors accordingly),
ensuring SelectModalItem only closes when a dashboard selection actually
succeeded.
---
Nitpick comments:
In
`@packages/widget/src/pages/details/earn-page/components/select-token-section/select-token-list-item.tsx`:
- Around line 22-23: The component SelectTokenListItem is wrapped with
React.memo but per project React Compiler guidelines it should not be manually
memoized for render-performance; remove the memo wrapper and export the plain
functional component (keep the existing Props, props names like item,
isConnected, isSelected, onTokenBalanceSelect and the component implementation
intact) so the file exports SelectTokenListItem as a regular function/constant
instead of memo(…).
In
`@packages/widget/src/pages/details/earn-page/components/select-token-section/select-token.tsx`:
- Line 3: The imported useMemo is unnecessary for the derived `data` value in
select-token.tsx; remove the `useMemo` import and the useMemo wrapper around the
`data` computation and instead compute `data` inline where it is used (the same
logic now inside the `data` variable at lines ~42-56). Update any references to
the `data` variable to use the new plain value; keep all other logic and
signatures in the SelectToken component unchanged.
In `@packages/widget/src/pages/details/earn-page/state/earn-page-context.tsx`:
- Around line 513-590: The two functions selectDashboardTokenYield and
getDashboardTokenYield are unnecessarily wrapped with useCallback — remove the
useCallback wrappers and export them as plain functions declared in the same
provider scope (keep their existing names: selectDashboardTokenYield and
getDashboardTokenYield) so they continue to close over dispatch, apiClient,
queryClient, and isLedgerLive; delete the dependency arrays and surrounding
useCallback imports/usages and leave the internal logic (including calls to
dispatch, queryClient.fetchQuery,
getYieldOpportunity/getYieldOpportunityFromSummary, and
getDashboardCategoryYieldIdsForToken) unchanged so identity is no longer
memoized but behavior remains identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 25d09ff3-05fd-476e-a587-f9a4839bd66f
📒 Files selected for processing (27)
packages/widget/src/components/molecules/select-opportunity-list-item/index.tsxpackages/widget/src/domain/types/stake.tspackages/widget/src/domain/types/yields.tspackages/widget/src/hooks/api/use-dashboard-yield-catalog.tspackages/widget/src/hooks/api/use-multi-yields.tspackages/widget/src/hooks/api/use-token-list-yields.tspackages/widget/src/hooks/api/use-yield-opportunity/get-yield-opportunity.tspackages/widget/src/hooks/api/use-yield-providers.tspackages/widget/src/hooks/api/use-yield-summaries.tspackages/widget/src/pages/details/earn-page/components/select-token-section/select-token-list-item.tsxpackages/widget/src/pages/details/earn-page/components/select-token-section/select-token.tsxpackages/widget/src/pages/details/earn-page/components/select-token-section/styles.css.tspackages/widget/src/pages/details/earn-page/state/earn-page-context.tsxpackages/widget/src/pages/details/earn-page/state/types.tspackages/widget/src/pages/details/earn-page/state/use-stake-enter-request-dto.tspackages/widget/src/pages/details/earn-page/types.tspackages/widget/src/pages/position-details/hooks/use-stake-exit-request-dto.tspackages/widget/src/pages/position-details/hooks/use-unstake-machine.tspackages/widget/src/pages/review/hooks/use-stake-review.hook.tspackages/widget/src/pages/review/hooks/use-unstake-review.hook.tspackages/widget/src/providers/api/api-client.tspackages/widget/tests/components/select-validator-section.test.tsxpackages/widget/tests/domain/kyc.test.tspackages/widget/tests/fixtures/index.tspackages/widget/tests/pages-dashboard/position-details-model.test.tsxpackages/widget/tests/use-cases/staking-flow/setup.tspackages/widget/tests/use-cases/trust-incentive-apy/setup.ts
💤 Files with no reviewable changes (5)
- packages/widget/tests/use-cases/staking-flow/setup.ts
- packages/widget/src/providers/api/api-client.ts
- packages/widget/src/pages/details/earn-page/components/select-token-section/styles.css.ts
- packages/widget/tests/pages-dashboard/position-details-model.test.tsx
- packages/widget/src/pages/details/earn-page/state/types.ts
| type FetchByIdsWithProvidersArgs = FetchByIdsArgs & { | ||
| queryClient: QueryClient; | ||
| }; |
There was a problem hiding this comment.
Plumb chunkSize through or remove it from this API.
FetchByIdsWithProvidersArgs includes chunkSize, but fetchYieldSummariesWithProvidersByIds ignores it (Line 152 call path), so caller expectations are silently broken.
Proposed fix
export const fetchYieldSummariesWithProvidersByIds = async ({
apiClient,
+ chunkSize,
queryClient,
signal,
suppressRichErrors,
yieldIds,
}: FetchByIdsWithProvidersArgs): Promise<YieldSummaryWithProvider[]> => {
const client = apiClient.withOptions({ signal, suppressRichErrors });
const summaries = await fetchYieldSummariesByIds({
apiClient,
+ chunkSize,
signal,
suppressRichErrors,
yieldIds,
});Also applies to: 144-157
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/widget/src/hooks/api/use-yield-summaries.ts` around lines 61 - 63,
FetchByIdsWithProvidersArgs declares a chunkSize but
fetchYieldSummariesWithProvidersByIds (and the call path from line ~152) ignores
it, causing callers' chunking expectations to be lost; update
fetchYieldSummariesWithProvidersByIds to accept and forward chunkSize (or remove
chunkSize from FetchByIdsWithProvidersArgs and related types) so the API is
consistent—specifically either add chunkSize parameter handling in
fetchYieldSummariesWithProvidersByIds and any internal helper it calls, ensuring
it uses the passed chunkSize instead of a hardcoded value, or remove chunkSize
from FetchByIdsWithProvidersArgs and all call sites (including FetchByIdsArgs
references) so the property is not present.
Fetch dashboard token yield summaries only after a token is selected. Avoid preloading yield metadata for the entire token picker.
99b75ed to
806d66c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/widget/src/pages/details/earn-page/state/earn-page-state-context.tsx`:
- Around line 94-104: The current Maybe.fromFalsy condition skips updating the
state when the selected token is the same and there is no selectedStakeId,
leaving autoSelectYield true; update the boolean expression so the branch also
runs for token-only selects where the token is unchanged but
selectedStakeId.isNothing(), ensuring the map always returns the reset state
with autoSelectYield: false. Specifically, adjust the condition around
state.selectedToken, equalTokens(action.data), and state.selectedStakeId so it
triggers when (a) token changed, (b) selectedStakeId.isJust(), or (c) token is
the same but selectedStakeId.isNothing() — then return getInitialState() with
selectedToken: Maybe.of(action.data) and autoSelectYield: false.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9ed0f11a-1ce1-45ef-ba86-d75937758451
📒 Files selected for processing (10)
package.jsonpackages/widget/src/hooks/api/use-dashboard-yield-catalog.tspackages/widget/src/hooks/api/use-token-list-yields.tspackages/widget/src/pages/details/earn-page/components/select-token-section/select-token-list-item.tsxpackages/widget/src/pages/details/earn-page/components/select-token-section/select-token.tsxpackages/widget/src/pages/details/earn-page/components/select-token-section/styles.css.tspackages/widget/src/pages/details/earn-page/components/select-yield-section/index.tsxpackages/widget/src/pages/details/earn-page/state/earn-page-context.tsxpackages/widget/src/pages/details/earn-page/state/earn-page-state-context.tsxpackages/widget/src/pages/details/earn-page/state/types.ts
💤 Files with no reviewable changes (1)
- packages/widget/src/pages/details/earn-page/components/select-token-section/styles.css.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/widget/src/hooks/api/use-dashboard-yield-catalog.ts
- packages/widget/src/hooks/api/use-token-list-yields.ts
- packages/widget/src/pages/details/earn-page/components/select-token-section/select-token.tsx
- packages/widget/src/pages/details/earn-page/components/select-token-section/select-token-list-item.tsx
- packages/widget/src/pages/details/earn-page/state/earn-page-context.tsx
| return Maybe.fromFalsy( | ||
| state.selectedToken | ||
| .map((v) => !equalTokens(v, action.data)) | ||
| .orDefault(true) || state.selectedStakeId.isJust() | ||
| ) | ||
| .map(() => ({ | ||
| ...getInitialState(), | ||
| selectedToken: Maybe.of(action.data), | ||
| autoSelectYield: false, | ||
| })) | ||
| .orDefault(state); |
There was a problem hiding this comment.
token-only/select does not always disable auto-select
Line 94 can return the previous state for same-token + no-selected-yield, so autoSelectYield may stay true and the init-yield effects (Line 340/Line 353) can still auto-select unexpectedly in dashboard token-only flow.
Suggested patch
case "token-only/select": {
- return Maybe.fromFalsy(
- state.selectedToken
- .map((v) => !equalTokens(v, action.data))
- .orDefault(true) || state.selectedStakeId.isJust()
- )
- .map(() => ({
- ...getInitialState(),
- selectedToken: Maybe.of(action.data),
- autoSelectYield: false,
- }))
- .orDefault(state);
+ const shouldReset =
+ state.selectedToken
+ .map((v) => !equalTokens(v, action.data))
+ .orDefault(true) || state.selectedStakeId.isJust();
+
+ if (!shouldReset && !state.autoSelectYield) return state;
+
+ return {
+ ...(shouldReset ? getInitialState() : state),
+ selectedToken: Maybe.of(action.data),
+ autoSelectYield: false,
+ };
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/widget/src/pages/details/earn-page/state/earn-page-state-context.tsx`
around lines 94 - 104, The current Maybe.fromFalsy condition skips updating the
state when the selected token is the same and there is no selectedStakeId,
leaving autoSelectYield true; update the boolean expression so the branch also
runs for token-only selects where the token is unchanged but
selectedStakeId.isNothing(), ensuring the map always returns the reset state
with autoSelectYield: false. Specifically, adjust the condition around
state.selectedToken, equalTokens(action.data), and state.selectedStakeId so it
triggers when (a) token changed, (b) selectedStakeId.isJust(), or (c) token is
the same but selectedStakeId.isNothing() — then return getInitialState() with
selectedToken: Maybe.of(action.data) and autoSelectYield: false.
Fetch enabled networks from the legacy yields endpoint for chain configuration. Include wallet mode inputs in wagmi config cache keys. Omit empty wallet groups from the generated wallet list.
378c24f to
ffbe10f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/widget/src/common/get-enabled-networks.ts`:
- Around line 20-21: The legacy API call YieldControllerGetMyNetworks returns
ReadonlyArray<string> but the code blindly casts each item to Networks; instead,
validate each returned string against the known Networks set before adding to
the Set<Networks> to avoid pollution. In get-enabled-networks.ts, replace the
direct cast (network) => network as Networks with a guard that checks membership
against the canonical Networks values (e.g., via a isNetwork type-guard or an
allowedNetworks array/Object.values) and only include the item in the Set when
the guard returns true; discard or log invalid strings so the resulting Set has
only valid Networks and consumers can still use Set.has(v.skChainName) safely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c8f1124e-415d-4aef-b374-e31921ee34d8
📒 Files selected for processing (16)
packages/widget/src/common/get-enabled-networks.tspackages/widget/src/providers/api/api-client.tspackages/widget/src/providers/ethereum/config.tspackages/widget/src/providers/settings/types.tspackages/widget/src/providers/wagmi/index.tspackages/widget/tests/mocks/legacy-api-handlers.tspackages/widget/tests/mocks/yield-api-handlers.tspackages/widget/tests/use-cases/deep-links-flow/setup.tspackages/widget/tests/use-cases/external-provider/setup.tspackages/widget/tests/use-cases/gas-warning-flow/setup.tspackages/widget/tests/use-cases/geo-block.test.tsxpackages/widget/tests/use-cases/renders-initial-page.test.tsxpackages/widget/tests/use-cases/select-opportunity.test.tsxpackages/widget/tests/use-cases/sk-wallet.test.tsxpackages/widget/tests/use-cases/staking-flow/setup.tspackages/widget/tests/use-cases/trust-incentive-apy/setup.ts
💤 Files with no reviewable changes (1)
- packages/widget/tests/mocks/yield-api-handlers.ts
✅ Files skipped from review due to trivial changes (1)
- packages/widget/src/providers/settings/types.ts
| (await apiClient.legacy.YieldControllerGetMyNetworks(undefined)).map( | ||
| (network) => network as Networks |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify the API contract for YieldControllerGetMyNetworks and check Networks type usage
# Check the Networks type definition
echo "=== Networks type definition ==="
rg -n "type Networks|enum Networks|interface Networks" --type=ts -A 5
# Find all usages of getEnabledNetworks to understand how the result is consumed
echo -e "\n=== getEnabledNetworks usage sites ==="
rg -n "getEnabledNetworks" --type=ts -C 3
# Look for the generated API type for YieldControllerGetMyNetworks
echo -e "\n=== YieldControllerGetMyNetworks type definition ==="
rg -n "YieldControllerGetMyNetworks" --type=ts -A 10 -g "generated/**"Repository: stakekit/widget
Length of output: 10380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo_root="$(pwd)"
echo "Repo root: $repo_root"
# 1) Inspect the exact code around the changed mapping
echo "=== get-enabled-networks.ts (lines ~1-120) ==="
sed -n '1,140p' packages/widget/src/common/get-enabled-networks.ts | nl -ba
# 2) Inspect Networks type + allowed values
echo -e "\n=== Networks type (domain) ==="
sed -n '1,120p' packages/widget/src/domain/types/chains/networks.ts | nl -ba
# 3) Find YieldControllerGetMyNetworks in generated API typings
echo -e "\n=== Search for YieldControllerGetMyNetworks in generated typings ==="
rg -n "YieldControllerGetMyNetworks" packages/widget/src/generated/api -S || true
# 4) If found, print the relevant generated legacy type for its return payload
# (use a narrower window around the first match)
match_file="$(rg -n "YieldControllerGetMyNetworks" packages/widget/src/generated/api -S -o --files-with-matches | head -n 1 || true)"
if [ -n "${match_file:-}" ]; then
echo -e "\n=== Context from generated file: $match_file ==="
# Print a window around the first occurrence
line_no="$(rg -n "YieldControllerGetMyNetworks" "$match_file" -S | head -n 1 | cut -d: -f1)"
start=$((line_no-40))
end=$((line_no+80))
sed -n "${start},${end}p" "$match_file" | nl -ba
else
echo -e "\n(No generated typings match for YieldControllerGetMyNetworks under packages/widget/src/generated/api.)"
fi
# 5) Check how getEnabledNetworks result is used (to assess downstream impact)
echo -e "\n=== Downstream usage: where networks are treated as Networks[] ==="
rg -n "getEnabledNetworks\\(|caseOf\\(\\{|\\.caseOf\\(\\{|\bnetworks\b.*Set<|Set<.*Networks" packages/widget/src -SRepository: stakekit/widget
Length of output: 197
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== get-enabled-networks.ts (context) ==="
sed -n '1,220p' packages/widget/src/common/get-enabled-networks.ts | cat -n
echo -e "\n=== Networks type (domain) ==="
sed -n '1,140p' packages/widget/src/domain/types/chains/networks.ts | cat -n
echo -e "\n=== Search for YieldControllerGetMyNetworks in generated typings ==="
rg -n "YieldControllerGetMyNetworks" packages/widget/src/generated/api -S || true
echo -e "\n=== Search for legacy.YieldControllerGetMyNetworks usage ==="
rg -n "legacy\\.YieldControllerGetMyNetworks" packages/widget/src -S || true
match_file="$(rg -n "YieldControllerGetMyNetworks" packages/widget/src/generated/api -S -o --files-with-matches | head -n 1 || true)"
if [ -n "${match_file:-}" ]; then
echo -e "\n=== Context from generated file: $match_file ==="
line_no="$(rg -n "YieldControllerGetMyNetworks" "$match_file" -S | head -n 1 | cut -d: -f1)"
start=$((line_no-60))
end=$((line_no+120))
sed -n "${start},${end}p" "$match_file" | cat -n
fi
echo -e "\n=== Downstream usage: networks typed/assumed as Networks[] / Set<Networks> ==="
rg -n "Set<\\s*Networks\\s*>|Networks\\[\\]|ReadonlyArray<\\s*Networks\\s*>|\\bcaseOf\\b\\(" packages/widget/src -SRepository: stakekit/widget
Length of output: 21728
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo -e "\n=== TokenDto network type (generated api/yield.ts) ==="
rg -n "export type TokenDto|interface TokenDto|type TokenDto" packages/widget/src/generated/api/yield.ts -S | head -n 20
token_line="$(rg -n "type TokenDto" packages/widget/src/generated/api/yield.ts -S | head -n 1 | cut -d: -f1 || true)"
if [ -n "$token_line" ]; then
start=$((token_line-10))
end=$((token_line+80))
sed -n "${start},${end}p" packages/widget/src/generated/api/yield.ts | head -n 120
fi
echo -e "\n=== enabledNetworks consumption sites ==="
rg -n "enabledNetworks\\s*:\\s*Set<Networks>|enabledNetworks\\b" packages/widget/src -S
echo -e "\n=== providers/ethereum/config.ts: getEnabledNetworks Right: networks usage ==="
sed -n '1,120p' packages/widget/src/providers/ethereum/config.ts | cat -n
sed -n '120,240p' packages/widget/src/providers/ethereum/config.ts | cat -n
echo -e "\n=== providers/misc/config.ts (enabledNetworks Set<Networks>) ==="
sed -n '1,120p' packages/widget/src/providers/misc/config.ts | cat -n
echo -e "\n=== providers/substrate/config.ts (networks usage) ==="
sed -n '1,140p' packages/widget/src/providers/substrate/config.ts | cat -n
echo -e "\n=== providers/cosmos/config.ts (networks usage) ==="
sed -n '90,170p' packages/widget/src/providers/cosmos/config.ts | cat -nRepository: stakekit/widget
Length of output: 224
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo -e "\n=== TokenDto network type (generated api/yield.ts) ==="
token_line="$(rg -n "^export type TokenDto\\b" packages/widget/src/generated/api/yield.ts -S | head -n 1 | cut -d: -f1 || true)"
if [[ -n "${token_line}" && "${token_line}" =~ ^[0-9]+$ ]]; then
start=$((token_line-5))
end=$((token_line+140))
if [ $start -lt 1 ]; then start=1; fi
sed -n "${start},${end}p" packages/widget/src/generated/api/yield.ts | cat -n
else
echo "Could not find exact 'export type TokenDto' line in packages/widget/src/generated/api/yield.ts"
rg -n "type TokenDto" packages/widget/src/generated/api/yield.ts -S | head -n 5
fi
echo -e "\n=== enabledNetworks consumption sites (enabledNetworks: Set<Networks> / enabledNetworks variable) ==="
rg -n "enabledNetworks\\s*:\\s*Set<\\s*Networks\\s*>|enabledNetworks\\b" packages/widget/src -S
echo -e "\n=== providers/misc/config.ts excerpt ==="
sed -n '1,120p' packages/widget/src/providers/misc/config.ts | cat -n
echo -e "\n=== providers/ethereum/config.ts excerpt around Right: (networks) ==="
rg -n "getEnabledNetworks\\(\\{.*\\}\\)\\.caseOf|Right:\\s*\\(networks\\)" packages/widget/src/providers/ethereum/config.ts -S
sed -n '1,120p' packages/widget/src/providers/ethereum/config.ts | cat -n
echo -e "\n=== providers/cosmos/config.ts excerpt around networks usage ==="
sed -n '90,170p' packages/widget/src/providers/cosmos/config.ts | cat -n
echo -e "\n=== providers/substrate/config.ts excerpt around networks usage ==="
sed -n '1,120p' packages/widget/src/providers/substrate/config.ts | cat -nRepository: stakekit/widget
Length of output: 18123
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo -e "\n=== occurrences of enabledNetworks variable ==="
rg -n "\\benabledNetworks\\b" packages/widget/src -S
echo -e "\n=== networks.has / enabledNetworks.has usages ==="
rg -n "\\b(enabledNetworks|networks)\\.has\\(" packages/widget/src -S
echo -e "\n=== any iteration over Set<Networks> (for..of / [...networks] / Array.from(networks)) ==="
rg -n "for \\(.*of (enabledNetworks|networks)\\)|Array\\.from\\((enabledNetworks|networks)\\)|\\[\\.\\.\\.(enabledNetworks|networks)\\]" packages/widget/src -S
echo -e "\n=== any place passing the Set contents to typed APIs expecting Networks ==="
rg -n "(Networks\\b|skChainName\\b)" packages/widget/src -S | head -n 200Repository: stakekit/widget
Length of output: 1092
Guard the legacy networks response before casting to Networks
YieldControllerGetMyNetworks is typed in the generated legacy API as ReadonlyArray<string>, and the Networks domain type is TokenDto["network"] (string-literal union). The current (network) => network as Networks cast is unchecked. Since consumers only use the resulting set for Set.has(v.skChainName)-based filtering, invalid strings won’t break execution, but they will silently pollute a value typed as Set<Networks>.
Consider filtering/validating each returned string against the known Networks values before inserting into the Set in packages/widget/src/common/get-enabled-networks.ts (lines 20-21).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/widget/src/common/get-enabled-networks.ts` around lines 20 - 21, The
legacy API call YieldControllerGetMyNetworks returns ReadonlyArray<string> but
the code blindly casts each item to Networks; instead, validate each returned
string against the known Networks set before adding to the Set<Networks> to
avoid pollution. In get-enabled-networks.ts, replace the direct cast (network)
=> network as Networks with a guard that checks membership against the canonical
Networks values (e.g., via a isNetwork type-guard or an allowedNetworks
array/Object.values) and only include the item in the Set when the guard returns
true; discard or log invalid strings so the resulting Set has only valid
Networks and consumers can still use Set.has(v.skChainName) safely.
| selectionRequestId !== dashboardTokenSelectionRequestRef.current || | ||
| !tokenYield | ||
| ) { | ||
| return; |
There was a problem hiding this comment.
token-only/select was already dispatched on line 621, so if tokenYield is null here (no yield for the category, or fetch threw) the state is left with a token selected but no yield — no spinner, no "no opportunities" message recovery, and no way out without another user interaction. Should dispatch a reset here instead of returning silently, e.g. dispatch({ type: "token/deselect" }) or re-trigger the non-dashboard token/select path.
| export const isYieldValidatorSelectionRequired = (yieldDto: Yield) => | ||
| !!( | ||
| yieldDto.mechanics.requiresValidatorSelection || | ||
| isYieldIntegrationAggregator(yieldDto) || |
There was a problem hiding this comment.
just double check, this validator is not needed anymore?
dnehl
left a comment
There was a problem hiding this comment.
as discussed in the review - fix comments together with q/a
Summary by CodeRabbit
UI Improvements
Bug Fixes
Refactor
__fallback__.Why
The dashboard and earn flows were doing too much yield metadata work up front, especially in token selection and opportunity discovery. This moves those paths to lightweight summaries first, then hydrates the selected opportunity only when needed.
Notes