chore(assets-data): scaffold the DDD target packages (LIVE-35225) - #20285
Conversation
LucasWerey
changed the title
Web Tools Build Status
|
Create the four empty packages that libs/ledger-live-common/src/dada-client will move into (LIVE-35223). Scaffolding only: no code moved, no behaviour change. @domain/entity-asset crypto-asset meta, network info, currency order @domain/entity-interest-rate interest rate shape and its APY type @domain/api-assets-data DADA wire contract, RTK Query, transforms @features/platform-assets-data hooks, cache selectors, discovery, selection The React layer sits in features/platform rather than features/flow because Market, Portfolio, Global Search and the asset selectors all consume it. READMEs carry the decisions instead of a docs/ page, since the repo has no ADR convention: why wire types belong to the api package, why markets stays loosely typed with no domain/entity/market, the copied dadaIdToMarketId debt, and the invariants that break with no type error. Registered in knip.json per the born-migrated rule for new packages.
There was a problem hiding this comment.
🟡 Not ready to approve
The new platform package’s Jest setup is incomplete for jest-dom usage, and the lockfile diff includes non-scaffold-related metadata churn that should be minimized or explained.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR scaffolds new DDD workspaces to host the upcoming DADA (Dynamic Assets Data Aggregator) migration out of live-common, creating placeholder domain/entity, domain/api, and features/platform packages with initial tooling/config.
Changes:
- Add new empty DDD packages:
@domain/entity-asset,@domain/entity-interest-rate, and@domain/api-assets-data. - Add a new app-facing platform layer package
@features/platform-assets-data(currently scaffolded/empty). - Register the new workspaces in
knip.json, add a changeset, and update the lockfile for the new importers.
File summaries
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds new workspace importers for the scaffolded packages; also updates some existing package snapshot/peer metadata. |
| knip.json | Registers the new workspaces for knip entrypoints. |
| features/platform/assets-data/tsconfig.json | TypeScript config for the new platform package. |
| features/platform/assets-data/src/index.ts | Placeholder entrypoint. |
| features/platform/assets-data/README.md | Documents intended responsibilities and constraints of the platform layer. |
| features/platform/assets-data/project.json | Nx project scaffold (noop build). |
| features/platform/assets-data/package.json | Workspace manifest, scripts, and dependencies for the platform package. |
| features/platform/assets-data/jest.config.js | Jest config scaffold for the platform package. |
| domain/entity/interest-rate/tsconfig.json | TypeScript config for the new entity package. |
| domain/entity/interest-rate/src/index.ts | Placeholder entrypoint. |
| domain/entity/interest-rate/README.md | Documents intended scope and future migration source. |
| domain/entity/interest-rate/project.json | Nx project scaffold (noop build). |
| domain/entity/interest-rate/package.json | Workspace manifest and scripts for the interest-rate entity package. |
| domain/entity/interest-rate/jest.config.js | Jest config scaffold for the interest-rate entity package. |
| domain/entity/asset/tsconfig.json | TypeScript config for the new entity package. |
| domain/entity/asset/src/index.ts | Placeholder entrypoint. |
| domain/entity/asset/README.md | Documents intended scope and future migration source. |
| domain/entity/asset/project.json | Nx project scaffold (noop build). |
| domain/entity/asset/package.json | Workspace manifest and scripts for the asset entity package. |
| domain/entity/asset/jest.config.js | Jest config scaffold for the asset entity package. |
| domain/api/assets-data/tsconfig.json | TypeScript config for the new API package. |
| domain/api/assets-data/src/index.ts | Placeholder entrypoint. |
| domain/api/assets-data/README.md | Documents the planned cross-entity contract and invariants for DADA integration. |
| domain/api/assets-data/project.json | Nx project scaffold (noop build). |
| domain/api/assets-data/package.json | Workspace manifest and scripts for the assets-data API package. |
| domain/api/assets-data/jest.config.js | Jest config scaffold for the assets-data API package. |
| .changeset/tidy-pandas-gather.md | Changeset documenting the introduction of the new scaffolded packages. |
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
- Files reviewed: 26/27 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
fcc77ff to
64a63a9
Compare
There was a problem hiding this comment.
🟡 Not ready to approve
The new React package Jest scaffold includes jest-dom types/deps but doesn’t load jest-dom in setup, and the lockfile includes unrelated churn that should be minimized or split.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (2)
features/platform/assets-data/jest.config.js:5
@testing-library/jest-domis included (devDependency + TS types), but the Jest config doesn't load it viasetupFilesAfterEnv, unlike otherfeatures/platform/*packages (e.g.features/platform/currencies/jest.config.js). Without this, common matchers liketoBeInTheDocument()won't be registered at runtime when tests are added.
module.exports = {
testEnvironment: "jsdom",
roots: ["<rootDir>/src"],
testMatch: ["**/*.test.ts?(x)"],
transform: {
pnpm-lock.yaml:2206
- This lockfile hunk changes
@react-native/babel-presetresolution to include an explicit(@babel/core@...)peer param even though this PR only scaffolds new DDD workspace packages (Jest/SWC/TS) and does not touch any React Native deps. Per repo review guidance, consider splitting unrelated lockfile churn into a separate PR (or regenerate the lockfile in a way that only reflects the new workspaces) to keep the diff reviewable and reduce merge conflicts.
- Files reviewed: 26/27 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
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:
📁 desktop-workersPath:
📁 mobilePath:
Generated by Rsdoctor GitHub Action |
The package is jsdom and its tsconfig declares @testing-library/jest-dom types,
but nothing loaded the matchers at runtime, so a test using toBeInTheDocument
would have compiled and then failed. Adds jest.setup.js + setupFilesAfterEnv,
matching features/platform/{style,currencies,feature-flags}.
Also regenerates pnpm-lock.yaml from the base commit rather than from the state
left by an interrupted install.
There was a problem hiding this comment.
🟢 Ready to approve
The changes are isolated scaffolding consistent with existing DDD package patterns and introduce no runtime behavior changes.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
- Files reviewed: 27/28 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
🟢 Ready to approve
The changes are limited to consistent scaffolding of new private workspaces plus expected knip/lockfile wiring, with no runtime code movement or behavior changes.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
- Files reviewed: 27/28 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
Note
Stacked PR — base is #20276, not
develop. Review that one first; merge it first too.✅ Checklist
npx changesetwas attached.typecheckandjesttargets are verified to run, which is all there is to assert at this stage. Code and its tests arrive in LIVE-35226 / LIVE-35227.- No runtime impact. Four new private workspace packages, all empty (
export {};). No existing file was modified exceptknip.json(+12) andpnpm-lock.yaml.- Nothing imports these packages yet, so no app behaviour can change.
- No QA pass needed.
📝 Description
Problem.
libs/ledger-live-common/src/dada-clientis being migrated into the DDD layers under LIVE-35223. The relocation itself (LIVE-35226 / LIVE-35227) is a large diff, so the destination packages are created first, empty, in their own reviewable step.Solution. Scaffold four packages per the
docs/new-library.mdchecklist. No code moved.domain/entity/asset@domain/entity-assetCryptoAssetMeta,NetworkInfo,CurrenciesOrderdomain/entity/interest-rate@domain/entity-interest-rateInterestRate,ApyTypedomain/api/assets-data@domain/api-assets-datafeatures/platform/assets-data@features/platform-assets-dataEach gets
package.json,tsconfig.json,jest.config.js,project.json,README.mdwith a status marker, and a placeholdersrc/index.ts. The jsdom package additionally getsjest.setup.jsloading@testing-library/jest-dom, matchingfeatures/platform/{style,currencies,feature-flags}.Decisions worth reviewing
assets-data, notdada, per the naming table indocs/new-library.md.features/platform, notfeatures/flow. Market, Portfolio, Global Search and the asset/network selectors all consume it, so it is a capability shared across flows rather than one journey's internals.ApiAsset/RawApiResponsedescribe DADA's transport format; entities own only canonical shapes. Mirrorsdomain/api/market-sentiment+domain/entity/market-sentiment.marketsstays loosely typed — nodomain/entity/market. It would roughly double the migration and pull in another team's surface, and sincePartialMarketItemResponseisPartial<MarketItemResponse>there is nothing meaningful to validate. Documented as an explicit scope boundary.docs/page, because this repo has no ADR convention (nothing indocs/about-docs,docs/contributing,AGENTS.md, or thedocs-update-or-createskill). A reader of the package finds the rationale without needing the Jira epic. The@domain/api-assets-dataREADME records the accepteddadaIdToMarketIdcopy debt and the two leniency invariants; the@features/platform-assets-dataREADME records the hooks-only API, the frozenreducerPathstring, and the cross-query cache-scan quirk.knip.json(+12 lines, oneworkspacesentry each) per the knip-migration skill's "new package = born migrated" rule. Exports are explicit with no./*wildcard. Verified:pnpm run unimportedindomain/api/assets-dataruns clean.Notes for reviewers
0.1.0on all four, matching themarket-sentimentprecedent where the creating PR carried aminorchangeset (0.1.0→0.2.0).testscripts use--passWithNoTestsso the empty packages don't fail their targets. Remove when real tests land in LIVE-35226 / LIVE-35227.zod,@reduxjs/toolkit,@shared/envand the currency entities are added in the PR that moves the code in, so this PR doesn't declare deps nothing imports.@features/platform-assets-datadoes carryreact/react-reduxpeers, matchingfeatures/platform/currencies.pnpm-lock.yamlhas 5 deleted lines beyond the new importers, and they are explainable rather than avoidable. Regenerated from the base commit, which cut the diff from 56 deletions to 5. Two experiments:--lockfile-onlyon the base branch (which adds no packages) produces zero churn in 693ms, so the committed lockfile is internally consistent; resetting this branch's lockfile to the base and regenerating reproduces the same churn in 34.7s. So it is pnpm re-resolving more broadly when the workspace set changes (203 → 207 projects), which re-keys@storybook/addon-react-native-weband materialises the@babel/*snapshots it pulls in. Two of the five deletions aren't dependency changes at all — npm changed its own deprecation text upstream for@apollo/serverandconventional-changelog. No version changed for any existing package.packageManagerpinspnpm@10.24.0and that is the version used.nx show projectslists all four,commitlint --from developpasses across the stack.❓ Context
🧐 Checklist for the PR Reviewers