Skip to content

Commit

Permalink
Moves extension linting to the @thesis-co/eslint-config setup
Browse files Browse the repository at this point in the history
This is a more restrictive and opinionated set of rules based on the
AirBnB style guide that reduces the degrees of freedom and bikeshedding
across the repo, as well as generally representing Thesis best
practices.

A few notes:
- This definitively moves us from a prior expressed preference for
  conditional React rendering of condition ? <Element /> : <></> to the
  more common null-punning approach of condition && <Element />. The
  style used in the past is disallowed by react/no-jsx-useless-fragment,
  which is part of the default airbnb style and therefore inherited
  through the Thesis config.
- That rule also shifted a few places where we were returning <></> to
  return null instead, and updated return types accordingly. In one or
  two places where doing that would have a far-reaching impact, an
  eslint-ignore is used for now (with clarifying comment of course),
  which can be revisited in the future if we feel strongly about it.
- This also pushes us away from the pattern of having () => {}-style
  functions with a single return statement inside, favoring instead
  making these return implicitly. A lot of the changes are around that.
- There were also a few cases where undefineds were being left to dangle
  that got flagged and now get alternate empty states. Delightful!

Last but not least, one whole rule is disabled so that we can get the
core of this merged: react/no-unstable-nested-components has flagged a
bunch of places where another pattern adopted in the current codebase
(defining anonymous functions that return elements lazily) is being used
in a way that's creating a lot of unnecessary diffs. Probably not
impactful in the large, but regardless it would be good to fix this in
the future, for now the rule is turned off with a FIXME comment.
  • Loading branch information
Shadowfiend committed Jul 25, 2023
1 parent 4d97a2e commit fd45e9f
Show file tree
Hide file tree
Showing 164 changed files with 3,932 additions and 2,995 deletions.
84 changes: 9 additions & 75 deletions .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,56 +1,13 @@
// This is a non-ESM JS file, so this rule can't be followed.
/* eslint-disable @typescript-eslint/no-var-requires */
const {
rules: {
"@typescript-eslint/naming-convention":
airbnbTypeScriptNamingConventionRules,
},
} = require("eslint-config-airbnb-typescript/lib/shared")

const {
rules: { "no-param-reassign": airbnbNoParamReassignRules },
} = require("eslint-config-airbnb-base/rules/best-practices")
/* eslint-enable @typescript-eslint/no-var-requires */

module.exports = {
extends: [
"airbnb",
"airbnb-typescript",
"airbnb/hooks",
"plugin:import/typescript",
"plugin:@typescript-eslint/recommended",
"plugin:prettier/recommended",
],
plugins: ["no-only-tests"],
extends: ["@thesis-co"],
globals: {
document: "readonly",
window: "readonly",
},
rules: {
// Styled-jsx does not play nice with this rule, unfortunately.
"react/jsx-one-expression-per-line": [0],
// JSX for the extension is explicitly rejected by Dan Abramov with good
// reasoning @
// https://github.com/airbnb/javascript/pull/985#issuecomment-239145468 .
// The rule is also mostly irrelevant to this codebase due to TypeScript
// usage (where .tsx is required).
"react/jsx-filename-extension": [0],
"@typescript-eslint/naming-convention": [
...airbnbTypeScriptNamingConventionRules,
// Allow underscore-only identifiers to indicate ignored positional variables.
{
selector: "variable",
format: null,
filter: {
regex: "^_+$",
match: true,
},
custom: {
regex: "^_+$",
match: true,
},
},
],
// FIXME Disabled to get an initial lint pass in, but needs to be
// FIXME reenabled.
"react/no-unstable-nested-components": [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
Expand All @@ -60,6 +17,10 @@ module.exports = {
{ allowRequiredDefaults: true },
],
"react/require-default-props": [0],
// Styled-jsx does not play nice with this rule, unfortunately.
"react/jsx-one-expression-per-line": [0],
// stlyed-jsx also uses a couple of not-known-by-default properties.
"react/no-unknown-property": [2, { ignore: ["jsx", "global"] }],
// Shared components may have labels associated externally in a way ESLint
// does not detect.
"jsx-a11y/label-has-associated-control": [
Expand All @@ -76,40 +37,13 @@ module.exports = {
"error",
{ devDependencies: ["!+(src/api|ui)/**/*.+(ts|js)"] },
],
// dissalow it.only, assert.only, etc..
"no-only-tests/no-only-tests": ["error"],
// Add known-safe exceptions to no-param-reassign.
"no-param-reassign": [
airbnbNoParamReassignRules[0],
{
props: airbnbNoParamReassignRules[1].props,
ignorePropertyModificationsFor:
airbnbNoParamReassignRules[1].ignorePropertyModificationsFor,
ignorePropertyModificationsForRegex: [
...(airbnbNoParamReassignRules[1]
.ignorePropertyModificationsForRegex || []),
"^immer", // For redux-toolkit reducers using immer.
],
},
],
"@typescript-eslint/no-unused-vars": [
"error",
{
argsIgnorePattern: "^_",
varsIgnorePattern: "^_",
},
],
"no-unused-vars": "off",
},
ignorePatterns: [
"dist/",
"extension-reload.js",
"**/validate/*.js",
"**/local-chain/**",
"!.github",
"size-plugin.json",
],
parser: "@typescript-eslint/parser",
parserOptions: {
project: "./.tsconfig-eslint.json",
},
}
8 changes: 4 additions & 4 deletions .github/workflows/builds/post-build-link.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ module.exports = async function postBuildLink({ github, context }) {
)

if (Number.isNaN(workflowRunId)) {
throw new Error(`Failed to get workflow run id`)
throw new Error("Failed to get workflow run id")
}

const {
Expand Down Expand Up @@ -50,9 +50,9 @@ module.exports = async function postBuildLink({ github, context }) {
)
}

const matchArtifact = allArtifacts.filter((artifact) => {
return artifact.name.startsWith("extension-builds-")
})[0]
const matchArtifact = allArtifacts.filter((artifact) =>
artifact.name.startsWith("extension-builds-")
)[0]

if (matchArtifact === undefined || matchArtifact === null) {
throw new Error(
Expand Down
5 changes: 4 additions & 1 deletion .github/workflows/pledge-signer-sync/pledge-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ if (!GALXE_ACCESS_TOKEN || !FIRESTORE_USER || !FIRESTORE_PASSWORD) {
// Limit sync range to last 4 days ( 2 days from last sync + 2 days from now )
const TARGET_DATE = new Date(Date.now() - 4 * 24 * 60 * 60_000)

const wait = (ms) => new Promise((r) => setTimeout(r, ms))
const wait = (ms) =>
new Promise((r) => {
setTimeout(r, ms)
})

const getAddresses = async () => {
const app = initializeApp({
Expand Down
3 changes: 2 additions & 1 deletion __mocks__/@ethersproject/web.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// eslint-disable-next-line import/no-extraneous-dependencies
// Fixing this here requires digging into Jest a bit, kicking for now.
// eslint-disable-next-line import/no-import-module-exports
import sinon from "sinon"

const mock =
Expand Down
2 changes: 2 additions & 0 deletions __mocks__/webextension-polyfill.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// Fixing this here requires digging into Jest a bit, kicking for now.
// eslint-disable-next-line import/no-import-module-exports
import { Tabs } from "webextension-polyfill"

const browserMock = jest.createMockFromModule<
Expand Down
108 changes: 52 additions & 56 deletions background/lib/gas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,67 +84,63 @@ const getPolygonGasPrices = async (price: bigint): Promise<BlockPrices> => {
const getArbitrumPrices = async (
baseFeePerGas: bigint,
blockNumber: number
): Promise<BlockPrices> => {
return {
network: ARBITRUM_ONE,
blockNumber,
baseFeePerGas,
estimatedPrices: [
{
confidence: 99,
maxPriorityFeePerGas: 0n, // priority fee doesn't make sense for Arbitrum
maxFeePerGas: 0n, // max fee doesn't make sense for Arbitrum
price: baseFeePerGas * 3n,
},
{
confidence: 95,
maxPriorityFeePerGas: 0n,
maxFeePerGas: 0n,
price: baseFeePerGas * 2n,
},
{
confidence: 70,
maxPriorityFeePerGas: 0n,
maxFeePerGas: 0n,
price: baseFeePerGas,
},
],
dataSource: "local",
}
}
): Promise<BlockPrices> => ({
network: ARBITRUM_ONE,
blockNumber,
baseFeePerGas,
estimatedPrices: [
{
confidence: 99,
maxPriorityFeePerGas: 0n, // priority fee doesn't make sense for Arbitrum
maxFeePerGas: 0n, // max fee doesn't make sense for Arbitrum
price: baseFeePerGas * 3n,
},
{
confidence: 95,
maxPriorityFeePerGas: 0n,
maxFeePerGas: 0n,
price: baseFeePerGas * 2n,
},
{
confidence: 70,
maxPriorityFeePerGas: 0n,
maxFeePerGas: 0n,
price: baseFeePerGas,
},
],
dataSource: "local",
})

const getLegacyGasPrices = async (
network: EVMNetwork,
gasPrice: bigint,
blockNumber: number
): Promise<BlockPrices> => {
return {
network,
blockNumber,
baseFeePerGas: gasPrice,
estimatedPrices: [
{
confidence: 99,
maxPriorityFeePerGas: 0n, // doesn't exist
maxFeePerGas: 0n, // doesn't exist
price: gasPrice,
},
{
confidence: 95,
maxPriorityFeePerGas: 0n,
maxFeePerGas: 0n,
price: gasPrice,
},
{
confidence: 70,
maxPriorityFeePerGas: 0n,
maxFeePerGas: 0n,
price: gasPrice,
},
],
dataSource: "local",
}
}
): Promise<BlockPrices> => ({
network,
blockNumber,
baseFeePerGas: gasPrice,
estimatedPrices: [
{
confidence: 99,
maxPriorityFeePerGas: 0n, // doesn't exist
maxFeePerGas: 0n, // doesn't exist
price: gasPrice,
},
{
confidence: 95,
maxPriorityFeePerGas: 0n,
maxFeePerGas: 0n,
price: gasPrice,
},
{
confidence: 70,
maxPriorityFeePerGas: 0n,
maxFeePerGas: 0n,
price: gasPrice,
},
],
dataSource: "local",
})

export default async function getBlockPrices(
network: EVMNetwork,
Expand Down
8 changes: 3 additions & 5 deletions background/lib/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -309,11 +309,9 @@ class Logger {

const entriesByDateAsc = logEntries
// Sort by date.
.sort((a, b) => {
return a
.substr(1, iso8601Length)
.localeCompare(b.substr(1, iso8601Length))
})
.sort((a, b) =>
a.substr(1, iso8601Length).localeCompare(b.substr(1, iso8601Length))
)

const lastEntry = entriesByDateAsc[entriesByDateAsc.length - 1]
const lastEntryDate = dateFromLogEntry(lastEntry)
Expand Down
5 changes: 2 additions & 3 deletions background/lib/posthog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@ export enum OneTimeAnalyticsEvent {

export const isOneTimeAnalyticsEvent = (
eventName: string
): eventName is OneTimeAnalyticsEvent => {
return Object.values<string>(OneTimeAnalyticsEvent).includes(eventName)
}
): eventName is OneTimeAnalyticsEvent =>
Object.values<string>(OneTimeAnalyticsEvent).includes(eventName)

const POSTHOG_PROJECT_ID = "11112"

Expand Down
30 changes: 14 additions & 16 deletions background/lib/token-lists.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,19 +90,17 @@ function tokenListToFungibleAssetsForNetwork(
// Filter out assets with the same symbol as the network base asset
symbol !== network.baseAsset.symbol
)
.map((tokenMetadata) => {
return {
metadata: {
...(tokenMetadata.logoURI ? { logoURL: tokenMetadata.logoURI } : {}),
tokenLists: [tokenListCitation],
},
name: tokenMetadata.name,
symbol: tokenMetadata.symbol,
decimals: tokenMetadata.decimals,
homeNetwork: network,
contractAddress: normalizeEVMAddress(tokenMetadata.address),
}
})
.map((tokenMetadata) => ({
metadata: {
...(tokenMetadata.logoURI ? { logoURL: tokenMetadata.logoURI } : {}),
tokenLists: [tokenListCitation],
},
name: tokenMetadata.name,
symbol: tokenMetadata.symbol,
decimals: tokenMetadata.decimals,
homeNetwork: network,
contractAddress: normalizeEVMAddress(tokenMetadata.address),
}))
}

/**
Expand Down Expand Up @@ -176,9 +174,9 @@ export function mergeAssets<T extends FungibleAsset>(
// As for cache key generation we are using the total number of assets that were provided.
// This is not 100% accurate, but given that we are dealing with token lists it seems to be
// a safe bet. The chances are slim that 1 asset is added and 1 is removed in 1 minute.
export const memoizedMergeAssets = memoize(mergeAssets, (...assetLists) => {
return assetLists.reduce((acc, curr) => acc + curr.length, 0)
})
export const memoizedMergeAssets = memoize(mergeAssets, (...assetLists) =>
assetLists.reduce((acc, curr) => acc + curr.length, 0)
)

/*
* Return all tokens in the provided lists, de-duplicated and structured in our
Expand Down
13 changes: 6 additions & 7 deletions background/lib/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,19 +179,16 @@ export function truncateAddress(address: string): string {
export const getNumericStringValueFromBigNumber = (
value: BigNumber,
tokenDecimals: number
): string => {
return Number(value.toBigInt() / 10n ** BigInt(tokenDecimals)).toString()
}
): string => Number(value.toBigInt() / 10n ** BigInt(tokenDecimals)).toString()

export const numberTo32BytesHex = (value: string, decimals: number): string => {
const withDecimals = BigInt(value) * 10n ** BigInt(decimals)
const hex = utils.hexlify(withDecimals)
return hex
}

export const isMaxUint256 = (amount: BigNumber | bigint | string): boolean => {
return ethers.BigNumber.from(amount).eq(ethers.constants.MaxUint256)
}
export const isMaxUint256 = (amount: BigNumber | bigint | string): boolean =>
ethers.BigNumber.from(amount).eq(ethers.constants.MaxUint256)

/**
* Converts a string of hexidecimals bytes to ascii text
Expand All @@ -205,7 +202,9 @@ export const hexToAscii = (hex_: string): string => {
}

export const wait = (ms: number): Promise<void> =>
new Promise<void>((r) => setTimeout(r, ms))
new Promise<void>((r) => {
setTimeout(r, ms)
})

export const getUNIXTimestamp = (time = Date.now()): UNIXTime =>
Math.floor(time / 1000)
Loading

0 comments on commit fd45e9f

Please sign in to comment.