Tech debt 1265: Enable strictNullChecks TS compiler option by mmv08 · Pull Request #1301 · 5afe/safe-react

mmv08 · 2020-08-28T14:31:27Z

This PR:

closes Enable strictNullChecks option for typescript configuration #1265
enables strictNullChecks ts compiler option
fixes all corresponding errors (not in a best way sometimes), but this already showed that we were typing things wrong and there were many inconsistencies

…strictNullChecks

github-actions · 2020-08-28T14:31:42Z

CLA Assistant Lite All Contributors have signed the CLA.

github-actions · 2020-08-28T14:37:05Z

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	0	0
Warnings	77	0
Ignored	6	N/A

Result: ✅ success
Annotations: 77 total

[warning] @typescript-eslint/explicit-module-boundary-types

Require explicit return and argument types on exported functions' and classes' public class methods

src/components/App/ReceiveModal.tsx Line 82 - Missing return type on function.
src/components/Table/TableHead.tsx Line 22 - Argument 'property' should be typed.
src/components/Table/TableHead.tsx Line 22 - Argument 'orderAttr' should be typed.
src/components/Table/TableHead.tsx Line 22 - Missing return type on function.
src/components/Table/TableHead.tsx Line 28 - Missing return type on function.
src/logic/addressBook/utils/index.ts Line 14 - Missing return type on function.
src/logic/addressBook/utils/index.ts Line 14 - Argument 'addressBook' should be typed.
src/logic/addressBook/utils/index.ts Line 22 - Missing return type on function.
src/logic/addressBook/utils/index.ts Line 24 - Missing return type on function.
src/logic/addressBook/utils/index.ts Line 24 - Argument 'addressBook' should be typed.
src/logic/addressBook/utils/index.ts Line 24 - Argument 'userAddress' should be typed.
src/logic/currencyValues/store/middleware/index.ts Line 6 - Argument 'store' should be typed.
src/logic/currencyValues/store/middleware/index.ts Line 6 - Argument 'next' should be typed.
src/logic/currencyValues/store/middleware/index.ts Line 6 - Missing return type on function.
src/logic/currencyValues/store/middleware/index.ts Line 6 - Argument 'action' should be typed.
src/logic/currentSession/store/actions/loadCurrentSessionFromStorage.ts Line 5 - Missing return type on function.
src/logic/currentSession/store/actions/loadCurrentSessionFromStorage.ts Line 5 - Argument 'dispatch' should be typed.
src/logic/currentSession/utils/index.ts Line 9 - Missing return type on function.
src/logic/currentSession/utils/index.ts Line 9 - Argument 'currentSession' should be typed.
src/logic/safe/store/actions/processTransaction.ts Line 27 - Object pattern argument should be typed.
src/logic/safe/store/actions/processTransaction.ts Line 27 - Missing return type on function.
src/logic/safe/store/actions/processTransaction.ts Line 28 - Argument 'dispatch' should be typed.
src/logic/safe/store/actions/processTransaction.ts Line 29 - Argument 'getState' should be typed.
src/logic/safe/store/middleware/notificationsMiddleware.ts Line 70 - Argument 'store' should be typed.
src/logic/safe/store/middleware/notificationsMiddleware.ts Line 70 - Argument 'next' should be typed.
src/logic/safe/store/middleware/notificationsMiddleware.ts Line 70 - Missing return type on function.
src/logic/safe/store/middleware/notificationsMiddleware.ts Line 70 - Argument 'action' should be typed.
src/logic/safe/transactions/awaitingTransactions.ts Line 8 - Argument 'cancellationTxs' should be typed.
src/logic/safe/transactions/offchainSigner/ethSigner.ts Line 7 - Object pattern argument should be typed.
src/logic/safe/utils/safeStorage.ts Line 25 - Missing return type on function.
src/logic/safe/utils/safeStorage.ts Line 25 - Argument 'safes' should be typed.
src/logic/tokens/store/actions/fetchTokens.ts Line 46 - Missing return type on function.
src/logic/tokens/store/actions/fetchTokens.ts Line 46 - Argument 'contractAddress' should be typed.
src/logic/tokens/store/actions/fetchTokens.ts Line 46 - Argument 'methodHash' should be typed.
src/logic/tokens/store/actions/fetchTokens.ts Line 90 - Missing return type on function.
src/logic/tokens/store/actions/fetchTokens.ts Line 90 - Argument 'dispatch' should be typed.
src/logic/tokens/store/actions/fetchTokens.ts Line 90 - Argument 'getState' should be typed.
src/logic/wallets/store/middlewares/providerWatcher.ts Line 20 - Argument 'store' should be typed.
src/logic/wallets/store/middlewares/providerWatcher.ts Line 20 - Argument 'next' should be typed.
src/logic/wallets/store/middlewares/providerWatcher.ts Line 20 - Missing return type on function.
src/logic/wallets/store/middlewares/providerWatcher.ts Line 20 - Argument 'action' should be typed.
src/routes/load/components/DetailsForm/index.tsx Line 45 - Argument 'values' should be typed.
src/routes/load/components/OwnerList/index.tsx Line 159 - Object pattern argument should be typed.
src/routes/load/components/OwnerList/index.tsx Line 159 - Argument 'network' should be typed.
src/routes/load/components/OwnerList/index.tsx Line 160 - Argument 'controls' should be typed.
src/routes/load/components/OwnerList/index.tsx Line 160 - Object pattern argument should be typed.
src/routes/open/components/SafeOwnersConfirmationsForm/index.tsx Line 39 - Missing return type on function.
src/routes/open/components/SafeOwnersConfirmationsForm/index.tsx Line 39 - Argument 'index' should be typed.
src/routes/open/components/SafeOwnersConfirmationsForm/index.tsx Line 39 - Argument 'notRemovedOwners' should be typed.
src/routes/open/components/SafeOwnersConfirmationsForm/index.tsx Line 39 - Argument 'values' should be typed.
src/routes/open/components/SafeOwnersConfirmationsForm/index.tsx Line 222 - Object pattern argument should be typed.
src/routes/open/components/SafeOwnersConfirmationsForm/index.tsx Line 223 - Missing return type on function.
src/routes/open/components/SafeOwnersConfirmationsForm/index.tsx Line 223 - Argument 'controls' should be typed.
src/routes/open/components/SafeOwnersConfirmationsForm/index.tsx Line 223 - Object pattern argument should be typed.
src/routes/open/container/Open.tsx Line 48 - Missing return type on function.
src/routes/open/container/Open.tsx Line 48 - Argument 'safeAddress' should be typed.
src/routes/open/container/Open.tsx Line 48 - Argument 'safeName' should be typed.
src/routes/open/container/Open.tsx Line 48 - Argument 'ownersNames' should be typed.
src/routes/open/container/Open.tsx Line 48 - Argument 'ownerAddresses' should be typed.
src/routes/open/container/Open.tsx Line 56 - Missing return type on function.
src/routes/open/container/Open.tsx Line 56 - Argument 'values' should be typed.
src/routes/open/container/Open.tsx Line 56 - Argument 'userAccount' should be typed.
src/routes/opening/index.tsx Line 105 - Missing return type on function.
src/routes/opening/index.tsx Line 105 - Object pattern argument should be typed with a non-any type.
src/routes/safe/components/Balances/SendModal/SafeInfo/index.tsx Line 7 - Missing return type on function.
src/routes/safe/components/Balances/SendModal/screens/ChooseTxType/index.tsx Line 63 - Missing return type on function.
src/routes/safe/components/Balances/SendModal/screens/ChooseTxType/index.tsx Line 63 - Object pattern argument should be typed.
src/routes/safe/components/Balances/SendModal/screens/SendCollectible/index.tsx Line 46 - Missing return type on function.
src/routes/safe/components/Balances/SendModal/screens/SendCollectible/index.tsx Line 46 - Object pattern argument should be typed.
src/routes/safe/components/Balances/SendModal/screens/SendFunds/index.tsx Line 51 - Object pattern argument should be typed.
src/routes/safe/components/Settings/ManageOwners/RemoveOwnerModal/screens/ThresholdForm/index.tsx Line 26 - Missing return type on function.
src/routes/safe/components/Settings/ManageOwners/RemoveOwnerModal/screens/ThresholdForm/index.tsx Line 26 - Object pattern argument should be typed.
src/routes/safe/components/Settings/RemoveSafeModal/index.tsx Line 32 - Missing return type on function.
src/routes/safe/components/Settings/RemoveSafeModal/index.tsx Line 32 - Object pattern argument should be typed.
src/utils/intercom.ts Line 4 - Missing return type on function.

[warning] @typescript-eslint/no-non-null-assertion

Disallows non-null assertions using the ! postfix operator

src/routes/safe/components/Transactions/TxsTable/TxType/index.tsx Line 74 - Forbidden non-null assertion.
src/routes/safe/components/Transactions/TxsTable/TxType/index.tsx Line 74 - Forbidden non-null assertion.

^{Report generated by eslint-plus-action}

github-actions · 2020-08-28T14:37:05Z

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	3	0
Warnings	15	0
Ignored	1	N/A

Result: ❌ failure
Annotations: 18 total

[failure] @typescript-eslint/no-unused-vars

Disallow unused variables

src/components/App/ReceiveModal.tsx Line 6 - 'useSelector' is defined but never used.
src/components/App/ReceiveModal.tsx Line 17 - 'safeNameSelector' is defined but never used.
src/components/App/ReceiveModal.tsx Line 17 - 'safeParamAddressFromStateSelector' is defined but never used.

[warning] @typescript-eslint/explicit-module-boundary-types

Require explicit return and argument types on exported functions' and classes' public class methods

src/components/App/ReceiveModal.tsx Line 84 - Missing return type on function.
src/logic/addressBook/utils/index.ts Line 14 - Missing return type on function.
src/logic/addressBook/utils/index.ts Line 14 - Argument 'addressBook' should be typed.
src/logic/addressBook/utils/index.ts Line 22 - Missing return type on function.
src/logic/addressBook/utils/index.ts Line 25 - Missing return type on function.
src/logic/addressBook/utils/index.ts Line 25 - Argument 'addressBook' should be typed.
src/logic/addressBook/utils/index.ts Line 25 - Argument 'userAddress' should be typed.
src/logic/currencyValues/store/middleware/index.ts Line 6 - Argument 'store' should be typed.
src/logic/currencyValues/store/middleware/index.ts Line 6 - Argument 'next' should be typed.
src/logic/currencyValues/store/middleware/index.ts Line 6 - Missing return type on function.
src/logic/currencyValues/store/middleware/index.ts Line 6 - Argument 'action' should be typed.
src/logic/currentSession/store/actions/loadCurrentSessionFromStorage.ts Line 5 - Missing return type on function.
src/logic/currentSession/store/actions/loadCurrentSessionFromStorage.ts Line 5 - Argument 'dispatch' should be typed.
src/logic/currentSession/utils/index.ts Line 9 - Missing return type on function.
src/logic/currentSession/utils/index.ts Line 9 - Argument 'currentSession' should be typed.

^{Report generated by eslint-plus-action}

…strictNullChecks

ghost · 2020-09-03T10:46:50Z

Travis automatic deployment:
https://pr1301--safereact.review.gnosisdev.com/app

ghost · 2020-09-03T10:58:24Z

Travis automatic deployment:
https://pr1301--safereact.review.gnosisdev.com/app

src/routes/safe/components/Balances/SendModal/screens/SendFunds/index.tsx

dasanra · 2020-09-03T13:08:38Z

src/routes/safe/components/Settings/Advanced/index.tsx

  const nonce = useSelector(safeNonceSelector)
  const modules = useSelector(safeModulesSelector)
-  const moduleData = getModuleData(modules) ?? null
+  const moduleData = modules ? getModuleData(modules) ?? null : null


So getModuleData can return undefined?

Do we want to control this inside the function?

what do you mean? the return type is correct

I was just wondering if we should make the function to directly return null when result is undefined. Is not a big deal because it's only used here.
I feel that the logic to call the function gets a bit more complex to read just to ensure the result is null

As I said I didn’t dive deep into the logic, my goal was to enable the option and make project compile without changing the logic/returned values

As this method is only used here yet I would add the logic inside. Afterwards could be more complicated. But I leave the decision in your hands.

src/routes/safe/components/AddressBook/index.tsx

ghost · 2020-09-03T14:00:24Z

Travis automatic deployment:
https://pr1301--safereact.review.gnosisdev.com/app

src/test/utils/tokenMovements.ts

src/logic/safe/store/actions/transactions/utils/transactionHelpers.ts

src/routes/safe/components/Transactions/TxsTable/ExpandedTx/OwnersColumn/index.tsx

src/routes/safe/components/Transactions/TxsTable/ExpandedTx/TxDescription/utils.ts

dasanra · 2020-09-04T09:20:43Z

src/routes/safe/components/Settings/Advanced/index.tsx

  const nonce = useSelector(safeNonceSelector)
  const modules = useSelector(safeModulesSelector)
-  const moduleData = getModuleData(modules) ?? null
+  const moduleData = modules ? getModuleData(modules) ?? null : null


I was just wondering if we should make the function to directly return null when result is undefined. Is not a big deal because it's only used here.
I feel that the logic to call the function gets a bit more complex to read just to ensure the result is null

src/logic/contractInteraction/sources/ABIService/index.ts

ghost · 2020-09-04T10:11:16Z

Travis automatic deployment:
https://pr1301--safereact.review.gnosisdev.com/app

ghost · 2020-09-04T11:48:17Z

Travis automatic deployment:
https://pr1301--safereact.review.gnosisdev.com/app

ghost · 2020-09-04T11:56:13Z

Travis automatic deployment:
https://pr1301--safereact.review.gnosisdev.com/app

dasanra · 2020-09-04T12:00:47Z

Big work adding types here!

Agupane · 2020-09-07T15:38:59Z

src/logic/currencyValues/store/middleware/index.ts

        dispatch(fetchCurrencyRate(safeAddress, selectedCurrency))
        break
      }
-      case SET_CURRENCY_RATE:


Hi @mikheevm, want to ask you why did you removed this. Because while I was working on this ticket I found that this was part of the problem. Also, now the storage of the selected currency is broken, for example, if you choose EUR as your preferred currency and reload the page, you will see USD again

Hey, as I communicated before we don't need to store the balances locally as the rates frequently change and it's fine to fetch this again on app startup. I didn't expect that SET_CURRENCY_RATE action is capable of storing currently selected currency, I thought SET_CURRENCY_CURRENCY action was responsible for that because that's literally the action name. Sorry for this.

No problem! thanks for the clarification :)

mmv08 added 16 commits August 24, 2020 17:23

dep bump, enable strictNullChecks ts compiler option

c90d892

working on errors wip

604e8dc

remove unused imports

d3f432d

nullchecks errors wip

ca9f5e3

Merge branch 'development' of github.com:gnosis/safe-react into 1265-…

99f5e7e

…strictNullChecks

fixing errors

5e424fe

fixing errors

a38677f

fixing errors

efc9a93

fix clipboard func type errors

129fc99

error fixes

c176c06

error fixes

e930107

error fixes

ab0bf1e

error fixes

3938b07

error fixes

ee831e0

error fixes

a838390

Merge branch 'development' of github.com:gnosis/safe-react into 1265-…

a709e31

…strictNullChecks

mmv08 added 11 commits August 31, 2020 16:39

error fixing

db9d8ea

error fixing

a787264

Merge branch 'development' of github.com:gnosis/safe-react into 1265-…

5d572a6

…strictNullChecks

error fixing

70f087c

error fixing

0257727

pull from dev

ff3c663

error fixing

5f72ec7

error fixing

ef5f65e

error fixing

290b6e1

error fixing

2eb7fe1

error fixing

9c789ac

mmv08 added 6 commits September 3, 2020 12:31

error fixing

2a9638c

error fixing

9f96bd0

error fixing

7f6d0a2

error fixing

c8c9029

error fixing

e3baca1

Finally made it

5c65d0a

mmv08 marked this pull request as ready for review September 3, 2020 10:36

mmv08 requested a review from dasanra September 3, 2020 10:37

eslint fixes

3b25fd8

eslint fixes 2

c75cac0

dasanra suggested changes Sep 3, 2020

View reviewed changes

fix send funds validation

3f4d6d2

Update style load in ThresholdSettings

876444a

dasanra suggested changes Sep 4, 2020

View reviewed changes

Fix isValidAddress default state for SendCollectible

e6f8120

mmv08 added 2 commits September 4, 2020 15:35

fix OwnersColumn return type

d728ef1

fix extractUsefulMethods typing

9b8d96f

dasanra approved these changes Sep 4, 2020

View reviewed changes

mmv08 merged commit bfed967 into development Sep 4, 2020

mmv08 deleted the 1265-strictNullChecks branch September 4, 2020 12:03

github-actions bot locked and limited conversation to collaborators Sep 4, 2020

Agupane reviewed Sep 7, 2020

View reviewed changes

dasanra restored the 1265-strictNullChecks branch September 9, 2020 14:56

Conversation

mmv08 commented Aug 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 28, 2020

Uh oh!

github-actions bot commented Aug 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ESLint Summary View Full Report

[warning] @typescript-eslint/explicit-module-boundary-types

[warning] @typescript-eslint/no-non-null-assertion

Uh oh!

github-actions bot commented Aug 28, 2020

ESLint Summary View Full Report

[failure] @typescript-eslint/no-unused-vars

[warning] @typescript-eslint/explicit-module-boundary-types

Uh oh!

ghost commented Sep 3, 2020

Uh oh!

ghost commented Sep 3, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmv08 Sep 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ghost commented Sep 3, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ghost commented Sep 4, 2020

Uh oh!

ghost commented Sep 4, 2020

Uh oh!

ghost commented Sep 4, 2020

Uh oh!

dasanra commented Sep 4, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mmv08 commented Aug 28, 2020 •

edited

Loading

github-actions bot commented Aug 28, 2020 •

edited

Loading

mmv08 Sep 4, 2020 •

edited

Loading