Skip to content

Conversation

@mcayuelas-ledger
Copy link
Contributor

@mcayuelas-ledger mcayuelas-ledger commented Jan 7, 2026

βœ… Checklist

  • npx changeset was attached.
  • Covered by automatic tests.
  • Impact of the changes:
    • ...

πŸ“ Description

Initiate first step to migrate Market from TanStackQuery to RTK Query

Following dada-client guidelines

❓ Context


🧐 Checklist for the PR Reviewers

  • The code aligns with the requirements described in the linked JIRA or GitHub issue.
  • The PR description clearly documents the changes made and explains any technical trade-offs or design decisions.
  • There are no undocumented trade-offs, technical debt, or maintainability issues.
  • The PR has been tested thoroughly, and any potential edge cases have been considered and handled.
  • Any new dependencies have been justified and documented.
  • Performance considerations have been taken into account. (changes have been profiled or benchmarked if necessary)

@mcayuelas-ledger mcayuelas-ledger requested a review from a team as a code owner January 7, 2026 16:41
Copilot AI review requested due to automatic review settings January 7, 2026 16:41
@vercel
Copy link

vercel bot commented Jan 7, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
web-tools Ready Ready Preview, Comment Jan 8, 2026 9:00am
3 Skipped Deployments
Project Deployment Review Updated (UTC)
ledger-live-github-bot Ignored Ignored Preview Jan 8, 2026 9:00am
native-ui-storybook Ignored Ignored Preview Jan 8, 2026 9:00am
react-ui-storybook Ignored Ignored Preview Jan 8, 2026 9:00am

@live-github-bot live-github-bot bot added desktop Has changes in LLD mobile Has changes in LLM common Has changes in live-common labels Jan 7, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2026

⚠️ E2E tests are required

Changes detected require e2e testing before merge (even before asking for any review).

πŸ–₯️ Desktop

-> Run Desktop E2E

  • Select "Run workflow"
  • Branch: feat/migrate-performers-to-rtk
  • Device: nanoSP or stax

πŸ“± Mobile

-> Run Mobile E2E

  • Select "Run workflow"
  • Branch: feat/migrate-performers-to-rtk
  • Device: nanoX

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR migrates the MarketPerformer feature from TanStack Query to RTK Query, establishing a pattern for future market data state management migrations.

  • Introduces RTK Query API setup for market performers with proper caching and polling
  • Refactors the useMarketPerformers hook to use the new RTK Query endpoint
  • Removes legacy TanStack Query implementation and query keys

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
libs/ledger-live-common/src/market/state-manager/api.ts Creates new RTK Query API with getMarketPerformers endpoint including query params, response transformation, and caching configuration
libs/ledger-live-common/src/market/state-manager/types.ts Defines types for RTK Query including MarketPerformersQueryParams and tag enums for cache invalidation
libs/ledger-live-common/src/market/hooks/useMarketPerformers.ts Updates hook to use RTK Query's useGetMarketPerformersQuery instead of TanStack's useQuery, replacing refetchInterval with pollingInterval
libs/ledger-live-common/src/market/api/index.ts Removes the legacy fetchMarketPerformers function and unused imports
libs/ledger-live-common/src/market/utils/queryKeys.ts Removes obsolete MarketPerformers query key entry
apps/ledger-live-mobile/src/context/rtkQueryApi.ts Registers the new marketApi reducer and middleware in the mobile app Redux store
apps/ledger-live-desktop/src/renderer/reducers/rtkQueryApi.ts Registers the new marketApi reducer and middleware in the desktop app Redux store
.changeset/many-snakes-wait.md Documents the migration as a minor version change across affected packages

πŸ’‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 16 to 19

export interface MarketPerformersResult {
data: MarketItemPerformer[];
}
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The MarketPerformersResult interface is defined but never used in the codebase. The RTK Query endpoint getMarketPerformers returns MarketItemPerformer[] directly, not wrapped in a result object with a data property. This interface should be removed to avoid confusion.

Suggested change
export interface MarketPerformersResult {
data: MarketItemPerformer[];
}

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this type be removed as suggested ?

thesan
thesan previously approved these changes Jan 8, 2026
Copy link
Contributor

@thesan thesan left a comment

Choose a reason for hiding this comment

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

LGTM!
But as part of the migration to RTKQuery I'm wondering if it would make sense to have a single countervalues api encompassing all the usage of env("LEDGER_COUNTERVALUES_API") (countervalues, ofac, market performers, etc...). This way we could just add/remove endpoints from the api in a single place instead of adding removing whole apis for each usage.
WDYT ?

Comment on lines 16 to 19

export interface MarketPerformersResult {
data: MarketItemPerformer[];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this type be removed as suggested ?

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.


πŸ’‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 8, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
14.3% Coverage on New Code (required β‰₯ 80%)

See analysis details on SonarQube Cloud

@mcayuelas-ledger mcayuelas-ledger merged commit 7f2a667 into develop Jan 8, 2026
81 of 83 checks passed
@mcayuelas-ledger mcayuelas-ledger deleted the feat/migrate-performers-to-rtk branch January 8, 2026 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common Has changes in live-common desktop Has changes in LLD mobile Has changes in LLM

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants