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

Chore: Improve linting & auto-alphabetize translations #2294

Merged
merged 34 commits into from
Dec 23, 2021
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
e3cb0c9
cleanup: lint codebase, update eslint recommended
Dec 3, 2021
20bb560
test: unecessary space
Dec 3, 2021
895f3e3
add eslint prettier
Dec 3, 2021
84965e7
another test
Dec 3, 2021
8cec420
bad commit format
Dec 3, 2021
49ed15c
bad commit format
Dec 3, 2021
87a7ecd
testing pre-commit hook and
Dec 3, 2021
8af6581
commit message
Dec 3, 2021
22a136d
trying again
Dec 3, 2021
6c511a6
chore: fixed bug by clearing node_modules and reinstalling
Dec 3, 2021
affef56
chore: autosort translation on save
Dec 4, 2021
19f4d20
docs: more info when a bad commit format is used
Dec 4, 2021
9c3dd93
chore: lint pattern match cleanup
Dec 4, 2021
fb5941c
chore: undo file update
Dec 4, 2021
8440503
chore: revert file update
Dec 4, 2021
f4ec97e
docs: add linting and formatting docs
Dec 5, 2021
573be50
docs: add git hooks link
Dec 6, 2021
7bb0bbc
chore: remove prettier from project
Dec 6, 2021
36be00b
chore: run e2e on precommit
Dec 17, 2021
5bd6c7b
chore: Merge remote-tracking branch 'origin' into chore/auto-alphabet…
Dec 21, 2021
66b5fa1
chore: switch to prepush
Dec 21, 2021
548b4ba
chore: update commit msg regex and docs
Dec 22, 2021
00640e8
chore: update commit msg
Dec 22, 2021
bef66d1
chore: localize image reference
Dec 22, 2021
e3b2b2e
chore: Merge branch 'master' of https://github.com/worldclassdev/near…
Dec 22, 2021
3b7b326
chore: remove commit msg length check
Dec 22, 2021
b00de19
chore: remove lint-staged, run commands on prepush
Dec 22, 2021
358cd3b
chore: update prepush command
Dec 22, 2021
5507fe7
chore(docs): remove reference to lint-staged
Dec 22, 2021
8b365df
chore: revert prettier formatting
Dec 23, 2021
f946e19
chore: revert prettier formatting
Dec 23, 2021
2555b94
chore: turn off eslint boolean cast rule
Dec 23, 2021
bbbd21a
chore: revert prettier formatting
Dec 23, 2021
b158169
chore: more eslint config refinement
Dec 23, 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
10 changes: 10 additions & 0 deletions .husky/commit-msg
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#!/bin/sh
# This script checks if the commit message follows a consistent format(usually useful for consistency and devops)
if ! head -1 "$1" | grep -qE "^(feat|fix|ci|chore|docs|test|style|refactor|perf|build|revert)(\(.+?\))?: .{1,}$"; then
gutsyphilip marked this conversation as resolved.
Show resolved Hide resolved
echo "Aborting commit. Your commit message is invalid. See accepted format here: https://commitizen.github.io/cz-cli/" >&2
exit 1
fi
if ! head -1 "$1" | grep -qE "^.{1,88}$"; then
echo "Aborting commit. Your commit message is too long." >&2
exit 1
fi
5 changes: 5 additions & 0 deletions .husky/pre-commit
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/bin/sh
# This runs the precommit script on any modified packages
. "$(dirname "$0")/_/husky.sh"

npx lerna run --concurrency 1 --stream precommit --since HEAD --exclude-dependents
6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
"scripts": {
"lint": "cd packages/frontend && yarn && yarn lint",
"build": "cd packages/frontend && yarn && yarn build",
"test": "cd packages/frontend && yarn && yarn test"
"test": "cd packages/frontend && yarn && yarn test",
"prepare": "husky install"
},
"devDependencies": {
"lerna": "^4.0.0"
"lerna": "^4.0.0",
"husky": "^7.0.0"
}
}
3 changes: 3 additions & 0 deletions packages/frontend/.eslintignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
dist/
build
node_modules
.github

21 changes: 9 additions & 12 deletions packages/frontend/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,21 @@
module.exports = {
extends: 'react-app',
extends: ['react-app', 'eslint:recommended'],
rules: {
'jsx-a11y/no-access-key': 'off',
'no-useless-escape': 'off',
'semi': ['error', 'always'],
'no-extra-boolean-cast': 'off',
semi: ['error', 'always'],
'no-console': 'off',
'import/order': [
'error',
{
alphabetize: {
order: 'asc',
caseInsensitive: true
caseInsensitive: true,
},
'newlines-between': 'always',
groups: [
'builtin',
['external', 'internal'],
['sibling', 'parent', 'index'],
'object'
]
}
groups: ['builtin', ['external', 'internal'], ['sibling', 'parent', 'index'], 'object'],
},
],
}
}
},
};
60 changes: 60 additions & 0 deletions packages/frontend/docs/Linting-and-formatting.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# Linting and Formatting
gutsyphilip marked this conversation as resolved.
Show resolved Hide resolved
For consistency and predictable organization of code, this project uses [Eslint](https://eslint.org/) and [Prettier](https://prettier.io/) for linting and formatting.

## The Setup
The configuration for Eslint and prettier can be found at the root of this project:
- Eslint - `.eslintrc.js`
- Prettier - `.prettierrc`
gutsyphilip marked this conversation as resolved.
Show resolved Hide resolved

VSCode is configured to automatically fix fixable linting errors on save. See `.vscode/settings.json`.

With `package.json`, there scripts that you can run to lint, autoFix and format the entire codebase.

```
"lint": "eslint --ext .js --ext .jsx .",
"fix": "eslint --ext .js --ext .jsx . --fix",
"format": "prettier --write",
```

## Git Commit hooks
Git commit hooks are a provision that allows us run custom scripts when specific events occur in the git workflow. [See more here](https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks).

For this project, we are using the clientside commit hooks to automatically lint and alphabetize staged files before they get committed.

## Auto-alphabetization of translation files
This is meant to help ensure consistency and ease of reference within the translation files. For this, we are making use of git hooks via a package called [husky](https://typicode.github.io/husky/#/). Husky allows us to configure a defined pre-commit command which triggers the `precommit` script within modified packages. The `precommit` script in `packages/frontend/package.json` is configured to run [lint-staged](https://github.com/okonet/lint-staged). Lint-staged allows us run linters on files that have been staged through git.

Within `package.json`, the block below defines what linters and formatters get applied to what filetype.

```
"lint-staged": {
"*.{js,ts,tsx, jsx}": [
"eslint --quiet --fix",
"prettier --write"
],
"src/translations/*.json": "sort-json --ignore-case true"
}
```

Two main things are ensured through this configuration:
- Staged translation files `src/translations/*.json"` get sorted in alphabetical order automatically using a package called [sort-json](https://github.com/kesla/sort-json)
- Staged `.js` and `.jsx` files get linted for any major linting errors and committing will fail until linting errors are resolved e.g unused vars. We can make the rules as tight or loose as we want through the configuration in `.eslintrc.js`.
- Staged `.js` and `.jsx` automatically get formatted using [prettier](https://prettier.io/). This too makes use of the configuration found in `.prettierrc`.

## Proposal: Commit messages
Within `.husky/commit-msg`, there's a script that ensures consistent formatting with commit messages using the `commit-msg` git hook.

The proposed format is:
```
git commit -m "{commit type goes here}: {commit information}"
```

Example:
```
git commit -m "feat: linting support using eslint"
```

The image below provides more information on the commit types.
![image](https://user-images.githubusercontent.com/16845555/144693890-b0a4d6a9-9463-489e-8d8b-1a8cb4489d67.png)
gutsyphilip marked this conversation as resolved.
Show resolved Hide resolved

Learn more [here](https://commitizen.github.io/cz-cli/). This is not a major thing, however. It just seemed like a somewhat beneficial low-hanging fruit.
gutsyphilip marked this conversation as resolved.
Show resolved Hide resolved
10 changes: 9 additions & 1 deletion packages/frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@
"lint": "eslint --ext .js --ext .jsx .",
"fix": "eslint --ext .js --ext .jsx . --fix",
"storybook": "start-storybook -p 6006",
"build-storybook": "build-storybook"
"build-storybook": "build-storybook",
"precommit": "lint-staged"
gutsyphilip marked this conversation as resolved.
Show resolved Hide resolved
},
"browserslist": [
">0.2%",
Expand Down Expand Up @@ -88,8 +89,15 @@
"eslint-plugin-jsx-a11y": "^6.4.1",
"eslint-plugin-react": "^7.23.2",
"jest": "^27.0.1",
"lint-staged": "^12.1.2",
"parcel-bundler": "^1.12.4",
"parcel-plugin-bundle-visualiser": "^1.2.0",
"react-is": "^17.0.2"
},
"lint-staged": {
"*.{js,jsx}": [
"eslint --quiet --fix"
],
"src/translations/*.json": "sort-json --ignore-case true"
gutsyphilip marked this conversation as resolved.
Show resolved Hide resolved
}
}
99 changes: 54 additions & 45 deletions packages/frontend/src/components/Recaptcha.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ const ENABLE_DEBUG_LOGGING = false;
const debugLog = (...args) => ENABLE_DEBUG_LOGGING && console.log('Recaptcha:', ...args);

const RecaptchaFailedBox = styled.div`
border: 1px dashed #FF8588;
border: 1px dashed #ff8588;
border-radius: 2px;
background-color: #FEF2F2;
background-color: #fef2f2;
display: flex;
flex-direction: column;
align-items: center;
Expand All @@ -28,28 +28,30 @@ const RecaptchaFailedBox = styled.div`
margin-bottom: 30px;

.title {
color: #A00005;
color: #a00005;
font-weight: 600;
}

.desc {
color: #DC1F25;
color: #dc1f25;
}

button {
font-weight: normal !important;
text-decoration: underline !important;
}

.title, .desc, button {
.title,
.desc,
button {
margin-top: 20px !important;
}
`;

const RecaptchaString = styled.div`
font-size: 12px;
font-weight: 300;
color: #72727A;
color: #72727a;
margin: 10px auto 40px auto;
text-align: center;
max-width: 304px;
Expand All @@ -60,25 +62,21 @@ const RecaptchaString = styled.div`
}
`;


export class Recaptcha extends Component {
recaptchaRef = null
recaptchaRef = null;
loadingTimeoutHandle = null;

state = {
loaded: false,
loadFailed: false
}
loadFailed: false,
};

resetLoadingTimeout() {
debugLog('Setting script load timeout', { RECAPTCHA_LOADING_TIMEOUT });
this.clearLoadingTimeout();
this.loadingTimeoutHandle = setTimeout(
() => {
this.setState({ loadFailed: true });
},
RECAPTCHA_LOADING_TIMEOUT
);
this.loadingTimeoutHandle = setTimeout(() => {
this.setState({ loadFailed: true });
}, RECAPTCHA_LOADING_TIMEOUT);
}

clearLoadingTimeout() {
Expand All @@ -97,25 +95,27 @@ export class Recaptcha extends Component {
}

setCaptchaRef(ref) {
if (ref) { this.recaptchaRef = ref; }
};
if (ref) {
this.recaptchaRef = ref;
}
}

/** Do not refactor this to an in-line function!
* Must be a stable function, or recaptcha will infinitely loop on reloading itself in the background
* If porting to Hooks, use useMemo() to get a stable reference to the function
* @param token recaptchaToken returned by recaptcha API
*/
handleOnChange = (token) => {
handleOnChange = token => {
debugLog('onchange()', token);

if (token) {
Mixpanel.track('solved reCaptcha');
}

this.props.onChange && this.props.onChange(token);
}
};

handleOnLoad = (scriptDetails) => {
handleOnLoad = scriptDetails => {
debugLog('handleOnLoad()', scriptDetails);

this.clearLoadingTimeout();
Expand All @@ -129,12 +129,13 @@ export class Recaptcha extends Component {
Mixpanel.track('loaded reCaptcha script');
this.setState({ loaded: true });
}

}
};

reset() {
debugLog('reset()');
if (this.recaptchaRef) { this.recaptchaRef.reset(); }
if (this.recaptchaRef) {
this.recaptchaRef.reset();
}
// Reset does not call onChange; manually notify subscribers that there is no longer a valid token on reset
this.handleOnChange(null);
}
Expand All @@ -145,17 +146,21 @@ export class Recaptcha extends Component {
if (loadFailed) {
return (
<RecaptchaFailedBox className='recaptcha-failed-box'>
<PuzzleIcon/>
<div className='title'><Translate id='reCAPTCHA.fail.title'/></div>
<div className='desc'><Translate id='reCAPTCHA.fail.desc'/></div>
<PuzzleIcon />
<div className='title'>
<Translate id='reCAPTCHA.fail.title' />
</div>
<div className='desc'>
<Translate id='reCAPTCHA.fail.desc' />
</div>
<FormButton
color='link'
onClick={() => {
Mixpanel.track('CA used reCaptcha escape hatch');
this.props.onFundAccountCreation();
}}
>
<Translate id='reCAPTCHA.fail.link'/>
<Translate id='reCAPTCHA.fail.link' />
</FormButton>
</RecaptchaFailedBox>
);
Expand All @@ -165,30 +170,34 @@ export class Recaptcha extends Component {

return (
<>
{!loaded &&
{!loaded && (
<span>
<Translate id='reCAPTCHA.loading'/>
<Translate id='reCAPTCHA.loading' />
</span>
}
{RECAPTCHA_CHALLENGE_API_KEY && <ReCAPTCHA
sitekey={RECAPTCHA_CHALLENGE_API_KEY}
ref={(ref) => this.setCaptchaRef(ref)}
onChange={this.handleOnChange}
asyncScriptOnLoad={this.handleOnLoad}
className='recaptcha-widget'
/>}
{loaded &&
)}
{RECAPTCHA_CHALLENGE_API_KEY && (
<ReCAPTCHA
sitekey={RECAPTCHA_CHALLENGE_API_KEY}
ref={ref => this.setCaptchaRef(ref)}
onChange={this.handleOnChange}
asyncScriptOnLoad={this.handleOnLoad}
className='recaptcha-widget'
/>
)}
{loaded && (
<RecaptchaString className='recaptcha-disclaimer'>
<Translate id='reCAPTCHA.disclaimer'/>
<Translate id='reCAPTCHA.disclaimer' />
</RecaptchaString>
}
)}
</>
);
}
}

export const isRetryableRecaptchaError = (e) => {
if (!e.code) { return false; }
export const isRetryableRecaptchaError = e => {
if (!e.code) {
return false;
}

return ['invalid-input-response','missing-input-response', 'timeout-or-duplicate'].includes(e.code);
};
return ['invalid-input-response', 'missing-input-response', 'timeout-or-duplicate'].includes(e.code);
};
4 changes: 2 additions & 2 deletions packages/frontend/src/components/accounts/SetupSeedPhrase.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ class SetupSeedPhrase extends Component {
}

render() {
const { recoveryMethods, recoveryMethodsLoader } = this.props;
const { recoveryMethods, recoveryMethodsLoader } = this.props;
const hasSeedPhraseRecovery = recoveryMethodsLoader || recoveryMethods.filter(m => m.kind === 'phrase').length > 0;
const { seedPhrase, enterWord, wordId, submitting, localAlert, isNewAccount, successSnackbar } = this.state;

Expand Down Expand Up @@ -294,7 +294,7 @@ const mapDispatchToProps = {
};

const mapStateToProps = (state, { match }) => {
const { accountId } = match.params;
const { accountId } = match.params;

return {
...selectAccountSlice(state),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import CreateAccountBtn from './CreateAccountBtn';
import Logo from './Logo';
import NavLinks from './NavLinks';
import UserAccount from './UserAccount';
import UserAccounts from './UserAccounts';

const Container = styled.div`
display: none;
Expand Down
Loading