Page MenuHomePhabricator

[Cashtab] Convert Tx screen and its deps to typescript
ClosedPublic

Authored by bytesofman on Sun, Dec 8, 05:11.

Details

Reviewers
emack
Group Reviewers
Restricted Project
Commits
rABC2c10d5fb3e97: [Cashtab] Convert Tx screen and its deps to typescript
Summary

As prep for refactoring for speed

Test Plan

npm test

Diff Detail

Repository
rABC Bitcoin ABC
Branch
tx-to-ts
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 31580
Build 62657: Build Diffcashtab-tests
Build 62656: arc lint + arc unit

Event Timeline

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 24s

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

6 vulnerabilities (4 moderate, 2 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

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

TS2305: Module '"ecash-lib"' has no exported member 'AddressType'.
    103 | import appConfig from 'config/app';
    104 | import { scriptOps } from 'ecash-agora';
  > 105 | import { Script, fromHex, OP_0, AddressType } from 'ecash-lib';
        |                                 ^^^^^^^^^^^
    106 | import CashtabState, { CashtabContact } from 'config/CashtabState';
    107 | import CashtabCache from 'config/CashtabCache';
    108 | import { CashtabCacheJson, StoredCashtabWallet } from 'helpers';


Build cashtab-tests failed with exit code 1
bytesofman published this revision for review.Sun, Dec 8, 05:51
emack requested changes to this revision.Sun, Dec 8, 09:34
emack added a subscriber: emack.
emack added inline comments.
cashtab/src/components/Home/Tx/index.tsx
111
131
1170

this isn't a ts related change is it?

cashtab/src/components/Home/TxHistory.tsx
22
This revision now requires changes to proceed.Sun, Dec 8, 09:34
bytesofman marked 4 inline comments as done.
bytesofman added inline comments.
cashtab/src/components/Home/Tx/index.tsx
111

this does seem right. I don't know if it's eslint or prettier but the stack of "always start types with man | with a leading | " is coming from one of them.

I don't have a strong enough opinion about it to overrule the linter.

131

same

801

here is another fix that is not directly related to ts but indicative of why we need ts in cashtab

ts caught this typo bug.

1170

this artifact was not being used bc there is no style prop accepted by the CopyIconButton component. Typescript picked up on this.

So, here we are removing dead code picked up by ts (and ts linter will fail if we leave it in, so it needs to come out). It probably used to work before the CopyIconButton was moved out of antd.

Can confirm by checking this component in prod dev console. There is no set z-index.

cashtab/src/components/Home/TxHistory.tsx
22

ditto as above, this is (strangely mb) the linter/formatter rule

This revision is now accepted and ready to land.Tue, Dec 10, 07:57
This revision was automatically updated to reflect the committed changes.
bytesofman marked 4 inline comments as done.