Skip to content
This repository has been archived by the owner on Jun 27, 2022. It is now read-only.

LL-8827 Introduce types libraries #748

Merged
merged 2 commits into from
Jan 4, 2022
Merged

LL-8827 Introduce types libraries #748

merged 2 commits into from
Jan 4, 2022

Conversation

gre
Copy link
Contributor

@gre gre commented Jan 3, 2022

This PR introduces 3 new libraries.

  • @ledgerhq/types-live (Account type, Operation, …)
  • @ledgerhq/types-cryptoassets (Currency, TokenCurrency,..)
  • @ledgerhq/types-devices (Transport, Device,...)

in order to do more decoupling between our types and the actual implementations and in a future goal to dedup our libraries.

https://ledgerhq.atlassian.net/browse/LL-8827

@gre
Copy link
Contributor Author

gre commented Jan 3, 2022

re how do we make a Account type that would evict the family specifics but do not create type issues literally everywhere when we need it? (e.g. bitcoinResources?: BitcoinResources) =>

One possible idea, is to have some subtyping like this:

export type BitcoinAccount = Account & { bitcoinResources: BitcoinResources }

and all parts where we would need it, we would need to cast,

const bitcoinAccount = account as BitcoinAccount;

and that BitcoinAccount type would be part of a coin integration family specific indeed.

@gre gre changed the title Bootstrap all types Introduction types libraries Jan 3, 2022
@gre gre changed the title Introduction types libraries Introduce types libraries Jan 3, 2022
@gre gre changed the title Introduce types libraries LL-8827 Introduce types libraries Jan 3, 2022
@juan-cortes
Copy link
Contributor

What is wrong with your proposed idea? I find it elegant and explicit enough 😕

@ghost
Copy link

ghost commented Jan 4, 2022

I don't see any issue either with that proposal.

@gre
Copy link
Contributor Author

gre commented Jan 4, 2022

@juan-cortes @haammar-ledger ok, then I can check my checkbox :) I'll commit the removing of the comments.

@gre gre marked this pull request as ready for review January 4, 2022 09:33
@gre gre requested a review from a team as a code owner January 4, 2022 09:33
@codecov
Copy link

codecov bot commented Jan 4, 2022

Codecov Report

Merging #748 (c53145a) into master (d40c77c) will decrease coverage by 0.11%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #748      +/-   ##
==========================================
- Coverage   45.00%   44.88%   -0.12%     
==========================================
  Files          81       83       +2     
  Lines        4911     4924      +13     
  Branches      811      812       +1     
==========================================
  Hits         2210     2210              
- Misses       2684     2697      +13     
  Partials       17       17              
Impacted Files Coverage Δ
packages/types-devices/src/index.ts 0.00% <0.00%> (ø)
packages/types-live/src/index.ts 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d40c77c...c53145a. Read the comment docs.

@gre gre merged commit 4598bea into master Jan 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants