Page MenuHomePhabricator

[Cashtab] Convert Etokens screen to ts
ClosedPublic

Authored by bytesofman on Tue, Dec 24, 14:24.

Details

Reviewers
emack
Group Reviewers
Restricted Project
Commits
rABCe2784416ff59: [Cashtab] Convert Etokens screen to ts
Summary

Incremental implementation of ts in Cashtab

Test Plan

npm test, npm run build for ts lint

Diff Detail

Repository
rABC Bitcoin ABC
Branch
ts-etokens
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 31756
Build 63006: Build Diffcashtab-tests
Build 63005: arc lint + arc unit

Event Timeline

Tail of the build log:

> ecash-lib@1.2.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/b58-ts /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/mock-chronik-client /work/modules/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/b58-ts /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/mock-chronik-client /work/modules/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 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.4.4 build
> node scripts/build.js

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

TS2322: Type '({ tokensKvArray, userLocale }: TokenListProps) => Element[]' is not assignable to type 'FC<TokenListProps>'.
  Type 'Element[]' is missing the following properties from type 'ReactElement<any, any>': type, props, key
    18 |     userLocale: string;
    19 | }
  > 20 | const TokenList: React.FC<TokenListProps> = ({ tokensKvArray, userLocale }) => {
       |       ^^^^^^^^^
    21 |     return tokensKvArray.map(keyValueArray => (
    22 |         <TokenLink key={keyValueArray[0]} to={`/token/${keyValueArray[0]}`}>
    23 |             <TokenListItem


Build cashtab-tests failed with exit code 1

patch bug in component def caught by tsc

emack requested changes to this revision.Wed, Dec 25, 00:01
emack added a subscriber: emack.
emack added inline comments.
cashtab/src/components/Etokens/Etokens.tsx
219–230

i can't tell if this is a ts lint thing, if not, kinda redundant specifying null

This revision now requires changes to proceed.Wed, Dec 25, 00:01

use same type check bail-out throughout component

cashtab/src/components/Etokens/Etokens.tsx
219–230

in this case, we are assigning a value to searchFileteredTokens

If userFilteredTokens is null, then searchFilteredTokens should also be null

If userFilteredTokens is not null -- i.e. it has loaded and is of its expected type -- then we can filter it appopriately.

It is somewhat a ts-lint thing. We did not have this check in the function before. Because userFilteredTokens can be null, it is possible for the function to try to filter on null without this check.

In practice, we do not really see this happen ever, because if userFilteredTokens is null then there isn't really anything to search.

Still, this instance is confusing. Updated to match the similar type check bailout implemented on this component.

Alternatively we could assert this will not be null if we are at this stage of the component lifecycle, i.e. tokens has loaded. But this is a complex component and performing an extra check or two to prevent the render method from throwing an error is imo the best practice.

This revision is now accepted and ready to land.Sat, Dec 28, 23:26
This revision was automatically updated to reflect the committed changes.