Page MenuHomePhabricator

[Cashtab-v2] Address linting errors throughout the app
ClosedPublic

Authored by kieran709 on Mar 25 2022, 19:25.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Commits
rABC8692f6575082: [Cashtab-v2] Address linting errors throughout the app
Summary

Addressed linting errors throughout the app.

App.js:
Import proptypes
Add App.defaultProps func
Add prop types check

ProtectableComponentWrapper:
Import proptypes
Add ProtectableComponentWrapper default props func
Add prop types check

QRCode:
Import proptypes
delete '...otherProps'
Add QRCode default props func
Add prop types check

ScanQRCode:
Delete eslint-disable-next-line

Configure.js:
Delete eslint-disable react-hooks/exhaustive-deps

Tx:
Remove unused CSS import
Remove unused styled components:
SentLabel
GenesisLabel
CashtabMessageLabel
MessageLabel
ReplyMessageLabel

TxHistory:
Removed unused styled-components import

Send:
Remove unused button import
Remove unused handleOneToManyXECSend func & related dev comments

CreateTokenForm:
Remove unused TokenCollapse import
Remove unused notification & collapse imports
console.log file param in transformToTokenIcon func to avoid unused error

useWallet:
Remove eslint-disable react-hooks / exhaustive-deps

cashMethods:
Remove unused isValidUtxo import

context.js:
Add proptypes import
Add WalletProvider & AuthenticationProvider default props func
Add proptypes check for WalletProvider & AuthenticationProvider

GoogleAnalytics:
console.log options param to avoid unused error

Test Plan

cd web/cashtab-v2
npm start
observe linting errors and warnings

Diff Detail

Repository
rABC Bitcoin ABC
Branch
v2-linting-errors
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 18639
Build 37069: Build Diff
Build 37068: arc lint + arc unit

Event Timeline

bytesofman added inline comments.
web/cashtab-v2/src/components/App.js
3

Make sure to take care of this on extension/src/components/App.js as well

web/cashtab-v2/src/components/Authentication/ProtectableComponentWrapper.js
32

This component does not use passLoadingStatus, so it does not need this placeholder function

Also -- components that do need this, also need to have the comment explaining what it is and why.

Remove

39

PropTypes.objectOf(PropTypes.node)

web/cashtab-v2/src/components/Common/QRCode.js
246

This component does not use passLoadingStatus, so it does not need this placeholder function

Also -- components that do need this, also need to have the comment explaining what it is and why.

Remove

web/cashtab-v2/src/components/Tokens/CreateTokenForm.js
216

Add a comment explaining this. Something like, Dragger requires this function to work properly, with file as an input; linter requires file to be used in the function.

web/cashtab-v2/src/utils/GoogleAnalytics.js
55

Looks like this param can be removed. Please create a task to update the entire google analytics implementation, there is probably a better way of doing this now.

web/cashtab-v2/src/utils/context.js
30

This component does not use passLoadingStatus, so it does not need this placeholder function

Also -- components that do need this, also need to have the comment explaining what it is and why.

Remove

37

PropTypes.objectOf(PropTypes.node)

40

This component does not use passLoadingStatus, so it does not need this placeholder function

Also -- components that do need this, also need to have the comment explaining what it is and why.

Remove

47

PropTypes.objectOf(PropTypes.node)

Responding to review feedback and fixing remaining linting errors.

bytesofman added inline comments.
web/cashtab-v2/extension/src/components/App.js
359 ↗(On Diff #32954)

The App component does not have a prop called passLoadingStatus, only the sub components do.

Delete this defaultProps

web/cashtab-v2/src/components/App.js
386 ↗(On Diff #32954)

The App component does not have a prop called passLoadingStatus, only the sub components do.

Delete this defaultProps

This revision now requires changes to proceed.Mar 28 2022, 14:29
This revision is now accepted and ready to land.Mar 28 2022, 15:32