chore(ui): enable react/recommended lint rules (#32214)

Closes https://github.com/microsoft/playwright/issues/32159. I originally set out to enable Strict Mode for our React UI, but found a way better thing: Enabling the lint rules we had already installed! `eslint-plugin-react` is already in of our `package.json`, and this PR enables it and fixes some of the reported issues. Most of them are around the `key` prop which is mostly about performance, but there's also fixes for misspelled `data-testid` props.
2024-11-24 06:49:04 +03:00 · 2024-08-20 14:16:28 +02:00 · 2024-08-20 14:16:28 +02:00 · b599335404
commit b599335404
parent 244761a3a2
21 changed files with 564 additions and 356 deletions
--- a/.eslintrc.js
+++ b/.eslintrc.js
@ -6,9 +6,14 @@ module.exports = {
      sourceType: "module",
    },
    extends: [
+      "plugin:react/recommended",
      "plugin:react-hooks/recommended"
    ],

+    settings: {
+        react: { version: "18" }
+    },
+
    /**
     * ESLint rules
     *
@ -124,5 +129,8 @@ module.exports = {
            "mustMatch": "Copyright",
            "templateFile": require("path").join(__dirname, "utils", "copyright.js"),
        }],
+
+        // react
+        "react/react-in-jsx-scope": 0
    }
 };
--- a/package-lock.json
+++ b/package-lock.json
--- a/package.json
+++ b/package.json
@ -85,8 +85,8 @@
    "eslint": "^8.55.0",
    "eslint-plugin-internal-playwright": "file:utils/eslint-plugin-internal-playwright",
    "eslint-plugin-notice": "^0.9.10",
-    "eslint-plugin-react": "^7.33.2",
-    "eslint-plugin-react-hooks": "^4.3.0",
+    "eslint-plugin-react": "^7.35.0",
+    "eslint-plugin-react-hooks": "^4.6.2",
    "formidable": "^2.1.1",
    "license-checker": "^25.0.1",
    "mime": "^3.0.0",
--- a/packages/html-reporter/src/icons.tsx
+++ b/packages/html-reporter/src/icons.tsx
@ -70,19 +70,19 @@ export const blank = () => {
 };

 export const externalLink = () => {
-  return <svg className='octicon' viewBox='0 0 16 16' width='16' height='16'><path fill-rule='evenodd' d='M10.604 1h4.146a.25.25 0 01.25.25v4.146a.25.25 0 01-.427.177L13.03 4.03 9.28 7.78a.75.75 0 01-1.06-1.06l3.75-3.75-1.543-1.543A.25.25 0 0110.604 1zM3.75 2A1.75 1.75 0 002 3.75v8.5c0 .966.784 1.75 1.75 1.75h8.5A1.75 1.75 0 0014 12.25v-3.5a.75.75 0 00-1.5 0v3.5a.25.25 0 01-.25.25h-8.5a.25.25 0 01-.25-.25v-8.5a.25.25 0 01.25-.25h3.5a.75.75 0 000-1.5h-3.5z'></path></svg>;
+  return <svg className='octicon' viewBox='0 0 16 16' width='16' height='16'><path fillRule='evenodd' d='M10.604 1h4.146a.25.25 0 01.25.25v4.146a.25.25 0 01-.427.177L13.03 4.03 9.28 7.78a.75.75 0 01-1.06-1.06l3.75-3.75-1.543-1.543A.25.25 0 0110.604 1zM3.75 2A1.75 1.75 0 002 3.75v8.5c0 .966.784 1.75 1.75 1.75h8.5A1.75 1.75 0 0014 12.25v-3.5a.75.75 0 00-1.5 0v3.5a.25.25 0 01-.25.25h-8.5a.25.25 0 01-.25-.25v-8.5a.25.25 0 01.25-.25h3.5a.75.75 0 000-1.5h-3.5z'></path></svg>;
 };

 export const calendar = () => {
-  return <svg className='octicon' viewBox='0 0 16 16' width='16' height='16'><path fill-rule='evenodd' d='M4.75 0a.75.75 0 01.75.75V2h5V.75a.75.75 0 011.5 0V2h1.25c.966 0 1.75.784 1.75 1.75v10.5A1.75 1.75 0 0113.25 16H2.75A1.75 1.75 0 011 14.25V3.75C1 2.784 1.784 2 2.75 2H4V.75A.75.75 0 014.75 0zm0 3.5h8.5a.25.25 0 01.25.25V6h-11V3.75a.25.25 0 01.25-.25h2zm-2.25 4v6.75c0 .138.112.25.25.25h10.5a.25.25 0 00.25-.25V7.5h-11z'></path></svg>;
+  return <svg className='octicon' viewBox='0 0 16 16' width='16' height='16'><path fillRule='evenodd' d='M4.75 0a.75.75 0 01.75.75V2h5V.75a.75.75 0 011.5 0V2h1.25c.966 0 1.75.784 1.75 1.75v10.5A1.75 1.75 0 0113.25 16H2.75A1.75 1.75 0 011 14.25V3.75C1 2.784 1.784 2 2.75 2H4V.75A.75.75 0 014.75 0zm0 3.5h8.5a.25.25 0 01.25.25V6h-11V3.75a.25.25 0 01.25-.25h2zm-2.25 4v6.75c0 .138.112.25.25.25h10.5a.25.25 0 00.25-.25V7.5h-11z'></path></svg>;
 };

 export const person = () => {
-  return <svg className='octicon' viewBox='0 0 16 16' width='16' height='16'><path fill-rule='evenodd' d='M10.5 5a2.5 2.5 0 11-5 0 2.5 2.5 0 015 0zm.061 3.073a4 4 0 10-5.123 0 6.004 6.004 0 00-3.431 5.142.75.75 0 001.498.07 4.5 4.5 0 018.99 0 .75.75 0 101.498-.07 6.005 6.005 0 00-3.432-5.142z'></path></svg>;
+  return <svg className='octicon' viewBox='0 0 16 16' width='16' height='16'><path fillRule='evenodd' d='M10.5 5a2.5 2.5 0 11-5 0 2.5 2.5 0 015 0zm.061 3.073a4 4 0 10-5.123 0 6.004 6.004 0 00-3.431 5.142.75.75 0 001.498.07 4.5 4.5 0 018.99 0 .75.75 0 101.498-.07 6.005 6.005 0 00-3.432-5.142z'></path></svg>;
 };

 export const commit = () => {
-  return <svg className='octicon' viewBox='0 0 16 16' width='16' height='16'><path fill-rule='evenodd' d='M10.5 7.75a2.5 2.5 0 11-5 0 2.5 2.5 0 015 0zm1.43.75a4.002 4.002 0 01-7.86 0H.75a.75.75 0 110-1.5h3.32a4.001 4.001 0 017.86 0h3.32a.75.75 0 110 1.5h-3.32z'></path></svg>;
+  return <svg className='octicon' viewBox='0 0 16 16' width='16' height='16'><path fillRule='evenodd' d='M10.5 7.75a2.5 2.5 0 11-5 0 2.5 2.5 0 015 0zm1.43.75a4.002 4.002 0 01-7.86 0H.75a.75.75 0 110-1.5h3.32a4.001 4.001 0 017.86 0h3.32a.75.75 0 110 1.5h-3.32z'></path></svg>;
 };

 export const image = () => {
--- a/packages/html-reporter/src/links.tsx
+++ b/packages/html-reporter/src/links.tsx
@ -81,7 +81,7 @@ export const AttachmentLink: React.FunctionComponent<{
    {attachment.path && <a href={href || attachment.path} download={downloadFileNameForAttachment(attachment)}>{linkName || attachment.name}</a>}
    {!attachment.path && <span>{linkifyText(attachment.name)}</span>}
  </span>} loadChildren={attachment.body ? () => {
-    return [<div className='attachment-body'><CopyToClipboard value={attachment.body!}/>{linkifyText(attachment.body!)}</div>];
+    return [<div key={1} className='attachment-body'><CopyToClipboard value={attachment.body!}/>{linkifyText(attachment.body!)}</div>];
  } : undefined} depth={0} style={{ lineHeight: '32px' }}></TreeItem>;
 };

--- a/packages/html-reporter/src/testCaseView.tsx
+++ b/packages/html-reporter/src/testCaseView.tsx
@ -58,7 +58,7 @@ export const TestCaseView: React.FC<{
      {labels && <LabelsLinkView labels={labels} />}
    </div>}
    {!!visibleAnnotations.length && <AutoChip header='Annotations'>
-      {visibleAnnotations.map(annotation => <TestCaseAnnotationView annotation={annotation} />)}
+      {visibleAnnotations.map((annotation, index) => <TestCaseAnnotationView key={index} annotation={annotation} />)}
    </AutoChip>}
    {test && <TabbedPane tabs={
      test.results.map((result, index) => ({
--- a/packages/recorder/src/recorder.tsx
+++ b/packages/recorder/src/recorder.tsx
@ -171,7 +171,7 @@ export const Recorder: React.FC<RecorderProps> = ({
      sidebarSize={200}
      main={<CodeMirrorWrapper text={source.text} language={source.language} highlight={source.highlight} revealLine={source.revealLine} readOnly={true} lineNumbers={true} />}
      sidebar={<TabbedPane
-        rightToolbar={selectedTab === 'locator' ? [<ToolbarButton icon='files' title='Copy' onClick={() => copy(locator)} />] : []}
+        rightToolbar={selectedTab === 'locator' ? [<ToolbarButton key={1} icon='files' title='Copy' onClick={() => copy(locator)} />] : []}
        tabs={[
          {
            id: 'locator',
--- a/packages/trace-viewer/src/ui/attachmentsTab.tsx
+++ b/packages/trace-viewer/src/ui/attachmentsTab.tsx
@ -126,7 +126,7 @@ export const AttachmentsTab: React.FunctionComponent<{
      const url = attachmentURL(a);
      return <div className='attachment-item' key={`screenshot-${i}`}>
        <div><img draggable='false' src={url} /></div>
-        <div><a target='_blank' href={url}>{a.name}</a></div>
+        <div><a target='_blank' href={url} rel='noreferrer'>{a.name}</a></div>
      </div>;
    })}
    {attachments.size ? <div className='attachments-section'>Attachments</div> : undefined}
--- a/packages/trace-viewer/src/ui/consoleTab.tsx
+++ b/packages/trace-viewer/src/ui/consoleTab.tsx
@ -213,6 +213,7 @@ function format(args: { preview: string, value: any }[]): JSX.Element[] {
 }

 function formatAnsi(text: string): JSX.Element[] {
+  // eslint-disable-next-line react/jsx-key
  return [<span dangerouslySetInnerHTML={{ __html: ansi2html(text.trim()) }}></span>];
 }

--- a/packages/trace-viewer/src/ui/networkFilters.tsx
+++ b/packages/trace-viewer/src/ui/networkFilters.tsx
@ -26,10 +26,10 @@ export type FilterState = {

 export const defaultFilterState: FilterState = { searchValue: '', resourceType: 'All' };

-export const NetworkFilters: React.FunctionComponent<{
+export const NetworkFilters = ({ filterState, onFilterStateChange }: {
  filterState: FilterState,
  onFilterStateChange: (filterState: FilterState) => void,
-}> = ({ filterState, onFilterStateChange }) => {
+}) => {
  return (
    <div className='network-filters'>
      <input
--- a/packages/trace-viewer/src/ui/networkResourceDetails.tsx
+++ b/packages/trace-viewer/src/ui/networkResourceDetails.tsx
@ -29,7 +29,7 @@ export const NetworkResourceDetails: React.FunctionComponent<{

  return <TabbedPane
    dataTestId='network-request-details'
-    leftToolbar={[<ToolbarButton icon='close' title='Close' onClick={onClose}></ToolbarButton>]}
+    leftToolbar={[<ToolbarButton key='close' icon='close' title='Close' onClick={onClose}></ToolbarButton>]}
    tabs={[
      {
        id: 'request',
--- a/packages/trace-viewer/src/ui/snapshotTab.tsx
+++ b/packages/trace-viewer/src/ui/snapshotTab.tsx
@ -184,6 +184,7 @@ export const SnapshotTab: React.FunctionComponent<{
      <ToolbarButton className='pick-locator' title='Pick locator' icon='target' toggled={isInspecting} onClick={() => setIsInspecting(!isInspecting)} />
      {['action', 'before', 'after'].map(tab => {
        return <TabbedPaneTab
+          key={tab}
          id={tab}
          title={renderTitle(tab)}
          selected={snapshotTab === tab}
--- a/packages/trace-viewer/src/ui/tag.tsx
+++ b/packages/trace-viewer/src/ui/tag.tsx
@ -17,7 +17,7 @@
 import { clsx } from '@web/uiUtils';
 import './tag.css';

-export const TagView: React.FC<{ tag: string, style?: React.CSSProperties, onClick?: (e: React.MouseEvent) => void }> = ({ tag, style, onClick }) => {
+export const TagView = ({ tag, style, onClick }: { tag: string, style?: React.CSSProperties, onClick?: (e: React.MouseEvent) => void }) => {
  return <span
    className={clsx('tag', `tag-color-${tagNameToColor(tag)}`)}
    onClick={onClick}
--- a/packages/trace-viewer/src/ui/uiModeFiltersView.tsx
+++ b/packages/trace-viewer/src/ui/uiModeFiltersView.tsx
@ -60,7 +60,7 @@ export const FiltersView: React.FC<{
    {expanded && <div className='hbox' style={{ marginLeft: 14, maxHeight: 200, overflowY: 'auto' }}>
      <div className='filter-list'>
        {[...statusFilters.entries()].map(([status, value]) => {
-          return <div className='filter-entry'>
+          return <div className='filter-entry' key={status}>
            <label>
              <input type='checkbox' checked={value} onClick={() => {
                const copy = new Map(statusFilters);
@ -74,7 +74,7 @@ export const FiltersView: React.FC<{
      </div>
      <div className='filter-list'>
        {[...projectFilters.entries()].map(([projectName, value]) => {
-          return <div className='filter-entry'>
+          return <div className='filter-entry' key={projectName}>
            <label>
              <input type='checkbox' checked={value} onClick={() => {
                const copy = new Map(projectFilters);
--- a/packages/web/src/components/gridView.tsx
+++ b/packages/web/src/components/gridView.tsx
@ -76,6 +76,7 @@ export function GridView<T>(model: GridViewProps<T>) {
      <div className='grid-view-header'>
        {model.columns.map((column, i) => {
          return <div
+            key={model.columnTitle(column)}
            className={'grid-view-header-cell ' + sortingHeader(column, model.sorting)}
            style={{
              width: i < model.columns.length - 1 ? model.columnWidths.get(column) : undefined,
@ -97,6 +98,7 @@ export function GridView<T>(model: GridViewProps<T>) {
            {model.columns.map((column, i) => {
              const { body, title } = model.render(item, column, index);
              return <div
+                key={model.columnTitle(column)}
                className={`grid-view-cell grid-view-column-${String(column)}`}
                title={title}
                style={{
--- a/packages/web/src/components/listView.tsx
+++ b/packages/web/src/components/listView.tsx
@ -152,6 +152,7 @@ export function ListView<T>({
          onMouseEnter={() => setHighlightedItem(item)}
          onMouseLeave={() => setHighlightedItem(undefined)}
        >
+          {/* eslint-disable-next-line react/jsx-key */}
          {indentation ? new Array(indentation).fill(0).map(() => <div className='list-view-indent'></div>) : undefined}
          {icon && <div
            className={'codicon ' + (icon(item, index) || 'codicon-blank')}
--- a/packages/web/src/components/tabbedPane.tsx
+++ b/packages/web/src/components/tabbedPane.tsx
@ -48,6 +48,7 @@ export const TabbedPane: React.FunctionComponent<{
        {mode === 'default' && <div style={{ flex: 'auto', display: 'flex', height: '100%', overflow: 'hidden' }}>
          {[...tabs.map(tab => (
            <TabbedPaneTab
+              key={tab.id}
              id={tab.id}
              title={tab.title}
              count={tab.count}
@ -67,7 +68,7 @@ export const TabbedPane: React.FunctionComponent<{
                suffix = ` (${tab.count})`;
              if (tab.errorCount)
                suffix = ` (${tab.errorCount})`;
-              return <option value={tab.id} selected={tab.id === selectedTab}>{tab.title}{suffix}</option>;
+              return <option key={tab.id} value={tab.id} selected={tab.id === selectedTab}>{tab.title}{suffix}</option>;
            })}
          </select>
        </div>}
--- a/packages/web/src/components/toolbarButton.tsx
+++ b/packages/web/src/components/toolbarButton.tsx
@ -51,7 +51,7 @@ export const ToolbarButton: React.FC<React.PropsWithChildren<ToolbarButtonProps>
    title={title}
    disabled={!!disabled}
    style={style}
-    data-testId={testId}
+    data-testid={testId}
  >
    {icon && <span className={`codicon codicon-${icon}`} style={children ? { marginRight: 5 } : {}}></span>}
    {children}
--- a/packages/web/src/shared/imageDiffView.tsx
+++ b/packages/web/src/shared/imageDiffView.tsx
@ -119,9 +119,9 @@ export const ImageDiffView: React.FC<{
        </div>}
      </div>
      <div style={{ alignSelf: 'start', lineHeight: '18px', marginLeft: '15px' }}>
-        <div>{diff.diff && <a target='_blank' href={diff.diff.attachment.path}>{diff.diff.attachment.name}</a>}</div>
-        <div><a target={noTargetBlank ? '' : '_blank'} href={diff.actual!.attachment.path}>{diff.actual!.attachment.name}</a></div>
-        <div><a target={noTargetBlank ? '' : '_blank'} href={diff.expected!.attachment.path}>{diff.expected!.attachment.name}</a></div>
+        <div>{diff.diff && <a target='_blank' href={diff.diff.attachment.path} rel='noreferrer'>{diff.diff.attachment.name}</a>}</div>
+        <div><a target={noTargetBlank ? '' : '_blank'} href={diff.actual!.attachment.path} rel='noreferrer'>{diff.actual!.attachment.name}</a></div>
+        <div><a target={noTargetBlank ? '' : '_blank'} href={diff.expected!.attachment.path} rel='noreferrer'>{diff.expected!.attachment.name}</a></div>
      </div>
    </>}
  </div>;
--- a/packages/web/src/shared/resizeView.tsx
+++ b/packages/web/src/shared/resizeView.tsx
@ -82,6 +82,7 @@ export const ResizeView: React.FC<{
    />}
    {offsets.map((offset, index) => {
      return <div
+        key={index}
        style={{
          ...dividerStyle,
          top: orientation === 'horizontal' ? 0 : offset,
--- a/tests/components/ct-react-vite/src/components/Fetcher.tsx
+++ b/tests/components/ct-react-vite/src/components/Fetcher.tsx
@ -20,7 +20,7 @@ export default function Fetcher() {
  }, [fetched, setFetched, setData]);

  return <div>
-    <div data-testId='name'>{data.name}</div>
+    <div data-testid='name'>{data.name}</div>
    <button onClick={() => {
      setFetched(false);
      setData({ name: '<none>' });