Skip to content

Commit

Permalink
Lint rule to detect broken Google Translate in CommissionBounds
Browse files Browse the repository at this point in the history
The reason d85c5ff was needed is:
- `CommissionBounds` returned <>string</>
- `details ? <CommissionBounds /> : <Spinner />` conditionally inserted it
  • Loading branch information
lukaw3d committed Jun 20, 2024
1 parent d85c5ff commit 0622983
Show file tree
Hide file tree
Showing 10 changed files with 51 additions and 7 deletions.
1 change: 1 addition & 0 deletions .changelog/1984.internal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Lint rule to detect broken Google Translate in CommissionBounds
15 changes: 15 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,21 @@ const noGoogleTranslateCrashingSyntax = [
'Plain text nodes before or after a condition could break React if used with Google Translate. Identifier could possibly return a string, so wrap it into an element.',
},

// Ban components returning text or variables inside fragments `return <>{t('text')}</>` (just in case vars return a string and parent components are conditionally inserted)
// TODO: doesn't catch `return t('text')`
{
selector:
':matches(ArrowFunctionExpression, ReturnStatement) > JSXFragment > JSXExpressionContainer > :not(' +
' JSXEmptyExpression, ' + // Allow `<>{/* Comment */}</>`
' ConditionalExpression[consequent.type="JSXElement"][alternate.type="JSXElement"], ' + // Allow `<><Box></Box>{condition ? <Box></Box> : <Box></Box>}</>`
' LogicalExpression[right.type="JSXElement"], ' + // Allow `<><Box></Box>{condition && <Box></Box>}</>`
' Identifier[name="children"], ' + // Allow `<>{children}</>`
' MemberExpression[property.name="children"]' + // Allow `<>{someProps.children}</>`
')',
message:
'Text nodes inside React fragments could break React if used with Google Translate. Identifier could possibly return a string, and parent components could be conditionally inserted, so wrap it into an element.',
},

// TODO: Nesting is not supported. Detect it as error for now
{
selector: 'JSXElement > JSXExpressionContainer > ConditionalExpression > ConditionalExpression',
Expand Down
2 changes: 1 addition & 1 deletion src/app/components/DateFormatter/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ interface Props {
}

export function DateFormatter(props: Props) {
return <>{intlDateTimeFormat(props.date)}</>
return <span>{intlDateTimeFormat(props.date)}</span>
}
2 changes: 1 addition & 1 deletion src/app/components/ErrorFormatter/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -105,5 +105,5 @@ export function ErrorFormatter(props: Props) {
}

const error = errorMap[props.code]
return <>{error}</>
return <span>{error}</span>
}
2 changes: 1 addition & 1 deletion src/app/components/ImportAccountsStepFormatter/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ export const ImportAccountsStepFormatter = memo((props: Props) => {
}

const message = stepMap[step]
return <>{message}</>
return <span>{message}</span>
})
2 changes: 1 addition & 1 deletion src/app/components/TimeToEpoch/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@ export function TimeToEpoch(props: Props) {
? relativeFormat.format(Math.round(remainingHours / 24), 'day')
: relativeFormat.format(remainingHours, 'hour')

return <>{formattedRemainingTime}</>
return <span>{formattedRemainingTime}</span>
}
2 changes: 1 addition & 1 deletion src/app/components/Transaction/__tests__/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jest.mock('copy-to-clipboard')

type TransType = typeof Trans
jest.mock('react-i18next', () => ({
Trans: (({ i18nKey }) => <>{i18nKey}</>) as TransType,
Trans: (({ i18nKey }) => i18nKey) as TransType,
useTranslation: () => {
return {
t: (str: string) => str,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,9 @@ exports[`<TransactionError /> should render component 1`] = `
class="c11"
style="vertical-align: middle;"
>
Unknown ParaTime error: error message
<span>
Unknown ParaTime error: error message
</span>
</span>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,9 @@ exports[`<DebondingDelegationList /> should match snapshot 1`] = `
id="cell-debondingTimeEnd-test-validator+100"
role="gridcell"
>
in 4 days
<span>
in 4 days
</span>
</div>
</div>
</div>
Expand Down
24 changes: 24 additions & 0 deletions src/utils/eslint-test-noGoogleTranslateCrashingSyntax.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* these work like "expect eslint error".
*/
/* eslint-disable @typescript-eslint/no-unused-vars */
/* eslint-disable react-refresh/only-export-components */

const condition = true
const bad = 'bad'
Expand Down Expand Up @@ -194,6 +195,29 @@ const textAfterTernaryOperator = (
</div>
)

function TextInFragment() {
const Arrow1 = () => <>{condition && <span>good</span>}</>
/* eslint-disable-next-line no-restricted-syntax */
const Arrow2 = () => <>{!condition ? <span>good</span> : 'bad'}</>

return (
<>
{/* good */}
{condition ? <Arrow1 /> : <Arrow2 />}
{condition && <span>good</span>}
{!condition ? <span>good</span> : <span>good</span>}
{/* eslint-disable-next-line no-restricted-syntax */}
{!condition ? <span>good</span> : 'bad'}
{/* eslint-disable-next-line no-restricted-syntax */}
{'bad'}
{/* eslint-disable-next-line no-restricted-syntax */}
{bad}
{/* eslint-disable-next-line no-restricted-syntax */}
{bad.toString()}
</>
)
}

const nestedConditionsAreNotSupported = (
<div>
<span>
Expand Down

0 comments on commit 0622983

Please sign in to comment.