Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strictly Explicit: Enable TypeScript strict mode #333

Merged
merged 31 commits into from
Oct 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
80b2c03
Component props are no longer optional when defaults are provided
Shadowfiend Oct 15, 2021
0179130
Allow and handle optionally undefined UI event handlers
Shadowfiend Oct 15, 2021
289f708
Type react-router-dom location parameters more precisely
Shadowfiend Oct 15, 2021
d889ae5
Tighten up renameAndPickKeys types further
Shadowfiend Oct 15, 2021
b0e7c49
Make onboarding useState types more explicit
Shadowfiend Oct 15, 2021
5d0056d
Explicitly handle the possibility of null/undefined in UI code
Shadowfiend Oct 15, 2021
5567118
Tighten up a few types in UI code
Shadowfiend Oct 15, 2021
caf7a8d
Adjust the way global.crypto is polyfilled in tests for strict mode
Shadowfiend Oct 15, 2021
0dad9f0
Make webpack config strict-mode compatible
Shadowfiend Oct 15, 2021
aa7a6b0
Make service types strict-mode compatible
Shadowfiend Oct 15, 2021
2589860
Add explicit null variants to a few types
Shadowfiend Oct 15, 2021
90974fe
Add AnyEVMBlock and use it in most places
Shadowfiend Oct 15, 2021
103be3c
Improve and strictify transaction type hierarchy
Shadowfiend Oct 16, 2021
91adb6b
Adjust JTD validation for strict mode
Shadowfiend Oct 16, 2021
7f17ead
Adjust price validation for strict mode
Shadowfiend Oct 16, 2021
551d82a
Handle a few simple undefined/null possibilities
Shadowfiend Oct 18, 2021
72cd959
Rework redux initialization for strict initialization requirements
Shadowfiend Oct 18, 2021
e39fe70
Adapt accounts slice lookup to deal with `to` not being required
Shadowfiend Oct 18, 2021
9d319d9
Specify a tuple type instead of an array type for pricesToSort
Shadowfiend Oct 18, 2021
5b9c67a
Adjust AsyncThunk utilities for strict mode
Shadowfiend Oct 18, 2021
535a74e
Slight type adjustments in KeyringService to handle nulls
Shadowfiend Oct 18, 2021
e790884
Fix a place where we expected only Error objects to be thrown
Shadowfiend Oct 18, 2021
6adfe9e
Adjust to updated null/undefined typing in a few places
Shadowfiend Oct 18, 2021
2f33612
Adjust block lookup to reflect possibly-unconfirmed transaction
Shadowfiend Oct 18, 2021
57756a1
In case price lookup fails, log the original error
Shadowfiend Oct 18, 2021
0f30aa8
Rework Blocknative API key to disable block prices without a key
Shadowfiend Oct 18, 2021
1aab12a
Adjust logger to handle potentially-missing stack trace
Shadowfiend Oct 19, 2021
2639036
Enable TypeScript strict mode
Shadowfiend Oct 18, 2021
c789691
Fix a precision bug in indexing service price handling
Shadowfiend Oct 19, 2021
c657b80
Improve setShowingActivityDetail typing in UI slice
Shadowfiend Oct 19, 2021
6d72e59
Correctly pull validation errors into lazy wrappers
Shadowfiend Oct 20, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ module.exports = {
// The rule is also mostly irrelevant to this codebase due to TypeScript
// usage (where .tsx is required).
"react/jsx-filename-extension": [0],
// TypeScript allows us to declare props that are non-optional internally
// but are interpreted as optional externally if they have defaultProps
// defined; the following two adjustments disable eslint-plugin-react
// checks that predate this ability for TS and that no longer apply.
"react/default-props-match-prop-types": [
2,
{ allowRequiredDefaults: true },
],
"react/require-default-props": [0],
// Shared components may have labels associated externally in a way ESLint
// does not detect.
"jsx-a11y/label-has-associated-control": [
Expand Down
42 changes: 22 additions & 20 deletions background/lib/alchemy.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import Ajv, { JTDDataType } from "ajv/dist/jtd"
import {
AlchemyProvider,
AlchemyWebSocketProvider,
Expand All @@ -8,8 +7,7 @@ import { utils } from "ethers"
import logger from "./logger"
import { AssetTransfer, HexString, SmartContractFungibleAsset } from "../types"
import { ETH, ETHEREUM } from "../constants"

const ajv = new Ajv()
import { jtdValidatorFor } from "./validation"

// JSON Type Definition for the Alchemy assetTransfers API.
// https://docs.alchemy.com/alchemy/documentation/enhanced-apis/transfers-api
Expand Down Expand Up @@ -45,12 +43,9 @@ const alchemyGetAssetTransfersJTD = {
},
} as const

type AlchemyAssetTransferResponse = JTDDataType<
typeof alchemyGetAssetTransfersJTD
>

const isValidAlchemyAssetTransferResponse =
ajv.compile<AlchemyAssetTransferResponse>(alchemyGetAssetTransfersJTD)
const isValidAlchemyAssetTransferResponse = jtdValidatorFor(
alchemyGetAssetTransfersJTD
)

/**
* Use Alchemy's getAssetTransfers call to get historical transfers for an
Expand Down Expand Up @@ -144,7 +139,7 @@ export async function getAssetTransfers(
dataSource: "alchemy",
} as AssetTransfer
})
.filter((t) => t)
.filter((t): t is AssetTransfer => t !== null)
}

// JSON Type Definition for the Alchemy token balance API.
Expand All @@ -167,10 +162,9 @@ const alchemyTokenBalanceJTD = {
additionalProperties: false,
} as const

type AlchemyTokenBalanceResponse = JTDDataType<typeof alchemyTokenBalanceJTD>

const isValidAlchemyTokenBalanceResponse =
ajv.compile<AlchemyTokenBalanceResponse>(alchemyTokenBalanceJTD)
const isValidAlchemyTokenBalanceResponse = jtdValidatorFor(
alchemyTokenBalanceJTD
)

/**
* Use Alchemy's getTokenBalances call to get balances for a particular address.
Expand Down Expand Up @@ -204,7 +198,16 @@ export async function getTokenBalances(

// TODO log balances with errors, consider returning an error type
return json.tokenBalances
.filter((b) => b.error === null && b.tokenBalance !== null)
.filter(
(
b
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This indentation is horrendous, I hope the linter made you do it 😜

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One step further, the linter actually did it for me lol.

There's a world where we introduce a couple of convenience type guards that handle non-null, non-undefined, and non-null-or-undefined... But there weren't as many places where it was needed as I expected, so I stayed away from factoring it out for now.

): b is typeof json["tokenBalances"][0] & {
tokenBalance: Exclude<
typeof json["tokenBalances"][0]["tokenBalance"],
null
>
} => b.error === null && b.tokenBalance !== null
)
.map((tokenBalance) => ({
contractAddress: tokenBalance.contractAddress,
amount:
Expand All @@ -228,10 +231,9 @@ const alchemyTokenMetadataJTD = {
additionalProperties: false,
} as const

type AlchemyTokenMetadataResponse = JTDDataType<typeof alchemyTokenMetadataJTD>

const isValidAlchemyTokenMetadataResponse =
ajv.compile<AlchemyTokenMetadataResponse>(alchemyTokenMetadataJTD)
const isValidAlchemyTokenMetadataResponse = jtdValidatorFor(
alchemyTokenMetadataJTD
)

/**
* Use Alchemy's getTokenMetadata call to get metadata for a token contract on
Expand Down Expand Up @@ -262,8 +264,8 @@ export async function getTokenMetadata(
name: json.name,
symbol: json.symbol,
metadata: {
logoURL: json.logo,
tokenLists: [],
...(json.logo ? { logoURL: json.logo } : {}),
},
homeNetwork: ETHEREUM, // TODO make multi-network friendly
contractAddress,
Expand Down
10 changes: 6 additions & 4 deletions background/lib/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function genericLogger(level: LogLevel, input: unknown[]) {

console.log(...input)

const stackTrace = new Error().stack.split("\n").filter((line) => {
const stackTrace = new Error().stack?.split("\n")?.filter((line) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh this is much simpler than your previous attempt, I like it 👍

// Remove empty lines from the output
// Chrome prepends the word "Error" to the first line of the trace, but Firefox doesn't
// Let's ignore that for consistency between browsers!
Expand All @@ -69,9 +69,11 @@ function genericLogger(level: LogLevel, input: unknown[]) {
return true
})

// The first two lines of the stack trace will always be generated by this
// file, so let's ignore them.
console.log(stackTrace.slice(2).join("\n"))
if (typeof stackTrace !== "undefined") {
// The first two lines of the stack trace will always be generated by this
// file, so let's ignore them.
console.log(stackTrace.slice(2).join("\n"))
}
console.groupEnd()
}

Expand Down
54 changes: 24 additions & 30 deletions background/lib/prices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const COINGECKO_API_ROOT = "https://api.coingecko.com/api/v3"
export async function getPrice(
coingeckoCoinId = "ethereum",
currencySymbol = "usd"
): Promise<number> {
): Promise<number | null> {
const url = `${COINGECKO_API_ROOT}/simple/price?ids=${coingeckoCoinId}&vs_currencies=${currencySymbol}&include_last_updated_at=true`

const json = await fetchJson(url)
Expand All @@ -29,11 +29,7 @@ export async function getPrice(
return null
}

return json
? parseFloat(
json[coingeckoCoinId][currencySymbol] as string // FIXME Drop as when strict mode arrives and price schema type can include this.
)
: null
return json?.[coingeckoCoinId]?.[currencySymbol] || null
}

function multiplyByFloat(n: bigint, f: number, precision: number) {
Expand Down Expand Up @@ -67,34 +63,32 @@ export async function getPrices(
validate.errors
)

return null
return []
}

return assets.reduce((acc, asset) => {
const resolutionTime = Date.now()
return assets.flatMap((asset) => {
const simpleCoinPrices = json[asset.metadata.coinGeckoId]
return acc.concat(
vsCurrencies
.map((c) => {
const symbol = c.symbol.toLowerCase()
if (symbol in simpleCoinPrices) {
return {
pair: [c, asset],
amounts: [
multiplyByFloat(
BigInt(10) ** BigInt(c.decimals),
simpleCoinPrices[symbol] as number, // FIXME Drop as when strict mode arrives and price schema type can include this.
8
),
BigInt(1),
],
time: Date.now(),
} as PricePoint

return vsCurrencies
.map<PricePoint | undefined>((c) => {
const symbol = c.symbol.toLowerCase()
const coinPrice = simpleCoinPrices?.[symbol]

if (coinPrice) {
return {
pair: [c, asset],
amounts: [
multiplyByFloat(BigInt(10) ** BigInt(c.decimals), coinPrice, 8),
BigInt(1),
],
time: resolutionTime,
}
return undefined
})
.filter((p) => p)
)
}, [])
}
return undefined
})
.filter((p): p is PricePoint => p !== undefined)
})
}

/*
Expand Down
9 changes: 5 additions & 4 deletions background/lib/tokenList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,10 @@ export function networkAssetsFromLists(
metadata: {
...original.metadata,
...asset.metadata,
tokenLists: original.metadata.tokenLists.concat(
asset.metadata.tokenLists
),
tokenLists:
original.metadata?.tokenLists?.concat(
asset.metadata?.tokenLists ?? []
) ?? [],
},
}
} else {
Expand All @@ -99,7 +100,7 @@ export function networkAssetsFromLists(

const merged = fungibleAssets.reduce(tokenReducer, {})
return Object.entries(merged)
.map(([k, v]) => v)
.map(([, v]) => v)
.slice()
.sort((a, b) =>
(a.metadata?.tokenLists?.length || 0) >
Expand Down
Loading