Page MenuHomePhabricator
Differential D17499

[Cashtab] useContext in OrderBook instead of prop drilling
ClosedPublic
Actions

Authored by bytesofman on Thu, Jan 9, 06:16.

Details

Reviewers
emack
Group Reviewers
Restricted Project
Commits
rABCdadc2ba7c3f4: [Cashtab] useContext in OrderBook instead of prop drilling
Summary

Props that are available from context should come from context and not "prop drilling", aka passing them down from the parent component. Prop drilling can cause performance issues esp for stuff like agora where the component is complex and we might want to load 100s at a time.

I originally wanted to just add websockets to Orderbook in this diff but ran into this as tech debt.

Have to get context first so we can just use our existing websocket. Had to upgrade testing to wrap OrderBook with context.

Cashtab testing has not really migrated to typescript yet but it should. This will also be done incrementally because Cashtab has all kinds of mocks and vectors that need to be typed. Imo the impact is worthwhile (easier to debug tests, easier to write tests, cleaner mocks, standardized mocks make new tests easier to write).

Test Plan

npm test

later diff in this stack is deployed at https://cashtab-local-dev.netlify.app/

Diff Detail

Repository
rABC Bitcoin ABC
Branch
orderbook-websockets
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 31996
Build 63484: Build Diffcashtab-tests
Build 63483: arc lint + arc unit

Event Timeline

bytesofman created this revision.Thu, Jan 9, 06:16
Herald added a reviewer: Restricted Project. · View Herald TranscriptThu, Jan 9, 06:16
Harbormaster completed remote builds in B31986: Diff 52057.Thu, Jan 9, 06:20
teamcity added a comment.Thu, Jan 9, 06:20
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

Run `npm audit` for details.

> ecash-lib@1.2.1 build
> tsc && tsc -p ./tsconfig.build.json && cp -r ./src/ffi ./dist

/work/modules/ecash-agora /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 2s

60 packages are looking for funding
  run `npm fund` for details

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@0.2.0 build
> tsc && tsc -p ./tsconfig.build.json

/work/cashtab /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3310 packages in 25s

320 packages are looking for funding
  run `npm fund` for details

8 vulnerabilities (6 moderate, 2 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

> cashtab@3.10.7 build
> node scripts/build.js

Creating an optimized production build...
Failed to compile.

TS2739: Type '{ xecTxType: XecTxType; satoshisSent: number; stackArray: string[]; recipients: string[]; }' is missing the following properties from type 'ParsedTx': appActions, parsedTokenEntries
    201 |                 tokenFailedParsings: [],
    202 |                 tokenStatus: 'TOKEN_STATUS_NORMAL',
  > 203 |                 parsed: {
        |                 ^^^^^^
    204 |                     xecTxType: 'Received' as XecTxType,
    205 |                     satoshisSent: 546,
    206 |                     stackArray: [


Build cashtab-tests failed with exit code 1
bytesofman added a comment.Thu, Jan 9, 06:21
  • Implement the new OrderBook on token and agora screens
  • Fix all the types in the new mocks.ts in this diff
  • Consider pulling out the type patch in CashtabWalletPaths into its own diff
bytesofman updated this revision to Diff 52066.Thu, Jan 9, 12:48

finish implementing ts in mocks, remove unnecessary memory router from orderbook test wrapper

Harbormaster completed remote builds in B31993: Diff 52066.Thu, Jan 9, 12:53
teamcity added a comment.Thu, Jan 9, 12:53
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

Run `npm audit` for details.

> ecash-lib@1.2.1 build
> tsc && tsc -p ./tsconfig.build.json && cp -r ./src/ffi ./dist

/work/modules/ecash-agora /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 2s

60 packages are looking for funding
  run `npm fund` for details

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@0.2.0 build
> tsc && tsc -p ./tsconfig.build.json

/work/cashtab /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3310 packages in 25s

320 packages are looking for funding
  run `npm fund` for details

8 vulnerabilities (6 moderate, 2 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

> cashtab@3.10.8 build
> node scripts/build.js

Creating an optimized production build...
Failed to compile.

TS2322: Type '{ key: string; tokenId: string; cachedTokenInfo: CashtabCachedTokenInfo | undefined; settings: CashtabSettings; userLocale: string; fiatPrice: number | null; ... 5 more ...; chaintipBlockheight: number; }' is not assignable to type 'IntrinsicAttributes & OrderBookProps'.
  Property 'cachedTokenInfo' does not exist on type 'IntrinsicAttributes & OrderBookProps'.
    424 |                                                                     offeredTokenId
    425 |                                                                 }
  > 426 |                                                                 cachedTokenInfo={cashtabCache.tokens.get(
        |                                                                 ^^^^^^^^^^^^^^^
    427 |                                                                     offeredTokenId,
    428 |                                                                 )}
    429 |                                                                 settings={


Build cashtab-tests failed with exit code 1
bytesofman updated this revision to Diff 52067.Thu, Jan 9, 12:59

actually implement the new component

Harbormaster completed remote builds in B31994: Diff 52067.Thu, Jan 9, 13:03
teamcity added a comment.Thu, Jan 9, 13:03
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

> ecash-lib@1.2.1 build
> tsc && tsc -p ./tsconfig.build.json && cp -r ./src/ffi ./dist

/work/modules/ecash-agora /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 1s

60 packages are looking for funding
  run `npm fund` for details

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@0.2.0 build
> tsc && tsc -p ./tsconfig.build.json

/work/cashtab /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3310 packages in 25s

320 packages are looking for funding
  run `npm fund` for details

8 vulnerabilities (6 moderate, 2 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

> cashtab@3.10.8 build
> node scripts/build.js

Creating an optimized production build...

Treating warnings as errors because process.env.CI = true.
Most CI servers set it automatically.

Failed to compile.

[eslint] 
src/components/Agora/index.tsx
  Line 41:9:   'ecc' is assigned a value but never used                  @typescript-eslint/no-unused-vars
  Line 42:9:   'fiatPrice' is assigned a value but never used            @typescript-eslint/no-unused-vars
  Line 47:9:   'chaintipBlockheight' is assigned a value but never used  @typescript-eslint/no-unused-vars
  Line 49:22:  'settings' is assigned a value but never used             @typescript-eslint/no-unused-vars


Build cashtab-tests failed with exit code 1
bytesofman updated this revision to Diff 52068.Thu, Jan 9, 13:06

lint, comment on token cache behavior

Harbormaster completed remote builds in B31995: Diff 52068.Thu, Jan 9, 13:12
bytesofman updated this revision to Diff 52069.Thu, Jan 9, 14:08

remove the ref stuff, do not get cache info from orderbook results in double calls, we already have too many calls

Harbormaster completed remote builds in B31996: Diff 52069.Thu, Jan 9, 14:13
teamcity added a comment.Thu, Jan 9, 14:13
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

Run `npm audit` for details.

> ecash-lib@1.2.1 build
> tsc && tsc -p ./tsconfig.build.json && cp -r ./src/ffi ./dist

/work/modules/ecash-agora /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 2s

60 packages are looking for funding
  run `npm fund` for details

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@0.2.0 build
> tsc && tsc -p ./tsconfig.build.json

/work/cashtab /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3310 packages in 24s

320 packages are looking for funding
  run `npm fund` for details

8 vulnerabilities (6 moderate, 2 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

> cashtab@3.10.8 build
> node scripts/build.js

Creating an optimized production build...

Treating warnings as errors because process.env.CI = true.
Most CI servers set it automatically.

Failed to compile.

[eslint] 
src/components/Agora/OrderBook/index.tsx
  Line 24:50:  'useRef' is defined but never used                  @typescript-eslint/no-unused-vars
  Line 95:10:  'CashtabCachedTokenInfo' is defined but never used  @typescript-eslint/no-unused-vars


Build cashtab-tests failed with exit code 1
bytesofman updated this revision to Diff 52072.Thu, Jan 9, 14:36

lint

Harbormaster completed remote builds in B31997: Diff 52072.Thu, Jan 9, 14:49
bytesofman published this revision for review.Thu, Jan 9, 14:59
bytesofman added a child revision: D17500: [Cashtab] Lazy loading for agora.Thu, Jan 9, 15:04
bytesofman edited the test plan for this revision. (Show Details)Thu, Jan 9, 15:06
emack accepted this revision.Fri, Jan 10, 03:02
This revision is now accepted and ready to land.Fri, Jan 10, 03:02
Closed by commit rABCdadc2ba7c3f4: [Cashtab] useContext in OrderBook instead of prop drilling (authored by bytesofman). · Explain WhyFri, Jan 10, 03:41
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABCdadc2ba7c3f4: [Cashtab] useContext in OrderBook instead of prop drilling.
bytesofman mentioned this in D17505: [Cashtab] Patch CI by including mock-chronik-client.Fri, Jan 10, 04:56
bytesofman mentioned this in rABC4d1dc91c44b4: [Cashtab] Patch CI by including mock-chronik-client.Fri, Jan 10, 05:08

Revision Contents
Changeset List

PathSize
cashtab/
src/
components/
Agora/
OrderBook/
__tests__/
index.test.tsx
index.test.js
641 lines
60 lines
fixtures/
mocks.ts
mocks.js
335 lines
57 lines
Etokens/
Token/
11 lines
test/
183 lines
wallet/
9 lines

Diff 52069

View Options

cashtab/src/components/Agora/OrderBook/__tests__/index.test.js

Loading...
View Options

cashtab/src/components/Agora/OrderBook/__tests__/index.test.tsx

Loading...
View Options

cashtab/src/components/Agora/OrderBook/index.tsx

Loading...
View Options

cashtab/src/components/Agora/fixtures/mocks.js

Loading...
View Options

cashtab/src/components/Agora/fixtures/mocks.ts

Loading...
View Options

cashtab/src/components/Agora/index.tsx

Loading...
View Options

cashtab/src/components/Etokens/Token/index.tsx

Loading...
View Options

cashtab/src/test/index.tsx

Loading...
View Options

cashtab/src/wallet/index.ts

Loading...