eui icon indicating copy to clipboard operation
eui copied to clipboard

Published 20 hours ago • verified publisher icon elastic

[eslint] Custom lint rule to find non-localized text in JSX

Open chandlerprall opened this issue 4 years ago • 5 comments

Summary

Caroline mentioned adding an item to our PR checklist to help catch hard-coded text in our components. I was curious if we could instead enforce this with a lint rule, and this is where I ended up. I disabled the check for jest & cypress tests.

This rule is not comprehensive! There are ways to bypass its logic, some of which are known but I'm sure there are many that are not. For example, you can do something like <MyComponent customLabel="something" /> and it will be allowed as only children are verified, and inside MyComponent the rule will see customLabel comes from a prop and assume it's safe.

The rule has flagged 44 existing problems in the codebase across 29 files:

Needs to use i18n

src/components/combo_box/combo_box_input/combo_box_input.tsx

  • [ ] line 216 removeOptionMessageContent has hard-coded text

src/components/datagrid/body/header/data_grid_header_cell.tsx

  • [ ] line 128 sortString conditional should be explicit, sortString != null
  • [ ] line 130 sortString has hard-coded text

src/components/date_picker/super_date_picker/date_popover/absolute_tab.tsx

  • [ ] line 131 sentenceCasedPosition is technically safe to use, but our sentence-casing strategy is an enemy to localization
  • [ ] line 131 has hard-coded text

src/components/date_picker/super_date_picker/date_popover/date_popover_button.tsx

  • [ ] line 107 formatTimeString needs localization work, and then be marked safe

src/components/date_picker/super_date_picker/date_popover/date_popover_content.tsx

src/components/date_picker/super_date_picker/quick_select_popover/recently_used.tsx

  • [ ] line 48 prettyDuration needs localization work, and then be marked safe

src/components/date_picker/super_date_picker/super_date_picker.tsx

  • [ ] line 381 prettyInterval needs localization work, and then be marked safe
  • [ ] line 408 prettyDuration needs localization work, and then be marked safe

src/components/markdown_editor/markdown_editor_footer.tsx

  • [ ] line 241 syntax prefix/link/suffix should be refactored to use a single localization token
  • [ ] line 291 syntax description/link should be refactored to use a single localization token

src/components/markdown_editor/plugins/markdown_tooltip/plugin.tsx

  • [ ] line 25 anchor & tooltip text need to be localized

src/components/notification/notification_event_meta.tsx

  • [ ] line 116 template literal should be localized

src/components/search_bar/filters/field_value_selection_filter.tsx

  • [ ] line 478 defaults need to be localized
  • [ ] line 504 defaults need to be localized

src/components/stat/stat.tsx

  • [ ] line 151 template literal that is probably better as a localization token; I feel this file may be a strong candidate for an accessibility audit

src/components/table/table_pagination/table_pagination.tsx

  • [ ] line 84 the : and itemsPerPage shoul dmove into the rowsPerPage i18n usage

Safe code that is flagged

These cases are okay and we need to find a way to ignore the error, my ideas are (in order of personal preference):

  • configuration option to allowlist functions/identifiers
  • in-code method to identify safe usages (e.g. decorators)
  • eslint-disable-line

src/components/avatar/avatar.tsx

  • [ ] line 165 calculatedInitials is a string computed by a function

src/components/basic_table/basic_table.tsx

  • [ ] line 726 captionElement can be the tableCaption from props

src/components/datagrid/body/popover_utils.tsx

  • [ ] line 39 we don't control the value of formattedText

src/components/expression/expression.tsx

  • [ ] line 160 lint rule needs updating to allow white-space string literals

src/components/form/form_row/form_row.tsx

  • [ ] line 248 lint rule needs updating to allow white-space string literals

src/components/highlight/highlight.tsx

src/components/markdown_editor/markdown_editor_footer.tsx

  • [ ] line 178 message ultimately comes from props and is safe

src/components/markdown_editor/markdown_format.tsx

  • [ ] line 60 result needs to be marked as safe

src/components/search_bar/filters/field_value_selection_filter.tsx

  • [ ] line 454 needs to be marked as safe

src/components/search_bar/filters/field_value_toggle_filter.tsx

  • [ ] line 70 this.resolveDisplay needs to be marked safe

src/components/search_bar/filters/field_value_toggle_group_filter.tsx

  • [ ] line 88 this.resolveDisplay needs to be marked safe

src/components/search_bar/filters/is_filter.tsx

  • [ ] line 65 this.resolveDisplay needs to be marked safe

src/components/selectable/selectable.tsx

  • [ ] line 605 messageContent is a ReactNode but being flagged as a string

src/components/selectable/selectable_list/selectable_list.tsx

src/components/steps/step_number.tsx

  • [ ] line 93 screenReaderText is safe, perhaps we update the lint rule to understand the useI18n*Step hooks as okay?

src/components/tabs/tab.tsx

  • [ ] line 105 prependNode is a ReactElement but flagged as a string
  • [ ] line 107 appendNode is a ReactElement but flagged as a string

Miscellaneous

src/components/date_picker/date_picker_range.tsx

  • [ ] line 117 uses an arrow character, →. I dunno if arrows should be localized or not, need to research.

chandlerprall avatar Nov 22 '21 17:11 chandlerprall

Preview documentation changes for this PR: https://eui.elastic.co/pr_5399/

kibanamachine avatar Nov 22 '21 18:11 kibanamachine

👋 Hey there. This PR hasn't had any activity for 90 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

github-actions[bot] avatar Apr 06 '22 00:04 github-actions[bot]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5399/

kibanamachine avatar Apr 06 '22 00:04 kibanamachine

👋 Hey there. This PR hasn't had any activity for 90 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

github-actions[bot] avatar Jul 05 '22 16:07 github-actions[bot]

Nothing like a stale check to remind yourself of the continual passage of time

chandlerprall avatar Jul 05 '22 16:07 chandlerprall

👋 Hey there. This PR hasn't had any activity for 90 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

github-actions[bot] avatar Oct 04 '22 00:10 github-actions[bot]

👋 Hey there. This PR hasn't had any activity for 90 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

github-actions[bot] avatar Jan 03 '23 00:01 github-actions[bot]