[LWDM] chore(aggregated-assets): move domain layer behind shims (LIVE-35226) - #20345
[LWDM] chore(aggregated-assets): move domain layer behind shims (LIVE-35226)#20345LucasWerey wants to merge 17 commits into
Conversation
Web Tools Build Status
|
There was a problem hiding this comment.
Pull request overview
Relocates the legacy libs/ledger-live-common/src/dada-client “domain layer” into the DDD domain/entity/* and domain/api/* packages, while keeping the previous import paths working via one-line re-export shims in live-common. This supports the ongoing DDD migration without forcing consumer rewrites in the same step.
Changes:
- Introduces
@domain/entity-aggregated-asset,@domain/entity-interest-rate, and@domain/api-aggregated-assetsimplementations (types, RTK Query API, transforms, errors, and internals). - Replaces the legacy
dada-clientimplementation files with re-export shims pointing to the new domain packages. - Updates workspace dependencies (live-common + lockfile) and adds a small test to pin the frozen
assetsDataApi.reducerPath.
Reviewed changes
Copilot reviewed 34 out of 35 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
pnpm-lock.yaml |
Adds new workspace importers; also contains additional lockfile churn. |
libs/ledger-live-common/src/dada-client/utils/mergeAssetsDataPages.ts |
Shim re-export to domain API implementation. |
libs/ledger-live-common/src/dada-client/utils/errorUtils.ts |
Shim re-export of error utilities/types from domain API. |
libs/ledger-live-common/src/dada-client/utils/deepMergeCryptoAssets.ts |
Shim re-export to domain API implementation. |
libs/ledger-live-common/src/dada-client/utils/chunkCurrencyIds.ts |
Shim re-export of chunking util/type from domain API. |
libs/ledger-live-common/src/dada-client/types/trend.ts |
Moves ApyType export to the interest-rate entity package. |
libs/ledger-live-common/src/dada-client/state-manager/types.ts |
Shim re-export of API types/enums/constants from domain API. |
libs/ledger-live-common/src/dada-client/state-manager/api.ts |
Shim re-export of the full aggregated-assets API surface. |
libs/ledger-live-common/src/dada-client/mocks/stocks.mock.ts |
Shim re-export of stocks mock from domain API subpath export. |
libs/ledger-live-common/src/dada-client/mocks/stablecoins.mock.ts |
Shim re-export of stablecoins mock from domain API subpath export. |
libs/ledger-live-common/src/dada-client/entities/index.ts |
Shims entity/wire types to the new DDD packages. |
libs/ledger-live-common/src/dada-client/__mocks__/assets.mock.ts |
Shim re-export of assets mocks from domain API mock export. |
libs/ledger-live-common/package.json |
Adds the new @domain/* packages as dependencies (and tidies ordering). |
domain/entity/interest-rate/src/types.ts |
Introduces ApyType union in entity package. |
domain/entity/interest-rate/src/schema.ts |
Adds interest-rate type definition (currently as TS interface). |
domain/entity/interest-rate/src/index.ts |
Exposes interest-rate exports from the entity package. |
domain/entity/aggregated-asset/src/schema.ts |
Adds aggregated-asset meta type definition (currently as TS interface). |
domain/entity/aggregated-asset/src/index.ts |
Exposes aggregated-asset exports from the entity package. |
domain/api/aggregated-assets/tsconfig.json |
Aligns TS libs/types with other domain/api packages (DOM + node types). |
domain/api/aggregated-assets/src/types.ts |
Defines API-facing types/enums/params used by RTK Query and consumers. |
domain/api/aggregated-assets/src/transforms.ts |
Extracts response transforms + wire→entity currency conversion. |
domain/api/aggregated-assets/src/stocks.mock.ts |
Moves stocks mock into the domain API package. |
domain/api/aggregated-assets/src/stablecoins.mock.ts |
Moves stablecoins mock into the domain API package. |
domain/api/aggregated-assets/src/schema.ts |
Defines the raw wire contract shapes (interfaces). |
domain/api/aggregated-assets/src/internals/mergeAssetsDataPages.ts |
Internal page-merging helper relocated into the domain API package. |
domain/api/aggregated-assets/src/internals/market.ts |
Internal market typing + dadaIdToMarketId copied to avoid legacy imports. |
domain/api/aggregated-assets/src/internals/deepMergeCryptoAssets.ts |
Internal deep-merge helper relocated into the domain API package. |
domain/api/aggregated-assets/src/internals/chunkCurrencyIds.ts |
Internal chunking helper relocated into the domain API package. |
domain/api/aggregated-assets/src/index.ts |
Barrel exports for the new domain API package (incl. selected internals + mocks). |
domain/api/aggregated-assets/src/errors.ts |
Error utilities moved into the domain API package. |
domain/api/aggregated-assets/src/assetsData.mock.ts |
Moves aggregated-assets mock fixtures into the domain API package. |
domain/api/aggregated-assets/src/api.ts |
RTK Query API relocated into the domain API package (incl. frozen reducerPath comment). |
domain/api/aggregated-assets/src/api.test.ts |
Adds test pinning assetsDataApi.reducerPath. |
domain/api/aggregated-assets/package.json |
Defines exports (incl. mock subpaths), deps, and peers for the domain API package. |
.changeset/quiet-comets-relocate.md |
Changeset documenting the relocation + live-common patch bump. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
| /** An interest rate attached to one currency. */ | ||
| export interface InterestRate { | ||
| /** Currency identifier */ | ||
| currencyId: string; | ||
| /** Interest rate value */ |
There was a problem hiding this comment.
You're right — fixed in 22b1c04.
domain/entity/README.md is explicit: "Use zod for schema definition", with schema.ts and schema.test.ts listed as required, and "The schema is always required — it is the canonical data model and the primary reason the package exists." Shipping plain interfaces diverged from that.
Now follows the domain/entity/market-sentiment layout: schema.ts holds the Zod schema, types.ts infers the type via z.infer, schema.test.ts covers validation, and the barrel re-exports both.
Worth noting why this was safe to do inside a behaviour-neutral relocation, since my initial reasoning for deferring it was wrong. I had assumed adopting Zod meant adopting branded types, which would force a .parse() at the boundary and break consumers building these shapes from raw JSON. But a plain z.object() without .brand() infers a structurally identical type — so the schema can exist and be exported without anything being obliged to parse yet. Confirmed: libs/ledger-live-common still typechecks with 0 errors and the characterization tests pass unmodified (21 suites, 245 tests).
Applying .parse() at the api boundary remains LIVE-35232, which is where the drop-invalid-item semantics get decided.
Two loosenesses are deliberate and now documented and tested rather than tightened:
InterestRate.typestaysz.string()rather than theApyTypeenum. DADA genuinely sends kinds outside"NRR" | "APY" | "APR"anduseInterestRatesByCurrenciesdrops them — behaviour pinned in test(dada-client): characterize the untested surface (LIVE-35224) #20276. Narrowing here would claim a guarantee the wire doesn't give.fetchAtstaysz.string()rather thanDateTimeIsoSchema. I checked: it has zero production reads — every occurrence is a mock, fixture or Storybook story. Validating a field nobody consumes could only discard otherwise-good rates.
| /** | ||
| * An aggregated asset: one logical asset grouping several per-network currencies. | ||
| * | ||
| * `assetsIds` maps a network id to the currency id that represents this asset on that network, | ||
| * which is what makes it "aggregated" rather than a single currency. |
There was a problem hiding this comment.
You're right — fixed in 22b1c04.
domain/entity/README.md is explicit: "Use zod for schema definition", with schema.ts and schema.test.ts listed as required, and "The schema is always required — it is the canonical data model and the primary reason the package exists." Shipping plain interfaces diverged from that.
Now follows the domain/entity/market-sentiment layout: schema.ts holds the Zod schema, types.ts infers the type via z.infer, schema.test.ts covers validation, and the barrel re-exports both.
Worth noting why this was safe to do inside a behaviour-neutral relocation, since my initial reasoning for deferring it was wrong. I had assumed adopting Zod meant adopting branded types, which would force a .parse() at the boundary and break consumers building these shapes from raw JSON. But a plain z.object() without .brand() infers a structurally identical type — so the schema can exist and be exported without anything being obliged to parse yet. Confirmed: libs/ledger-live-common still typechecks with 0 errors and the characterization tests pass unmodified (21 suites, 245 tests).
Applying .parse() at the api boundary remains LIVE-35232, which is where the drop-invalid-item semantics get decided.
Two loosenesses are deliberate and now documented and tested rather than tightened:
InterestRate.typestaysz.string()rather than theApyTypeenum. DADA genuinely sends kinds outside"NRR" | "APY" | "APR"anduseInterestRatesByCurrenciesdrops them — behaviour pinned in test(dada-client): characterize the untested surface (LIVE-35224) #20276. Narrowing here would claim a guarantee the wire doesn't give.fetchAtstaysz.string()rather thanDateTimeIsoSchema. I checked: it has zero production reads — every occurrence is a mock, fixture or Storybook story. Validating a field nobody consumes could only discard otherwise-good rates.
| metro-react-native-babel-preset: '*' | ||
| react: 19.1.4 | ||
| react-dom: 19.1.4 | ||
| webpack: ^5.89.0 | ||
| webpack: '*' | ||
| peerDependenciesMeta: |
There was a problem hiding this comment.
Fixed in 22b1c04 — the lockfile diff is now 48 insertions, 0 deletions, containing only the three new workspace importers.
You were right to keep pushing on this. I'd previously concluded the churn was unavoidable pnpm re-resolution triggered by the workspace set changing, and said so on an earlier PR. That was wrong. I tested it properly this time: reverting the two peer-range rewrites by hand and then running pnpm install --frozen-lockfile passes, which proves they were cosmetic rather than required.
The two reverted:
any-observable@0.3.0:rxjs: ^5.5.10(pnpm had rewritten it to'*')@storybook/addon-react-native-web@0.0.29:webpack: ^5.89.0(rewritten to'*')
Neither package is touched by this PR, and --frozen-lockfile is what CI enforces, so keeping develop's values is both correct and reviewable.
For the record on the other half of your note: packageManager pins pnpm@10.24.0 and that is the version used, so the rewrites weren't a version mismatch — pnpm just normalises those ranges opportunistically whenever it rewrites the file.
| @@ -25425,7 +25465,7 @@ packages: | |||
| resolution: {integrity: sha512-/FQM1EDkTsf63Ub2C6O7GuYFDsSXUwsaZDurV0np41ocwq0jthUAYCmhBX9f+KwlaCgIuWyr/4WlUQUBfKfZog==} | |||
| engines: {node: '>=6'} | |||
| peerDependencies: | |||
| rxjs: ^5.5.10 | |||
| rxjs: '*' | |||
There was a problem hiding this comment.
Fixed in 22b1c04 — the lockfile diff is now 48 insertions, 0 deletions, containing only the three new workspace importers.
You were right to keep pushing on this. I'd previously concluded the churn was unavoidable pnpm re-resolution triggered by the workspace set changing, and said so on an earlier PR. That was wrong. I tested it properly this time: reverting the two peer-range rewrites by hand and then running pnpm install --frozen-lockfile passes, which proves they were cosmetic rather than required.
The two reverted:
any-observable@0.3.0:rxjs: ^5.5.10(pnpm had rewritten it to'*')@storybook/addon-react-native-web@0.0.29:webpack: ^5.89.0(rewritten to'*')
Neither package is touched by this PR, and --frozen-lockfile is what CI enforces, so keeping develop's values is both correct and reviewable.
For the record on the other half of your note: packageManager pins pnpm@10.24.0 and that is the version used, so the rewrites weren't a version mismatch — pnpm just normalises those ranges opportunistically whenever it rewrites the file.
Rsdoctor Bundle Diff AnalysisFound 7 projects in monorepo, 7 projects with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 desktop-mainPath:
📁 desktop-preloaderPath:
📁 desktop-rendererPath:
📁 desktop-webviewDappPreloaderPath:
📁 desktop-webviewPreloaderPath:
📁 mobilePath:
📁 desktop-workersPath:
Generated by Rsdoctor GitHub Action |
Addresses review on #20345. domain/entity/README.md requires each entity package to define its canonical model as a Zod schema: 'Use zod for schema definition', with schema.ts and schema.test.ts listed as required. Both new entity packages shipped plain TypeScript interfaces instead, which diverged from that convention. Converted to the market-sentiment layout: schema.ts holds the Zod schema, types.ts infers the type, schema.test.ts covers validation, barrel re-exports both. Behaviour-neutral. The schemas use plain z.object without .brand(), so z.infer produces types structurally identical to the previous interfaces - live-common still typechecks with zero errors and the characterization tests pass unchanged. Nothing calls .parse() at the api boundary yet; that is LIVE-35232. Two deliberate loosenesses are documented and tested rather than tightened: InterestRate.type stays a plain string because DADA sends kinds outside ApyType and consumers drop them, and fetchAt stays a plain string rather than DateTimeIsoSchema because nothing reads it, so validating the format could only discard otherwise-good rates. Also fixes the Domain Test CI failure: the job installs only ./domain/** and ./shared/**, so @shared/env's transitive @ledgerhq/live-env was unresolvable. api.test.ts now mocks @shared/env with a factory so the real module is never required. And trims pnpm-lock.yaml to the three new importers only. The peer-range rewrites pnpm emitted for any-observable and @storybook/addon-react-native-web were cosmetic - reverting them keeps 'pnpm install --frozen-lockfile' passing, so the diff is now purely additive.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 40 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (3)
domain/api/aggregated-assets/src/api.ts:7
FetchBaseQueryError,FetchBaseQueryMeta, andQueryReturnValueare types, but they’re imported as values. With SWC/Jest (and in general ESM builds), importing type-only names as runtime exports can lead to runtime import errors or unnecessary runtime dependencies. Align with other domain/api packages by marking these as type imports.
import {
createApi,
fetchBaseQuery,
FetchBaseQueryError,
FetchBaseQueryMeta,
QueryReturnValue,
} from "@reduxjs/toolkit/query/react";
domain/api/aggregated-assets/src/errors.ts:1
FetchBaseQueryErroris only used as a type (in a type guard), so it should be imported as a type-only import. This avoids emitting a runtime named import for a type-only symbol (which may not exist at runtime) and matches the pattern used in other domain/api packages.
import { FetchBaseQueryError } from "@reduxjs/toolkit/query";
domain/api/aggregated-assets/src/api.ts:182
- This comment documents a frozen workaround (hard-coded reducerPath). Per repo comment guidance, workaround comments should include a link to the tracking ticket so the rationale doesn’t go stale when the migration continues.
/*
* FROZEN. createCurrencyDataSelector hand-scans state.assetsDataApi.queries by string, and
* Storybook stories preload this exact key. Renaming it silently returns undefined for every
* market and interest-rate lookup, with no type error.
*/
Won't be fixed here as we are only moving things |
Addresses review on #20345. domain/entity/README.md requires each entity package to define its canonical model as a Zod schema: 'Use zod for schema definition', with schema.ts and schema.test.ts listed as required. Both new entity packages shipped plain TypeScript interfaces instead, which diverged from that convention. Converted to the market-sentiment layout: schema.ts holds the Zod schema, types.ts infers the type, schema.test.ts covers validation, barrel re-exports both. Behaviour-neutral. The schemas use plain z.object without .brand(), so z.infer produces types structurally identical to the previous interfaces - live-common still typechecks with zero errors and the characterization tests pass unchanged. Nothing calls .parse() at the api boundary yet; that is LIVE-35232. Two deliberate loosenesses are documented and tested rather than tightened: InterestRate.type stays a plain string because DADA sends kinds outside ApyType and consumers drop them, and fetchAt stays a plain string rather than DateTimeIsoSchema because nothing reads it, so validating the format could only discard otherwise-good rates. Also fixes the Domain Test CI failure: the job installs only ./domain/** and ./shared/**, so @shared/env's transitive @ledgerhq/live-env was unresolvable. api.test.ts now mocks @shared/env with a factory so the real module is never required. And trims pnpm-lock.yaml to the three new importers only. The peer-range rewrites pnpm emitted for any-observable and @storybook/addon-react-native-web were cosmetic - reverting them keeps 'pnpm install --frozen-lockfile' passing, so the diff is now purely additive.
22b1c04 to
dbe7fc6
Compare
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 40 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (4)
domain/entity/aggregated-asset/src/index.ts:2
- The
domain/entity/README.mdconventions require aschema.mock.tsmock factory file for entity packages (and it should not be re-exported from the barrel). This package currently definesschema.ts+schema.test.tsbut has noschema.mock.ts, so consumers/tests won’t have the standard way to create valid fixtures.
Please add src/schema.mock.ts (e.g., makeCryptoAssetMeta() / makeCryptoAssetMetaId() helpers) following the entity conventions.
export * from "./schema";
export * from "./types";
domain/entity/interest-rate/src/index.ts:2
- The
domain/entity/README.mdconventions require aschema.mock.tsmock factory file for entity packages (and it should not be re-exported from the barrel). This package currently definesschema.ts+schema.test.tsbut has noschema.mock.ts, so downstream tests lack the standard fixture factory.
Please add src/schema.mock.ts (e.g., makeInterestRate() / makeApyType() helpers) consistent with other entity packages.
export * from "./schema";
export * from "./types";
domain/api/aggregated-assets/src/errors.ts:1
FetchBaseQueryErroris only used as a TypeScript type (in the predicate return type). Importing it as a value can introduce an unnecessary runtime dependency edge in emitted JS.
Prefer a type-only import here.
import { FetchBaseQueryError } from "@reduxjs/toolkit/query";
domain/api/aggregated-assets/src/api.ts:7
FetchBaseQueryError,FetchBaseQueryMeta, andQueryReturnValueare only referenced in thecollectAllByCategoryreturn type, so they should be imported as types. Keeping them in the value import can add unnecessary runtime imports when this is built/transpiled.
Consider splitting the import into value vs type-only.
import {
createApi,
fetchBaseQuery,
FetchBaseQueryError,
FetchBaseQueryMeta,
QueryReturnValue,
} from "@reduxjs/toolkit/query/react";
… api Addresses @ysitbon's review on #20345: comments 4, 5, 6, 7, 8 and 9. shared/api-services already exists on develop and its README explicitly names this migration: 'api-aggregated-assets is a placeholder for DADA ... when it migrates, its base belongs here as src/services/dada/ rather than as another standalone createApi'. I had missed it because I checked before rebasing. Adds services/dada with the endpoint-less base api, then the use-case package adds its endpoints with injectEndpoints and its cache tag with enhanceEndpoints({ addTagTypes }), per that README. reducerPath stays the frozen literal 'assetsDataApi', so one reducer, one middleware and one cache slice still serve every use case - which the hand-scanning cache selectors depend on. services/dada carries no extraArgument contract yet, unlike its siblings: DADA endpoints build absolute URLs and pick prod/staging per request from an isStaging query arg, so the base query has nothing to own. Migrating that to extraArgument drops isStaging and touches both apps' store config, so it is left as a documented TODO rather than smuggled into a relocation. Layout, per the review: index.ts is now export * over public modules only, matching the other three domain/api barrels instead of enumerating ~30 named exports. internals/ stops being exported and now means what it says. Split by real consumers rather than by where things happened to live: market.ts and pagination.ts move out (dadaIdToMarketId has 5 app call sites, PartialMarketItemResponse 4), while chunkCurrencyIds, deepMergeCryptoAssets, emptyAssetsData, collectAllByCategory, assertDadaApiUrl and allSettled have no external consumers and stay in. collectAllByCategory and emptyAssetsData move to internals with their own files; the two public category accessors move to accessors.ts; request building and the chunked page fetch move to requests.ts. api.ts is now just the endpoint definitions. The two internal helpers' characterization tests move with them from dada-client into the api package, unchanged apart from the relative import - they were the only remaining consumers, so keeping them where they were would have forced the helpers to stay public. Same 246 tests as before, redistributed: 229 in dada-client, 17 in the api package.
Addresses @ysitbon's review on #20345: comments 4, 5, 6, 7, 8 and 9. shared/api-services already exists on develop and its README explicitly names this migration: 'api-aggregated-assets is a placeholder for DADA ... when it migrates, its base belongs here as src/services/dada/ rather than as another standalone createApi'. I had missed it because I checked before rebasing. Adds services/dada with the endpoint-less base api, then the use-case package adds its endpoints with injectEndpoints and its cache tag with enhanceEndpoints({ addTagTypes }), per that README. reducerPath stays the frozen literal 'assetsDataApi', so one reducer, one middleware and one cache slice still serve every use case - which the hand-scanning cache selectors depend on. services/dada carries no extraArgument contract yet, unlike its siblings: DADA endpoints build absolute URLs and pick prod/staging per request from an isStaging query arg, so the base query has nothing to own. Migrating that to extraArgument drops isStaging and touches both apps' store config, so it is left as a documented TODO rather than smuggled into a relocation. Layout, per the review: index.ts is now export * over public modules only, matching the other three domain/api barrels instead of enumerating ~30 named exports. internals/ stops being exported and now means what it says. Split by real consumers rather than by where things happened to live: market.ts and pagination.ts move out (dadaIdToMarketId has 5 app call sites, PartialMarketItemResponse 4), while chunkCurrencyIds, deepMergeCryptoAssets, emptyAssetsData, collectAllByCategory, assertDadaApiUrl and allSettled have no external consumers and stay in. collectAllByCategory and emptyAssetsData move to internals with their own files; the two public category accessors move to accessors.ts; request building and the chunked page fetch move to requests.ts. api.ts is now just the endpoint definitions. The two internal helpers' characterization tests move with them from dada-client into the api package, unchanged apart from the relative import - they were the only remaining consumers, so keeping them where they were would have forced the helpers to stay public. Same 246 tests as before, redistributed: 229 in dada-client, 17 in the api package.
61a9427 to
d4f207e
Compare
0c2cfd8 to
4facc54
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 57 out of 59 changed files in this pull request and generated no new comments.
Suppressed comments (2)
shared/api-services/src/services/dada/api.test.ts:18
- Test name claims it verifies that the base DADA API declares no tag types, but the assertion only checks that
getRunningQueriesThunkexists (which is true regardless oftagTypes). This makes the test misleading and it won’t catch an accidental change fromtagTypes: []to a non-empty list.
domain/api/aggregated-assets/src/transforms.ts:20 - The follow-up ticket referenced here looks incorrect: this comment says LIVE-35232 for the per-item drop-and-count change, but the PR description and surrounding context indicate that work is tracked under LIVE-35233. Keeping the correct ticket id matters since this is a behavior-affecting edge case.
* The synthesised entity is validated by `CryptoCurrencySchema`, which also brands `id`. `parse`
* rather than `safeParse` is deliberate: it keeps the existing contract that an unusable currency
* surfaces as a query error instead of being silently dropped. Note this makes the whole response
* fail, so LIVE-35232 should convert it to per-item drop-and-count once that telemetry exists.
*/
Addresses review on #20345. domain/entity/README.md requires each entity package to define its canonical model as a Zod schema: 'Use zod for schema definition', with schema.ts and schema.test.ts listed as required. Both new entity packages shipped plain TypeScript interfaces instead, which diverged from that convention. Converted to the market-sentiment layout: schema.ts holds the Zod schema, types.ts infers the type, schema.test.ts covers validation, barrel re-exports both. Behaviour-neutral. The schemas use plain z.object without .brand(), so z.infer produces types structurally identical to the previous interfaces - live-common still typechecks with zero errors and the characterization tests pass unchanged. Nothing calls .parse() at the api boundary yet; that is LIVE-35232. Two deliberate loosenesses are documented and tested rather than tightened: InterestRate.type stays a plain string because DADA sends kinds outside ApyType and consumers drop them, and fetchAt stays a plain string rather than DateTimeIsoSchema because nothing reads it, so validating the format could only discard otherwise-good rates. Also fixes the Domain Test CI failure: the job installs only ./domain/** and ./shared/**, so @shared/env's transitive @ledgerhq/live-env was unresolvable. api.test.ts now mocks @shared/env with a factory so the real module is never required. And trims pnpm-lock.yaml to the three new importers only. The peer-range rewrites pnpm emitted for any-observable and @storybook/addon-react-native-web were cosmetic - reverting them keeps 'pnpm install --frozen-lockfile' passing, so the diff is now purely additive.
Addresses @ysitbon's review on #20345: comments 4, 5, 6, 7, 8 and 9. shared/api-services already exists on develop and its README explicitly names this migration: 'api-aggregated-assets is a placeholder for DADA ... when it migrates, its base belongs here as src/services/dada/ rather than as another standalone createApi'. I had missed it because I checked before rebasing. Adds services/dada with the endpoint-less base api, then the use-case package adds its endpoints with injectEndpoints and its cache tag with enhanceEndpoints({ addTagTypes }), per that README. reducerPath stays the frozen literal 'assetsDataApi', so one reducer, one middleware and one cache slice still serve every use case - which the hand-scanning cache selectors depend on. services/dada carries no extraArgument contract yet, unlike its siblings: DADA endpoints build absolute URLs and pick prod/staging per request from an isStaging query arg, so the base query has nothing to own. Migrating that to extraArgument drops isStaging and touches both apps' store config, so it is left as a documented TODO rather than smuggled into a relocation. Layout, per the review: index.ts is now export * over public modules only, matching the other three domain/api barrels instead of enumerating ~30 named exports. internals/ stops being exported and now means what it says. Split by real consumers rather than by where things happened to live: market.ts and pagination.ts move out (dadaIdToMarketId has 5 app call sites, PartialMarketItemResponse 4), while chunkCurrencyIds, deepMergeCryptoAssets, emptyAssetsData, collectAllByCategory, assertDadaApiUrl and allSettled have no external consumers and stay in. collectAllByCategory and emptyAssetsData move to internals with their own files; the two public category accessors move to accessors.ts; request building and the chunked page fetch move to requests.ts. api.ts is now just the endpoint definitions. The two internal helpers' characterization tests move with them from dada-client into the api package, unchanged apart from the relative import - they were the only remaining consumers, so keeping them where they were would have forced the helpers to stay public. Same 246 tests as before, redistributed: 229 in dada-client, 17 in the api package.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 57 out of 58 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (2)
shared/api-services/src/services/dada/api.test.ts:18
- This test name/comment claims to verify that the service declares no tag types, but the assertion only checks that
getRunningQueriesThunkexists (which is unrelated totagTypes). This makes the test misleading and harder to maintain.
domain/api/aggregated-assets/src/transforms.ts:20 - The comment references LIVE-35232 as the follow-up for per-item drop-and-count, but the PR description and surrounding context describe that work as LIVE-35233. Keeping the wrong ticket number here makes the rationale harder to trace later.
* The synthesised entity is validated by `CryptoCurrencySchema`, which also brands `id`. `parse`
* rather than `safeParse` is deliberate: it keeps the existing contract that an unusable currency
* surfaces as a query error instead of being silently dropped. Note this makes the whole response
* fail, so LIVE-35232 should convert it to per-item drop-and-count once that telemetry exists.
*/
Relocate the dada-client entity types and API layer into the aggregated-assets DDD packages (LIVE-35226, epic LIVE-35223). Behaviour-neutral: zero consumer files change, all 82 consumers keep working through one-line shims at the old paths. Pure relocation - no symbol renames, no signature changes, no zod, and no endpoint restructuring. The per-use-case injectEndpoints split is LIVE-35301. entity-aggregated-asset CryptoAssetMeta entity-interest-rate InterestRate, ApyType api-aggregated-assets wire schema, transforms, api, internals, errors, mocks transforms.ts is split out of api.ts so the wire->entity boundary is separately testable, matching the market-sentiment precedent. NetworkInfo and CurrenciesOrder land in the api package rather than an entity: a network IS a chain, already modelled by entity-currency-crypto, and CurrenciesOrder is server sort metadata. Both were entities in the original scoping; the naming review corrected that. Breaks the last boundary violation. dadaIdToMarketId and the market item type are copied from libs/ledger-live-common/src/market into internals/market.ts, since a domain/* package cannot import legacy libs/*. Drift risk is documented there and in the README: every field is optional, so divergence will never produce a type error. reducerPath stays the literal "assetsDataApi" and is now pinned by a test. createCurrencyDataSelector hand-scans that key as a string and Storybook stories preload it, so a rename would silently return undefined for every market and interest-rate lookup with no type error. The characterization tests from LIVE-35224 pass unmodified: 21 suites, 245 tests.
Addresses review on #20345. domain/entity/README.md requires each entity package to define its canonical model as a Zod schema: 'Use zod for schema definition', with schema.ts and schema.test.ts listed as required. Both new entity packages shipped plain TypeScript interfaces instead, which diverged from that convention. Converted to the market-sentiment layout: schema.ts holds the Zod schema, types.ts infers the type, schema.test.ts covers validation, barrel re-exports both. Behaviour-neutral. The schemas use plain z.object without .brand(), so z.infer produces types structurally identical to the previous interfaces - live-common still typechecks with zero errors and the characterization tests pass unchanged. Nothing calls .parse() at the api boundary yet; that is LIVE-35232. Two deliberate loosenesses are documented and tested rather than tightened: InterestRate.type stays a plain string because DADA sends kinds outside ApyType and consumers drop them, and fetchAt stays a plain string rather than DateTimeIsoSchema because nothing reads it, so validating the format could only discard otherwise-good rates. Also fixes the Domain Test CI failure: the job installs only ./domain/** and ./shared/**, so @shared/env's transitive @ledgerhq/live-env was unresolvable. api.test.ts now mocks @shared/env with a factory so the real module is never required. And trims pnpm-lock.yaml to the three new importers only. The peer-range rewrites pnpm emitted for any-observable and @storybook/addon-react-native-web were cosmetic - reverting them keeps 'pnpm install --frozen-lockfile' passing, so the diff is now purely additive.
Addresses @ysitbon's review on #20345: comments 4, 5, 6, 7, 8 and 9. shared/api-services already exists on develop and its README explicitly names this migration: 'api-aggregated-assets is a placeholder for DADA ... when it migrates, its base belongs here as src/services/dada/ rather than as another standalone createApi'. I had missed it because I checked before rebasing. Adds services/dada with the endpoint-less base api, then the use-case package adds its endpoints with injectEndpoints and its cache tag with enhanceEndpoints({ addTagTypes }), per that README. reducerPath stays the frozen literal 'assetsDataApi', so one reducer, one middleware and one cache slice still serve every use case - which the hand-scanning cache selectors depend on. services/dada carries no extraArgument contract yet, unlike its siblings: DADA endpoints build absolute URLs and pick prod/staging per request from an isStaging query arg, so the base query has nothing to own. Migrating that to extraArgument drops isStaging and touches both apps' store config, so it is left as a documented TODO rather than smuggled into a relocation. Layout, per the review: index.ts is now export * over public modules only, matching the other three domain/api barrels instead of enumerating ~30 named exports. internals/ stops being exported and now means what it says. Split by real consumers rather than by where things happened to live: market.ts and pagination.ts move out (dadaIdToMarketId has 5 app call sites, PartialMarketItemResponse 4), while chunkCurrencyIds, deepMergeCryptoAssets, emptyAssetsData, collectAllByCategory, assertDadaApiUrl and allSettled have no external consumers and stay in. collectAllByCategory and emptyAssetsData move to internals with their own files; the two public category accessors move to accessors.ts; request building and the chunked page fetch move to requests.ts. api.ts is now just the endpoint definitions. The two internal helpers' characterization tests move with them from dada-client into the api package, unchanged apart from the relative import - they were the only remaining consumers, so keeping them where they were would have forced the helpers to stay public. Same 246 tests as before, redistributed: 229 in dada-client, 17 in the api package.
mergeAssetsDataPages was in the api package but the api never calls it - its only consumers are useAssetsData and useStocksData, both hooks. Merging the pages of an infinite query is a consumption decision, not something the api does, so it belongs with the hooks in @features/platform-aggregated-assets. It was also misplaced twice: its test was still in dada-client importing through the shim, the same code-away-from-test split just corrected for chunkCurrencyIds and deepMergeCryptoAssets. Both move to features/platform/aggregated-assets as pagination.ts and pagination.test.ts, assertions unchanged. The platform package gains @domain/api-aggregated-assets for AssetsDataWithPagination, which is the correct downward dependency; live-common's shim now re-exports from the platform package. Still 246 tests: 216 in dada-client, 17 in the api package, 13 here.
Completes the second half of @ysitbon's comment on api.ts:53 - 'could go on internals and could be tested there'. The move landed in d4f207e but it was still only exercised indirectly, through the chunked endpoint returning it for an empty id list. Pins the invariant that matters: it must return a fresh object every call. The chunked lookup endpoint uses it as a reduce seed and then mutates the accumulator in place, so a shared instance would leak merged assets between queries. collectAllByCategory (comment api.ts:163) is left covered through its two public accessors, which the existing suite already exercises across pages, on a failing page and against an untrusted host - testing it directly would only duplicate that.
features/platform/README.md prescribes components/ hooks/ helpers/ with only files that fit no subdirectory at the root. mergeAssetsDataPages is a cross-feature domain-aware helper, which is what helpers/ is for.
Addresses @ysitbon's comments 1 and 2 on #20345, for consistency with the rest of the domain layer. fetchAt -> DateTimeIsoSchema currencyId -> union of CryptoCurrencyIdSchema and TokenCurrencyIdSchema Feasibility was the open question and it is cheaper than expected: 0 errors in libs/ledger-live-common, 0 in both new packages, and no brand breakage in either app. The many DADA fixtures are untyped object literals, so branding the fields does not reach them. Only one file needed touching - pagination.test.ts, whose inline rate fixtures now go through InterestRateSchema.parse. currencyId is a union, not CryptoCurrencyIdSchema alone: DADA keys interestRates by both crypto ids (ethereum) and token ids (ethereum/erc20/usd_tether__erc20_), so either brand on its own would mislabel half the data. fetchAt now rejects a malformed timestamp, so the test asserting the previous 'we deliberately do not validate this' intent is inverted rather than kept. All four timestamp shapes DADA actually sends are covered, including its 6-digit fractional seconds. No runtime behaviour changes: nothing calls InterestRateSchema.parse at the api boundary yet, which is LIVE-35232.
Fixes the live-common typecheck. Two characterization fixtures still assigned plain strings to InterestRate.currencyId and .fetchAt, which are branded as of the previous commit. Both now go through InterestRateSchema.parse, so the fixtures are valid by construction rather than cast. One fetchAt literal was '2026-07-31', which is not RFC 3339 and would not have parsed - corrected to '2026-07-31T00:00:00Z'. Missed locally because CI runs 'tsc --noEmit -p src/tsconfig.json --customConditions node' while I had checked the root tsconfig, which does not include these test files.
The branded ids and RFC 3339 fetchAt forced three characterization fixtures to be rebuilt through InterestRateSchema.parse, which breaks the guarantee that those tests pass unmodified through the migration. Revert to the structural z.string() shapes so the fixtures stay byte identical to develop. Branding lands in LIVE-35232 alongside assetsIds, where retargeting consumers is already in scope.
Review feedback from @ysitbon on #20345. - market.ts moves to internals/ and leaves the barrel. Nothing outside the package imported it: every external consumer of dadaIdToMarketId and PartialMarketItemResponse resolves the legacy live-common declaration, not this one. The two copies still need de-duplicating in the retarget tasks. - the five single-function pure helpers become internals/utils.ts, with their tests merged. collectAllByCategory stays its own file: it does network I/O and paginates, which is not a util. - ONE_DAY_IN_SECONDS leaves types.ts for constants.ts, written as 24 * 60 * 60. It stays exported because the live-common shim re-exports it. - the mocks move under src/fixtures/. The .mock.ts suffix is kept so knip's '**/*.mock.*' ignore still applies, and the ./mock* export names are unchanged, so no consumer moves.
Review feedback from @ysitbon on #20345: use CryptoCurrencySchema rather than parsing the id on its own. convertApiAssets synthesises a CryptoCurrency when DADA knows an asset the local registry does not. Only its id was validated, via CryptoCurrencyIdSchema.parse; the rest of the object was unchecked. Parsing the whole object through CryptoCurrencySchema validates every field and brands the id, so the standalone parse is no longer needed. parse rather than safeParse is deliberate: it preserves the contract the characterization tests pin, where an unusable currency surfaces as a query error instead of being dropped silently. It does widen what can fail the whole response, so LIVE-35232 should convert this to per-item drop-and-count once that telemetry exists.
Review feedback from @ysitbon on #20345: every function should be tested. Five exported functions had no reference in any test file in the repo: resolveBaseUrl, fetchAssetsPage, allSettled, assertDadaApiUrl, and this package's copy of dadaIdToMarketId (only the live-common duplicate was covered). collectAllByCategory was exercised only indirectly, through the two accessors. assertDadaApiUrl is the one that mattered most: it is the host guard for the endpoints that build their own fetch instead of going through baseQuery, and it was only reached as a side effect of untested code. dadaIdToMarketId's trailing-separator case is characterized rather than endorsed: '?? id' only catches null/undefined, so 'ethereum:erc20:' yields an empty market id instead of the original.
Review feedback from @ysitbon on #20345: use getCryptoCurrencyById for the mocks. These fixtures are transformed-shape AssetsData, so cryptoOrTokenCurrencies holds real CryptoCurrency entities — exactly what the registry returns. The three hand-built ones had already drifted from the CAL: injective claimed family "injective" and one unit where the registry says "cosmos" and two, and bitcoin and arbitrum were similarly stale. getCryptoCurrencyById rather than findCryptoCurrencyById on purpose: it throws, which is what a fixture should do if the CAL ever drops the id. The consuming assertions match on ids and assetsIds, not on presentation fields, so nothing needed updating.
So adding a category is a list of assets rather than another hand-written
fixture.
buildCategoryResponse takes { ticker, slug?, name?, token?, market? } per
asset and derives the rest: the urn:crypto:meta-currency: ids, the token
currency and its parent network, the markets map and currenciesOrder. A new
category becomes one call.
Stablecoins collapses to a ticker list, which is all the category endpoints
consume. Stocks keeps its slugs explicit because they are not derivable from
the tickers — AAPLX is applex, not aaplx. That was caught by diffing the
serialised fixtures before and after: both are byte-identical to what the
hand-written versions produced, so the MSW handlers and integration tests
see no change.
buildCategoryResponse accumulated into Record<string, unknown>, which erased the types the hand-written object literals used to infer. The desktop Portfolio integration test reads a currency straight out of the map and passes it where CryptoCurrency | TokenCurrency is expected, so it failed desktop typecheck with TS2322. Annotate the two maps with NetworkInfo and CryptoOrTokenCurrency instead. Type-only change: the serialised fixtures are still byte-identical to the pre-refactor originals.
Review feedback from @ysitbon on #20345: internal if nothing outside the package uses it. resolveBaseUrl and fetchAssetsPage have no consumer anywhere outside this package, so they move to internals/requests.ts and leave the barrel. buildAssetsQueryParams stays public — the LIVE-35224 characterization tests import it through the dada-client shim, and those must not change. collectAllByCategory duplicated resolveBaseUrl's getEnv branch inline; it now calls it instead. This is the cosmetic half of the point. Both helpers exist only because the shared base query is configured with baseUrl: "", so every endpoint resolves its own absolute url and the fan-out endpoint hand-rolls fetch — which is also why assertDadaApiUrl is needed and why RTK's AbortSignal never reaches the request. queryFn receives baseQuery as its fourth argument, so that is fixable; both files now say so, and it is an acceptance criterion on LIVE-35301 alongside the extraArgument migration.
The rebase onto develop conflicted on pnpm-lock.yaml in six commits. Each was resolved by taking develop's copy, so the workspace importer entries for the new @domain/* deps were dropped; this restores them. Regenerating with pnpm install produced 451 insertions and 189 deletions of unrelated peer-hash churn (react-native, expo, firebase, lumen), so instead develop's lockfile is taken verbatim and only the four changed importer blocks are spliced in. Against develop the diff is 51 insertions and 0 deletions, and pnpm install --frozen-lockfile succeeds.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 57 out of 58 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (2)
shared/api-services/src/services/dada/api.test.ts:18
- This test case says it verifies that
dadaApideclares no tag types, but it currently asserts ondadaApi.util.getRunningQueriesThunk, which is unrelated and would still be defined even if tag types were added. This makes the test misleading and won’t catch the regression it claims to cover.
domain/api/aggregated-assets/src/transforms.ts:20 - The doc comment references LIVE-35232 for the follow-up that converts this failure mode to per-item drop-and-count, but the PR description and surrounding context indicate that work is tracked as LIVE-35233. Keeping the ticket reference accurate matters since this comment is meant to document an intentional trade-off.
* The synthesised entity is validated by `CryptoCurrencySchema`, which also brands `id`. `parse`
* rather than `safeParse` is deliberate: it keeps the existing contract that an unusable currency
* surfaces as a query error instead of being silently dropped. Note this makes the whole response
* fail, so LIVE-35232 should convert it to per-item drop-and-count once that telemetry exists.
*/
|
Won't fix now |




✅ Checklist
npx changesetwas attached.- Behaviour-neutral by design. Zero consumer files change; all 82 consumers keep working through one-line re-export shims at the old paths.
- One deliberate exception, added during review: the synthesised currency in
convertApiAssetsis now validated byCryptoCurrencySchema— see Review round below.- QA focus if anything is spot-checked: Market (price / 24h change badges), Portfolio (distribution), and the asset/network selectors — the surfaces fed by the relocated cache selectors. Plus the Stocks and Stablecoins sections, whose MSW fixtures were refactored (proven byte-identical).
📝 Description
Problem.
libs/ledger-live-common/src/dada-clientis being migrated into the DDD layers under LIVE-35223. The packages were scaffolded empty in #20285; this PR moves the domain layer into them.Solution. Relocate the entity types and the API layer, leaving shims behind so consumer retargeting is a separate, independently revertable step (LIVE-35228 / 35229 / 35230).
@domain/entity-aggregated-assetCryptoAssetMetaSchema+ inferredCryptoAssetMeta@domain/entity-interest-rateInterestRateSchema,ApyTypeSchema+ inferred types@domain/api-aggregated-assetsschema.ts(wire),types.ts,transforms.ts,requests.ts,accessors.ts,api.ts,errors.ts,pagination.ts,constants.ts,internals/,fixtures/@shared/api-servicesservices/dada/— the endpoint-lesscreateApibase14 files under
dada-clientbecame one-line re-exports.libs/ledger-live-commongains the packages as workspace deps — it already declared 8@domain/*deps, so this is the established pattern.Decisions worth reviewing
@shared/api-services, and this package injects into it.services/dada/ownscreateApi({ reducerPath: DADA_REDUCER_PATH, tagTypes: [], endpoints: () => ({}) }); the domain package doesdadaApi.enhanceEndpoints({ addTagTypes }).injectEndpoints(...). One reducer, one middleware, one cache slice per backend. Splitting the singleinjectEndpointscall into three per-use-case modules is LIVE-35301.domain/entity/README.mdmandates it:schema.tsholds the schema and the TS types arez.infer<>. Both entity packages follow theentity/market-sentimentlayout.reducerPathstays the literal"assetsDataApi", pinned by a test.createCurrencyDataSelectorhand-scans that key as a string and three Storybook files preload it, so a rename would silently returnundefinedfor every market and interest-rate lookup with no type error. Any rename belongs to LIVE-35301.NetworkInfoandCurrenciesOrderland in the api package, not an entity. A network is a chain, already modelled by@domain/entity-currency-crypto, soNetworkInfostays a wire type resolving to that rather than duplicating the concept.CurrenciesOrderis{ key, order, metaCurrencyIds }— server sort metadata, not a business object. Agreed with @gre and Yoann; see DADA DDD compliant.internals/really is internal. Nothing in it is exported from the barrel.market.tslives there too:dadaIdToMarketId()and the market item type are copied from live-common rather than imported, because adomain/*package must not import legacylibs/*. The drift risk is documented in the file —PartialMarketItemResponseisPartial<MarketItemResponse>, so every field is optional and future divergence will never produce a type error.pagination.tsis public and lives here, not in the platform layer. It was moved out and back during review.mergeAssetsDataPagesis consumed by the platform hooks, and it is structurally the same kind of helper aserrors.ts/parseError— which already lives indomain/api— so consistency won.Review round (@ysitbon)
collectAllByCategoryCryptoCurrencySchemainstead of parsing the id alonegetCryptoCurrencyByIdfor the mocksfixtures/foldertypes.tsconstants.ts; Temporal declined, see belowfetchAt/currencyIdwith@shared/schema-primitivesfetchAssetsPageinternal, "à voir la pertinence de l'implém"The market type was fully internalisable. I had argued it wasn't, citing 7 consumers of
dadaIdToMarketIdand 5 ofPartialMarketItemResponse— those counts were for the legacy live-common declarations. Zero files import either from@domain/api-aggregated-assets, so dropping the barrel export cost nothing. Worth flagging the real finding:dadaIdToMarketIdwas duplicated, not moved — an identical copy remains atmarket/utils/index.ts:129, and only that copy had a test. De-duplicating is retarget work.Branding was reverted deliberately.
DateTimeIsoSchemaand the crypto-or-token id union are correct, but branding them broketscin three characterization fixtures, which had to be rebuilt throughInterestRateSchema.parse(...)to compile. Those fixtures are the epic's safety net and must pass unmodified, so the branding moved to LIVE-35232 where consumer churn is already expected. Note for whoever picks it up: verify with CI's command (tsc --noEmit -p src/tsconfig.json --customConditions node), because the roottsconfig.jsonreports zero errors on code that fails CI.Temporal was declined, with evidence. V8 in node 24.14 has it behind
--harmony-temporal, default off and marked in progress / experimental;typeof Temporalisundefinedwithout the flag, and there is notemporal-polyfillanywhere in the monorepo. Mobile runs Hermes and desktop runs Electron — neither exposes it either. It would mean a polyfill shipped into the mobile bundle, and sincekeepUnusedDataFor?: numbertakes plain seconds,Temporal.Duration.from({ days: 1 }).total({ unit: "seconds" })just returns86400. Written as24 * 60 * 60inconstants.tsinstead. Temporal's real home here is parsingfetchAt—DateTimeIsoSchema's own doc comment already anticipatesTemporal.Instant.from().The fetch helpers were the cosmetic half.
resolveBaseUrlandfetchAssetsPagemoved tointernals/— nothing outside the package used them. But both exist only because the shared baseis
fetchBaseQuery({ baseUrl: "" }), so every endpoint resolves its own absolute url and thefan-out endpoints hand-roll
fetch. RTK passesbaseQueryasqueryFn's fourth argument, sothat is fixable, and the bypass is what forces
assertDadaApiUrlto exist, drops RTK'sAbortSignal, and makes a DADA 5xx surface asFETCH_ERRORinstead of a numeric status — soisNetworkError()reports true for a server error while the paginated endpoints classify the samefailure correctly. Latent today: the only consumer chain is
useChunkedAssetsData→useAssetDistribution, which never readserrorInfo. Now acceptance criteria on LIVE-35301.The one behaviour change, stated plainly
convertApiAssetsnow builds the synthesised currency throughCryptoCurrencySchema.parse(...)rather than validating only its id.parseand notsafeParseis deliberate — it preserves the contract the characterization tests pin, where an unusable currency surfaces as a query error rather than vanishing.It does widen what can fail a response:
CryptoCurrencySchemarequiresunits.min(1)while the wire type does not, so an asset arriving withunits: []would now fail the whole response. That is recorded as an explicit acceptance criterion on LIVE-35233, which converts this path to per-item drop-and-count.What was deliberately not touched
getChunkedAssetsDatastill succeeds if any chunk resolves. Portfolio distribution depends on it.id: ""still fails the entire query. Pinned as current behaviour; fixed in LIVE-35233.getEnv("DADA_API_*")fromdomain/api, wheredomain/api/README.mdassigns it to@shared/api-services. Not behaviour-neutral, so it is scoped to LIVE-35301.dadaApi. Scoped to LIVE-35230.Fixtures
The three mocks moved to
src/fixtures/, keeping the.mock.tssuffix so knip's**/*.mock.*ignore still applies and the./mock*export names stay unchanged — no consumer moved.AssetsData, socryptoOrTokenCurrenciesholds real entities — whatgetCryptoCurrencyByIdreturns. The three hand-built ones had drifted:injectiveclaimedfamily: "injective"with one unit where the registry says"cosmos"with two.getCryptoCurrencyByIdoverfindCryptoCurrencyByIdon purpose: it throws, which is what a fixture should do if the CAL drops an id.buildCategoryResponsereplaces the hand-written category fixtures, so a new category is a list of assets rather than another file. Stablecoins collapses to a ticker list. Stocks keeps explicit slugs because they are not derivable from the tickers —AAPLXisapplex, notaaplx. I caught that by serialising both fixtures before the refactor and diffing after; with explicit slugs the output is byte-identical, so the MSW handlers and the desktop Portfolio integration test see no change.Verification
domain/api/aggregated-assets/src/transforms.ts, which confirms they exercise the moved code through the shims rather than passing vacuously.resolveBaseUrl,fetchAssetsPage,allSettled,assertDadaApiUrl, and this package'sdadaIdToMarketId.collectAllByCategorywas exercised only indirectly and now has direct tests.assertDadaApiUrlmattered most — it is the host guard for the endpoints that build their ownfetchinstead of going throughbaseQuery.libs/ledger-live-commontypechecks with 0 errors under CI's exact command; all five packages typecheck.pnpm install --frozen-lockfilesucceeds.Pre-existing issues encountered, not caused by this PR
libs/ui/packages/react/libis unbuilt, so all 62 suites fail intests/jestSetup.json@ledgerhq/react-ui/assets/fonts. Verified consumers via desktop typecheck instead.dada-client/entities/selectorUtils.ts:34(untyped cache scan). That file is byte-identical to develop and imports only@reduxjs/toolkit, so it is pre-existing — and a fitting demonstration of why this code is guarded by tests rather than types.libs/asset-detailreports 118 implicit-any errors, all in unrelated live-common files. None mention this code.Sonar Cloudjob passes; the findings predate this PR.❓ Context
🧐 Checklist for the PR Reviewers