chore(all): fix eslint and lint-staged setup #117
Conversation
przemyslaw-wlodek
changed the title
| @@ -173,7 +174,7 @@ export default new (class WebTester { | |||
| await sel.waitForDisplayed(); | |||
| return await sel | |||
| .getText() | |||
| .then((t) => Promise.resolve(t)) | |||
| .then((t) => t) | |||
There was a problem hiding this comment.
That then is redundant.
| @@ -117,14 +117,15 @@ export class TokensPage extends WebElement { | |||
|
|
|||
| async getTokenTableItemValueByIndex(index: number, mode: 'extended' | 'popup'): Promise<number> { | |||
| const tokenValue = (await webTester.getTextValueFromElement(this.tokensTableItemValue(index, mode))) as string; | |||
| return Number.parseFloat(tokenValue.replace(/,/g, '')); | |||
| // eslint-disable-next-line unicorn/prefer-string-replace-all | |||
| return Number.parseFloat(tokenValue.replace(',', '')); | |||
There was a problem hiding this comment.
I think this change might potentially break the code. Previous code replaces all occurrences of , while after your changes only the first one gets replaced. I thing it should be reverted.
There was a problem hiding this comment.
Brilliant catch! Thank you, it reverted just a part of the autofix. I should've noticed that! Will update this soon
| } | ||
|
|
||
| async getTokenTableItemValueByName(tokenName: string, mode: 'extended' | 'popup'): Promise<number> { | ||
| const tokenValue = (await webTester.getTextValueFromElement( | ||
| this.tokensTableItemValue(await this.getTokenRowIndex(tokenName), mode) | ||
| )) as string; | ||
| return Number.parseFloat(tokenValue.replace(/,/g, '')); | ||
| return Number.parseFloat(tokenValue.replace(',', '')); |
There was a problem hiding this comment.
I think this change might potentially break the code. Previous code replaces all occurrences of , while after your changes only the first one gets replaced. I thing it should be reverted.
| @@ -19,8 +19,13 @@ | |||
| "build": "rollup -c rollup.config.js", | |||
| "build-storybook": "export NODE_OPTIONS=--openssl-legacy-provider; build-storybook", | |||
| "cleanup": "shx rm -rf node_modules", | |||
| "format": "yarn prettier --write '**/*.tsx'", | |||
| "lint": "eslint .", | |||
| "eslint:check": "eslint --cache --cache-location ../../.cache/eslintcache --cache-strategy metadata --ignore-path '../../.eslintignore' .", | |||
There was a problem hiding this comment.
The cache and ignore path parameters assume this package lives in the monorepo. I remember @renanvalentin had a point that this package should stay independent of the lace mono repo so it might be easily moved to separate repo some day.
| "@typescript-eslint/eslint-plugin": "^5.51.0", | ||
| "@typescript-eslint/parser": "^5.51.0", | ||
| "@vanilla-extract/rollup-plugin": "^1.2.0", | ||
| "@vanilla-extract/webpack-plugin": "^2.2.0", | ||
| "babel-loader": "^8.3.0", | ||
| "concurrently": "^8.0.1", | ||
| "eslint": "^8.33.0", | ||
| "eslint-config-prettier": "^8.3.0", | ||
| "eslint-plugin-boundaries": "^3.1.0", | ||
| "eslint-plugin-functional": "^5.0.4", | ||
| "eslint-plugin-import": "^2.27.5", | ||
| "eslint-plugin-max-params-no-constructor": "^0.0.4", | ||
| "eslint-plugin-prefer-arrow-functions": "^3.1.4", | ||
| "eslint-plugin-react": "7.31.8", | ||
| "eslint-plugin-storybook": "^0.6.11", | ||
| "eslint-plugin-unicorn": "^45.0.2", |
There was a problem hiding this comment.
Following the same argumentation of this package independency from the lace monorepo I would keep all the required tooling packages here.
| @@ -1,4 +1,5 @@ | |||
| const convertVersionFormat = (version: string) => version.replace(/\./g, '_'); | |||
| // eslint-disable-next-line unicorn/prefer-string-replace-all | |||
| const convertVersionFormat = (version: string) => version.replace('.', '_'); | |||
There was a problem hiding this comment.
Now only the first occurrence of . will be replaced and I thing it is an incorrect behaviour. Consider reverting this change.
| @@ -45,7 +45,8 @@ class TokensPageAssert { | |||
| await t('browserView.assets.totalWalletBalance') | |||
| ); | |||
| await expect(await tokensPage.getTotalBalanceCurrency()).to.equal('USD'); | |||
| const actualTotalBalance = Number(((await tokensPage.getTotalBalanceValue()) as string).replace(/,/g, '')); | |||
| // eslint-disable-next-line unicorn/prefer-string-replace-all | |||
| const actualTotalBalance = Number(((await tokensPage.getTotalBalanceValue()) as string).replace(',', '')); | |||
There was a problem hiding this comment.
Now only the first occurrence of , will be replaced and I might be an incorrect behaviour. Please ensure this change won't introduce any problem
| @@ -165,4 +165,4 @@ export const getWalletName = (): string => { | |||
| export const getLastActiveTab: () => Promise<Tabs.Tab> = async () => | |||
| await ( | |||
| await tabs.query({ currentWindow: true, active: true }) | |||
| )[0]; | |||
| )[0]; // eslint-disable-line unicorn/no-await-expression-member | |||
There was a problem hiding this comment.
Consider disabling this rule.
Proposed solution
Minor fixes and updates for
eslintandlint-stagedconfigurations.Allure report
allure-report-publishergenerated test report!smokeTests: ✅ test report for c3ffd99d