frontend: add eslint rule restrict-template-expressions #2325
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
15 problems 🎉 |
So that we do not accidentally pass undefined, null, booleans or
just any type in tempalte literals, see:
`all these should error ${false} ${undefined} ${[]} ${null}`
This is a eslint rule, so only make weblint will check.
thisconnect
force-pushed
the
frontend-eslint-templateliterals
branch
from
cd023a2 to
f8bccf4
Compare
Last commit introduced restrict-template-expressions, with this all reported problems should be fixed.
There seem to be no major edge cases using numbers in template literals and allowing them seems reasonable.
In c6ee6e0 the large prop on the SwissMadeOpenSource component was removed as it did not work since a long time and was for BitBox01 only, this commit removes the unused CSS.
thisconnect
commented
Dec 2, 2024
| showQRCode: boolean; | ||
| type State = { | ||
| channel: string | null; | ||
| status: string | false; |
There was a problem hiding this comment.
changed boolean -> false as it is initialized with false.
| export const BitBoxSwissInverted = (props: GenericProps) => <img {...props} draggable={false} src={BitBoxSwissInvertedLogo} alt="BitBox" className={style.logo} />; | ||
| export const Shift = (props: GenericProps) => <img {...props} draggable={false} src={ShiftLogo} alt="Shift Crypto" className={style.logo} />; | ||
| export const SwissMadeOpenSource = ({ large: boolean, className, ...props }: GenericProps) => <img {...props} draggable={false} src={SwissOpenSourceLight} alt="Swiss Made Open Source" className={`${style.swissOpenSource} ${props.large ? style.large : ''} ${className ? className : ''}`} />; | ||
| export const SwissMadeOpenSourceDark = ({ large: boolean, className, ...props }: GenericProps) => <img {...props} draggable={false} src={SwissOpenSourceDark} alt="Swiss Made Open Source" className={`${style.swissOpenSource} ${props.large ? style.large : ''} ${className ? className : ''}`} />; |
There was a problem hiding this comment.
Removed large as this didn't work and props.large was always undefined:
({ large, ...props }) => props.large // props.large is always undefined| @@ -56,23 +56,22 @@ import PAXG_GREY from './assets/paxg-white.svg'; | |||
| import ShiftLogo from './assets/shift-cryptosecurity-logo.svg'; | |||
| import style from './logo.module.css'; | |||
|
|
|||
| interface GenericProps { | |||
| [property: string]: any; | |||
| } | |||
There was a problem hiding this comment.
removed and replaced by type ImgProps = JSX.IntrinsicElements['img']; as we already do in icons.tsx
| navigate(`/account/${code}/receive`); | ||
| if (code) { | ||
| navigate(`/account/${code}/receive`); | ||
| } |
There was a problem hiding this comment.
This looks a bit weird and could be cleaned up by refatoring AddBuyReceiveOnEmptyBalances and BuyReceiveCTA component.
I.e. balanceList is probably not really needed in BuyReceiveCTA and something like a boolean prop.isOnAccount or similar may be enough
| alertUser(this.props.t(`backup.restore.error.e${code}`, { | ||
| defaultValue: errorMessage, | ||
| })); | ||
| if (typeof code === 'string') { |
There was a problem hiding this comment.
some un-typed BitBox01 backups specific api call.
We usually know what type when we use the src/api backend calls, but here we call apiPost directly.
thisconnect
changed the title
shonsirsha
approved these changes
Dec 3, 2024
| @@ -41,7 +41,7 @@ export const Select = ({ | |||
| <select id={id} {...props}> | |||
| {options.map(({ value, text, disabled = false }) => ( | |||
| <option | |||
| key={`${value}`} | |||
| key={String(value)} | |||
There was a problem hiding this comment.
Is this necessary? I guess best practice, right?
There was a problem hiding this comment.
Without converting to string somehow i.e. String() this will error with:
Type 'string | number | readonly string[] | undefined' is not assignable to type 'Key | null | undefined'.
Type 'readonly string[]' is not assignable to type 'Key | null | undefined'.ts(2322)
index.d.ts(261, 9): The expected type comes from property 'key' which is declared here on type 'DetailedHTMLProps<OptionHTMLAttributes<HTMLOptionElement>, HTMLOptionElement>'
(property) Attributes.key?: Key | null | undefinedWe use the intrinsic element option (prob shouldn't :) ) this has the type value?: string | readonly string[] | number | undefined;
interface OptionHTMLAttributes<T> extends HTMLAttributes<T> {
disabled?: boolean | undefined;
label?: string | undefined;
selected?: boolean | undefined;
value?: string | readonly string[] | number | undefined;
}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So that we do not accidentally pass undefined, null, booleans or just any type in tempalte literals, see:
This is a eslint rule, so only make weblint will check.