Page MenuHomePhabricator
Differential D17288

[Cashtab] Convert wallet context to ts
ClosedPublic
Actions

Authored by bytesofman on Tue, Dec 3, 21:11.

Details

Reviewers
emack
Group Reviewers
Restricted Project
Commits
rABCd42543241000: [Cashtab] Convert wallet context to ts
Summary

Troubleshooting load speed issues in Cashtab. Need to get typescript at the highest level so I can find any mismatches. Start here.

Cashtab screens load important wallet information from Context. But, because type definitions have not been applied there, we aren't really getting a lot of typescript benefits on our screens.

Implement typescript on context file and at entry point of app. This required also implementing it in our GoogleAnalytics, which is loaded from this screen. We also see many "lint-like" changes in tsx files of app screens -- this is because, before context was updated, they were not actually getting the types of wallet context.

With typescript, implement improved condition check on context loading.

This is part of a broader effort to debottleneck Cashtab load times.

Test Plan

npm test, arc lint no ts errors

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bytesofman created this revision.Tue, Dec 3, 21:11
Herald added a reviewer: Restricted Project. · View Herald TranscriptTue, Dec 3, 21:11
Harbormaster completed remote builds in B31510: Diff 51386.Tue, Dec 3, 21:14
teamcity added a comment.Tue, Dec 3, 21:15
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

Installing ecash-agora dependencies...
/work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /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/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /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 1487 packages, and audited 3333 packages in 1m

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

4 vulnerabilities (2 moderate, 2 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

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

TS2339: Property 'ecc' does not exist on type 'UseWalletReturnType | null'.
    33 |     const ContextValue = React.useContext(WalletContext);
    34 |     const {
  > 35 |         ecc,
       |         ^^^
    36 |         fiatPrice,
    37 |         chronik,
    38 |         agora,


Build cashtab-tests failed with exit code 1
bytesofman updated this revision to Diff 51387.Tue, Dec 3, 23:02

more ts conversions, now need to handle undefined in context

Harbormaster completed remote builds in B31511: Diff 51387.Tue, Dec 3, 23:05
teamcity added a comment.Tue, Dec 3, 23:05
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

Installing ecash-agora dependencies...
/work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /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/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /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 3335 packages in 25s

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

4 vulnerabilities (2 moderate, 2 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

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

TS2339: Property 'wallets' does not exist on type 'CashtabState | undefined'.
    41 |         chaintipBlockheight,
    42 |     } = ContextValue;
  > 43 |     const { wallets, settings, cashtabCache } = cashtabState;
       |             ^^^^^^^
    44 |     const wallet = wallets.length > 0 ? wallets[0] : false;
    45 |     const pk =
    46 |         wallet === false ? null : wallet.paths.get(appConfig.derivationPath).pk;


Build cashtab-tests failed with exit code 1
bytesofman updated this revision to Diff 51403.Thu, Dec 5, 04:20

better type checking, propagating context type through tsx components (unfinished)

Harbormaster completed remote builds in B31524: Diff 51403.Thu, Dec 5, 04:24
teamcity added a comment.Thu, Dec 5, 04:24
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

Installing ecash-agora dependencies...
/work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /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/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /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 3335 packages in 24s

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

4 vulnerabilities (2 moderate, 2 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

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

TS18048: 'ReactGA' is possibly 'undefined'.
    27 |               const location = useLocation();
    28 |               useEffect(() => {
  > 29 |                   ReactGA.pageview(location.pathname + location.search);
       |                   ^^^^^^^
    30 |               }, [location]);
    31 |               return null;
    32 |           };


Build cashtab-tests failed with exit code 1
bytesofman updated this revision to Diff 51407.Thu, Dec 5, 13:39

fixing build lint, test format patch

bytesofman edited the summary of this revision. (Show Details)Thu, Dec 5, 13:41
bytesofman edited the summary of this revision. (Show Details)Thu, Dec 5, 13:43
Harbormaster completed remote builds in B31528: Diff 51407.Thu, Dec 5, 13:43
bytesofman published this revision for review.Thu, Dec 5, 14:19
emack requested changes to this revision.Thu, Dec 5, 14:45
emack added a subscriber: emack.
emack added inline comments.
cashtab/src/components/Agora/index.tsx
50 ↗(On Diff #51407)
cashtab/src/components/Etokens/CreateTokenForm/index.tsx
88 ↗(On Diff #51407)

Instead of returning null, should this be returning a standard error component instead of a blank page?

cashtab/src/components/Etokens/Token/index.tsx
182 ↗(On Diff #51407)

at this point is the tokenid, if exists, guaranteed to always being a valid 64 char hex?

1634 ↗(On Diff #51407)

This is still a technical possibility isn't it? Don't think this affects performance.

2680 ↗(On Diff #51407)

is this a linting thing? seems redundant if you're checking boolean

This revision now requires changes to proceed.Thu, Dec 5, 14:45
bytesofman marked 5 inline comments as done.Thu, Dec 5, 14:52
bytesofman added inline comments.
cashtab/src/components/Etokens/CreateTokenForm/index.tsx
88 ↗(On Diff #51407)

apparently this is the right way to do it.

It's a bit moot since in practice I do not think context ever fails to load. I do not know how this would happen since it's not something I'm able to recreate.

cashtab/src/components/Etokens/Token/index.tsx
182 ↗(On Diff #51407)

no -- it just needs to be any string

we validate it right below on line 187, and have a test setup to show we render expected component if this is not valid

the route only renders if we have some kind of string for tokenId

1634 ↗(On Diff #51407)

this is no longer possible. we do not render the component at all if context does not load (return null above). if context loads, then pk is defined, and changescript is defined.

2680 ↗(On Diff #51407)

yes, it's a ts issue

isuse is that nftListPriceError is either false or string -- but disabled will only take booleans.

it "works" because a string is truthy. But nftListPriceError !== false is the "right" way to pass the boolean, which ts requires.

bytesofman updated this revision to Diff 51409.Thu, Dec 5, 14:53
bytesofman marked 4 inline comments as done.

typo patch

Harbormaster completed remote builds in B31529: Diff 51409.Thu, Dec 5, 14:57
emack accepted this revision.Thu, Dec 5, 21:08
This revision is now accepted and ready to land.Thu, Dec 5, 21:08
Closed by commit rABCd42543241000: [Cashtab] Convert wallet context to ts (authored by bytesofman). · Explain WhyThu, Dec 5, 22:15
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABCd42543241000: [Cashtab] Convert wallet context to ts.
bytesofman added a reverting change: D17297: Revert "[Cashtab] Convert wallet context to ts".Thu, Dec 5, 22:26
bytesofman added a reverting change: rABCb88a433009ea: Revert "[Cashtab] Convert wallet context to ts".Thu, Dec 5, 22:41

Revision Contents
Changeset List

PathSize
cashtab/
29 lines
4 lines
src/
components/
Agora/
OrderBook/
2 lines
19 lines
Common/
73 lines
Etokens/
20 lines
CreateTokenForm/
30 lines
Token/
238 lines
__tests__/
2 lines
Nfts/
27 lines
Send/
20 lines
49 lines
wallet/
100 lines
3 lines
38 lines

Diff 51421

View Options

cashtab/package-lock.json

Loading...
View Options

cashtab/package.json

Loading...
View Options

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

Loading...
View Options

cashtab/src/components/Agora/index.tsx

Loading...
View Options

cashtab/src/components/Common/GoogleAnalytics.js

Loading...
View Options

cashtab/src/components/Common/GoogleAnalytics.ts

Loading...
View Options

cashtab/src/components/Etokens/CreateToken.tsx

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

cashtab/src/components/Etokens/__tests__/TokenActions.test.js

Loading...
View Options

cashtab/src/components/Nfts/index.tsx

Loading...
View Options

cashtab/src/components/Send/SendXec.tsx

Loading...
View Options

cashtab/src/index.js

Loading...
View Options

cashtab/src/index.tsx

Loading...
View Options

cashtab/src/wallet/context.js

Loading...
View Options

cashtab/src/wallet/context.tsx

Loading...
View Options

cashtab/src/wallet/index.ts

Loading...
View Options

cashtab/src/wallet/useWallet.ts

Loading...