diff --git a/.circleci/config.yml b/.circleci/config.yml index bf208573026..6beac901ce9 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -34,7 +34,7 @@ commands: steps: - run: name: Build Docker image - command: docker build -f docker/Dockerfile -t app:build . + command: DOCKER_BUILDKIT=1 docker build -f docker/Dockerfile -t app:build . deploy-to-dockerhub: description: "Deploy to Docker Hub" @@ -100,21 +100,25 @@ jobs: javascript-tests: executor: name: node/default - # The Node version here must be kept in sync with that in `package.json`. - tag: '22.11.0' + # The Node version here should be compatible with `package.json` engines. + # Note: cimg/node requires at least a minor version (e.g., '22.0'), not just major version + tag: '22.0' steps: - checkout - - node/install-packages: - # `yarn install --frozen-lockfile` is run and cache is enabled by default for this orb configuration - pkg-manager: yarn - run: - command: yarn lint + name: Install pnpm + command: sudo corepack enable && corepack prepare pnpm@9.15.0 --activate + - run: + name: Install dependencies + command: pnpm install --frozen-lockfile + - run: + command: pnpm lint name: Run linting - run: - command: yarn markdownlint + command: pnpm markdownlint name: Check markdown linting - run: - command: yarn test:coverage + command: pnpm test:ci name: Run Jest tests - codecov/upload @@ -123,10 +127,6 @@ jobs: - image: 'cimg/python:3.10-node' steps: - checkout - - restore_cache: - name: Restore Yarn Package Cache - keys: - - node-deps-v1-{{ .Branch }} - run: pip install tox - run: command: tox -e docs diff --git a/.gitignore b/.gitignore index 9fa2a09e3d8..e3f602acfcf 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ treeherder_backup_* # Caches .eslintcache +.jest-cache # PyCharm *.iml # Perf sheriffing criteria diff --git a/.markdownlintignore b/.markdownlintignore index 9b541f55d02..b9f5af39e61 100644 --- a/.markdownlintignore +++ b/.markdownlintignore @@ -1,2 +1,4 @@ docs/index.md -node_modules \ No newline at end of file +node_modules +venv +CLAUDE.md \ No newline at end of file diff --git a/.npmrc b/.npmrc new file mode 100644 index 00000000000..06bbb7aaec6 --- /dev/null +++ b/.npmrc @@ -0,0 +1,17 @@ +# pnpm configuration for Treeherder + +# Equivalent to yarn's ignore-engines - don't check engine compatibility +engine-strict=false + +# Save exact versions instead of ranges when adding packages +save-exact=true + +# Don't run lifecycle scripts during install for security +ignore-scripts=true + +# Hoist all dependencies to node_modules root for compatibility +# This makes pnpm behavior similar to npm/yarn flat node_modules +shamefully-hoist=true + +# Auto-install peers to avoid peer dependency warnings +auto-install-peers=true diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 09448c62ff6..24dd0a6d994 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -3,10 +3,11 @@ repos: rev: v0.10.0.1 hooks: - id: shellcheck - - repo: https://github.com/pre-commit/mirrors-prettier - rev: v2.2.1 + - repo: https://github.com/biomejs/pre-commit + rev: v2.1.1 hooks: - - id: prettier + - id: biome-ci + additional_dependencies: ["@biomejs/biome@2.3.10"] - repo: https://github.com/igorshubovych/markdownlint-cli rev: v0.43.0 hooks: diff --git a/.prettierignore b/.prettierignore index 05de75329cb..58b5554cd97 100644 --- a/.prettierignore +++ b/.prettierignore @@ -1,5 +1,8 @@ # Ignore generated directories. .*/ +venv/ +node_modules/ +pnpm-lock.yaml # Ignore our legacy JS since it will be rewritten when converted to React. ui/js/ diff --git a/.yarnrc b/.yarnrc deleted file mode 100644 index 6b4da54605a..00000000000 --- a/.yarnrc +++ /dev/null @@ -1,20 +0,0 @@ -# Whilst in theory yarn's checking of the node/yarn version against the `engines` property -# in package.json is a great idea, in practice it causes unnecessary confusion/hassle for -# contributors, since most node/yarn versions will work fine with Treeherder anyway. -ignore-engines true - -# `--no-bin-links` is required when using Vagrant on Windows hosts (where symlinks aren't -# allowed), and so we include here to ensure that the package.json scripts aren't relying -# on symlinks that won't exist elsewhere. This alternate config form is required due to: -# https://github.com/yarnpkg/yarn/issues/4925 ---*.no-bin-links true - -# Don't run preinstall/install/postinstall hooks during yarn install, since: -# - most are not actually required -# - they often don't work with --no-bin-links (the bin scripts called may not exist) -# - they cause Yarn to "unplug" the package when using the "Plug and Play" feature -# - it's more secure to not run arbitrary code during package installation ---ignore-scripts true - -# Default to saving the exact package version in package.json and not a tilde version range. -save-exact true diff --git a/babel.config.json b/babel.config.json index 76acd15661c..89539187ba2 100644 --- a/babel.config.json +++ b/babel.config.json @@ -18,18 +18,7 @@ [ "@babel/preset-react", { - "development": true, - "useSpread": true - } - ] - ], - "plugins": [ - "@babel/plugin-syntax-dynamic-import", - "react-hot-loader/babel", - [ - "@babel/plugin-proposal-class-properties", - { - "loose": true + "runtime": "automatic" } ] ] diff --git a/biome.json b/biome.json new file mode 100644 index 00000000000..2801e064bfe --- /dev/null +++ b/biome.json @@ -0,0 +1,74 @@ +{ + "$schema": "https://biomejs.dev/schemas/2.3.10/schema.json", + "vcs": { + "enabled": true, + "clientKind": "git", + "useIgnoreFile": true + }, + "files": { + "ignoreUnknown": true, + "includes": ["ui/**", "tests/ui/**"] + }, + "formatter": { + "enabled": false + }, + "javascript": { + "formatter": { + "quoteStyle": "single", + "trailingCommas": "all" + }, + "globals": ["page", "browser", "jestPuppeteer"] + }, + "css": { + "linter": { + "enabled": true + } + }, + "linter": { + "enabled": true, + "rules": { + "recommended": true, + "a11y": { + "useKeyWithClickEvents": "off", + "useSemanticElements": "off", + "noStaticElementInteractions": "off", + "useAriaPropsSupportedByRole": "off", + "useGenericFontNames": "off", + "noSvgWithoutTitle": "off" + }, + "complexity": { + "noUselessFragments": "off", + "useFlatMap": "off", + "noStaticOnlyClass": "off" + }, + "correctness": { + "noUnusedVariables": "error", + "noUnusedImports": "warn", + "noUnusedFunctionParameters": "off", + "useJsxKeyInIterable": "error", + "useExhaustiveDependencies": "off", + "noUnknownProperty": "off" + }, + "style": { + "noParameterAssign": "off", + "useDefaultParameterLast": "off", + "noNonNullAssertion": "off", + "useNodejsImportProtocol": "off" + }, + "suspicious": { + "noAlert": "off", + "noConsole": "off", + "noArrayIndexKey": "off", + "noImportAssign": "error", + "useIterableCallbackReturn": "off", + "noAsyncPromiseExecutor": "warn" + }, + "performance": { + "noAccumulatingSpread": "off" + } + } + }, + "assist": { + "enabled": false + } +} diff --git a/docker-compose.yml b/docker-compose.yml index 02ce9e1e2b2..bb0402e4d4a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -60,13 +60,13 @@ services: # https://hub.docker.com/_/node image: node:23.5.0-alpine3.20 # Installing JS dependencies at runtime so that they share the `node_modules` from the - # host, improving speed (both install and build due to the webpack cache) and ensuring + # host, improving speed (both install and build due to the rspack cache) and ensuring # the host copy stays in sync (for people that switch back and forth between UI-only # and full stack Treeherder development). working_dir: /app environment: BACKEND: http://backend:8000 - command: sh -c "yarn && yarn start --host 0.0.0.0" + command: sh -c "corepack enable && corepack prepare pnpm@9.15.0 --activate && pnpm install && pnpm start --host 0.0.0.0" volumes: - .:/app ports: diff --git a/docker/Dockerfile b/docker/Dockerfile index 7305fe49b80..9187ae2c966 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -1,14 +1,16 @@ ## Frontend stage -FROM node:22.11.0 AS frontend +FROM node:22-slim AS frontend WORKDIR /app +# Install pnpm +RUN corepack enable && corepack prepare pnpm@9.15.0 --activate + COPY ui/ /app/ui/ -COPY package.json babel.config.json webpack.config.js yarn.lock /app/ +COPY package.json rspack.config.js pnpm-lock.yaml .npmrc /app/ -RUN npm install -g --force yarn@1.22.22 -RUN yarn install -RUN yarn build +RUN pnpm install --frozen-lockfile +RUN pnpm build ## Backend stage @@ -38,7 +40,7 @@ RUN python manage.py collectstatic --noinput # WhiteNoise can then serve in preference to the originals. This is required # since WhiteNoise's Django storage backend only gzips assets handled by # collectstatic, and so does not affect files in the `.build/` directory -# since they are instead generated by webpack. +# since they are instead generated by rspack. RUN python -m whitenoise.compress .build RUN groupadd --gid 9500 treeherder && \ diff --git a/docs/backend_tasks.md b/docs/backend_tasks.md index 53d611309b7..52c3c307a26 100644 --- a/docs/backend_tasks.md +++ b/docs/backend_tasks.md @@ -16,8 +16,8 @@ docker-compose run backend bash ...which saves having to wait for docker-compose to spin up for every test run. -`yarn build` will generate a `.build` directory which will be seen within the `backend` container. -If you don't have `yarn` working on your host you can run this instead `docker-compose run frontend sh -c "yarn && yarn build"` +`pnpm build` will generate a `.build` directory which will be seen within the `backend` container. +If you don't have `pnpm` working on your host you can run this instead `docker-compose run frontend sh -c "corepack enable && pnpm install && pnpm build"` Then run the individual tools within that shell, like so: diff --git a/docs/biome-cleanup-todo.md b/docs/biome-cleanup-todo.md new file mode 100644 index 00000000000..6232391306f --- /dev/null +++ b/docs/biome-cleanup-todo.md @@ -0,0 +1,170 @@ +# Biome Code Cleanup TODO + +This document lists suggested code improvements identified during the Biome migration. Items are prioritized by impact and effort. + +## Priority 1: Quick Wins (Low effort, High value) + +### Fix Missing React Keys + +**Rule:** `useJsxKeyInIterable` +**Current:** warn +**Files:** + +- `ui/infra-compare/InfraCompareTable.jsx:74` - Add key to `` +- `ui/infra-compare/InfraCompareTableControls.jsx:134` - Add key to `` +- `ui/perfherder/alerts/AlertHeader.jsx:233` - Add key to `` +- `ui/shared/AdditionalInformationTable.jsx:165` - Add key to `` +- `ui/shared/AdditionalInformationTable.jsx:183` - Add key to `` + +**Why:** Missing keys can cause React rendering bugs and performance issues. + +### Remove Unused Variables + +**Rule:** `noUnusedVariables` +**Current:** warn +**Files:** + +- `ui/perfherder/graphs/GraphsContainer.jsx:325` - `selectedDataPoint` unused +- `ui/perfherder/graphs/GraphsContainer.jsx:326` - `highlightAlerts` unused + +**Why:** Dead code that should be removed. + +### Fix Import Assignment + +**Rule:** `noImportAssign` +**Current:** warn +**File:** `ui/taskcluster-auth-callback/TaskclusterCallback.jsx:30` + +**Why:** Reassigning an imported variable is a bug - imports are read-only. Use a local variable instead. + +--- + +## Priority 2: Code Quality (Medium effort, High value) + +### Fix Async Promise Executor + +**Rule:** `noAsyncPromiseExecutor` +**Current:** warn +**File:** `ui/shared/auth/AuthService.js:19` + +**Why:** Async promise executors can silently swallow errors. Refactor to use async/await directly without wrapping in Promise. + +### Enable React Hooks Exhaustive Dependencies + +**Rule:** `useExhaustiveDependencies` +**Current:** off +**Estimated files:** ~10 + +**Why:** Missing dependencies in useEffect/useCallback can cause stale closures and subtle bugs. + +### Enable Array Index Key Warnings + +**Rule:** `noArrayIndexKey` +**Current:** off +**Estimated files:** ~5 + +**Why:** Using array index as key can cause issues when list items are reordered or filtered. + +--- + +## Priority 3: Performance (Medium effort, Medium value) + +### Fix Accumulating Spread Operations + +**Rule:** `noAccumulatingSpread` +**Current:** off +**Files:** Multiple locations in reducers and helpers + +**Why:** Spreading in a loop (e.g., `arr.reduce((acc, x) => [...acc, x])`) is O(n²). Use `push()` or `concat()` instead. + +### Enable useFlatMap + +**Rule:** `useFlatMap` +**Current:** off + +**Why:** Replace `.map().flat()` with `.flatMap()` for better performance and readability. + +--- + +## Priority 4: Accessibility (Higher effort, High value) + +### Enable Semantic Elements + +**Rule:** `useSemanticElements` +**Current:** off +**Estimated files:** ~15 + +**Why:** Using `role="button"` on `
` instead of `
- ); - } -} + + ); +}; DateRangePicker.propTypes = { updateState: PropTypes.func.isRequired, }; + +export default DateRangePicker; diff --git a/ui/intermittent-failures/Graph.jsx b/ui/intermittent-failures/Graph.jsx index 5838e432404..454d67bcf9a 100644 --- a/ui/intermittent-failures/Graph.jsx +++ b/ui/intermittent-failures/Graph.jsx @@ -1,9 +1,9 @@ -import React from 'react'; + import PropTypes from 'prop-types'; import { VictoryChart, VictoryLine, VictoryLegend } from 'victory'; import { Col } from 'react-bootstrap'; -const Graph = ({ graphData, title, legendData }) => ( +const Graph = ({ graphData = null, title = '', legendData = [] }) => ( { +const GraphAlternateView = ({ className, graphData, colNum = 2, title }) => { const columnsTwo = [ { Header: 'Date', @@ -77,12 +77,8 @@ const GraphAlternateView = ({ className, graphData, colNum, title }) => { ); }; -export default GraphAlternateView; - GraphAlternateView.propTypes = { colNum: PropTypes.number, }; -GraphAlternateView.defaultProps = { - colNum: 2, -}; +export default GraphAlternateView; diff --git a/ui/intermittent-failures/GraphsContainer.jsx b/ui/intermittent-failures/GraphsContainer.jsx index a9891aacb88..e5350dce0a2 100644 --- a/ui/intermittent-failures/GraphsContainer.jsx +++ b/ui/intermittent-failures/GraphsContainer.jsx @@ -25,7 +25,7 @@ export default class GraphsContainer extends React.Component { }; render() { - const { graphOneData, graphTwoData, children } = this.props; + const { graphOneData = null, graphTwoData = null, children } = this.props; const { showGraphTwo, showAlternateView } = this.state; return ( @@ -103,8 +103,3 @@ GraphsContainer.propTypes = { ), children: PropTypes.element.isRequired, }; - -GraphsContainer.defaultProps = { - graphOneData: null, - graphTwoData: null, -}; diff --git a/ui/intermittent-failures/Layout.jsx b/ui/intermittent-failures/Layout.jsx index f29848d38d0..f443a82055f 100644 --- a/ui/intermittent-failures/Layout.jsx +++ b/ui/intermittent-failures/Layout.jsx @@ -1,4 +1,4 @@ -import React from 'react'; + import { Container } from 'react-bootstrap'; import PropTypes from 'prop-types'; @@ -12,21 +12,21 @@ import GraphsContainer from './GraphsContainer'; const Layout = (props) => { const { - graphData, - tableData, + graphData = null, + tableData = null, errorMessages, - tree, - isFetchingTable, - isFetchingGraphs, - tableFailureStatus, - graphFailureStatus, + tree = null, + isFetchingTable = null, + isFetchingGraphs = null, + tableFailureStatus = null, + graphFailureStatus = null, updateState, updateHash, graphOneData, - graphTwoData, - table, + graphTwoData = null, + table = null, datePicker, - header, + header = null, } = props; let failureMessage = null; @@ -109,17 +109,4 @@ Layout.propTypes = { isFetchingGraphs: PropTypes.bool, }; -Layout.defaultProps = { - graphTwoData: null, - tableFailureStatus: null, - graphFailureStatus: null, - isFetchingTable: null, - isFetchingGraphs: null, - tableData: null, - graphData: null, - tree: null, - table: null, - header: null, -}; - export default Layout; diff --git a/ui/intermittent-failures/MainView.jsx b/ui/intermittent-failures/MainView.jsx index f176bf821d3..bddaeb52345 100644 --- a/ui/intermittent-failures/MainView.jsx +++ b/ui/intermittent-failures/MainView.jsx @@ -1,13 +1,13 @@ import React from 'react'; import { Row, Col, Breadcrumb, BreadcrumbItem } from 'react-bootstrap'; import PropTypes from 'prop-types'; -import moment from 'moment'; import ReactTable from 'react-table-6'; import Checkbox from '@mui/material/Checkbox'; import TextField from '@mui/material/TextField'; import Autocomplete from '@mui/material/Autocomplete'; import Popper from '@mui/material/Popper'; +import dayjs from '../helpers/dayjs'; import { bugsEndpoint, getBugUrl } from '../helpers/url'; import { setUrlParam, getUrlParam } from '../helpers/location'; @@ -36,15 +36,15 @@ const CustomPopper = (props) => { const MainView = (props) => { const { - graphData, - tableData, + graphData = [], + tableData = [], initialParamsSet, startday, endday, updateState, tree, location, - updateAppState, + updateAppState = null, } = props; const [selectedFilter, setSelectedFilter] = React.useState({ @@ -208,7 +208,7 @@ const MainView = (props) => { } = calculateMetrics(graphData)); } - const getHeaderAriaLabel = (state, bug, data) => { + const getHeaderAriaLabel = (_state, _bug, data) => { const ariaLabelValue = data.Header === 'Count' ? 'Filter not available for count' @@ -302,7 +302,7 @@ const MainView = (props) => { getTheadFilterThProps={getHeaderAriaLabel} getTrProps={(state, rowInfo) => { const baseProps = tableRowStyling(state, rowInfo); - if (rowInfo && rowInfo.original) { + if (rowInfo?.original) { const { id, summary } = rowInfo.original; const pathname = '/intermittent-failures/bugdetails'; const search = `?startday=${startday}&endday=${endday}&tree=${tree}&bug=${id}`; @@ -367,16 +367,10 @@ MainView.propTypes = { notify: PropTypes.func.isRequired, }; -MainView.defaultProps = { - graphData: [], - tableData: [], - updateAppState: null, -}; - const defaultState = { tree: 'all', - startday: ISODate(moment().utc().subtract(7, 'days')), - endday: ISODate(moment().utc()), + startday: ISODate(dayjs().utc().subtract(7, 'days')), + endday: ISODate(dayjs().utc()), endpoint: bugsEndpoint, route: '/main', }; diff --git a/ui/intermittent-failures/Navigation.jsx b/ui/intermittent-failures/Navigation.jsx index 0b48f11aee5..0dcda6a4e9a 100644 --- a/ui/intermittent-failures/Navigation.jsx +++ b/ui/intermittent-failures/Navigation.jsx @@ -20,7 +20,7 @@ export default class Navigation extends React.Component { }; render() { - const { updateState, tree, user, setUser, notify } = this.props; + const { updateState, tree = null, user, setUser, notify } = this.props; return ( (WrappedComponent) => { }; updateData = (params, urlChanged = false) => { - const { mainGraphData, mainTableData } = this.props; + const { mainGraphData = null, mainTableData = null } = this.props; if (mainGraphData && mainTableData && !urlChanged) { this.setState({ graphData: mainGraphData, tableData: mainTableData }); @@ -210,11 +210,6 @@ const withView = (defaultState) => (WrappedComponent) => { mainTableData: PropTypes.arrayOf(PropTypes.shape({})), }; - View.defaultProps = { - mainGraphData: null, - mainTableData: null, - }; - return View; }; diff --git a/ui/intermittent-failures/helpers.jsx b/ui/intermittent-failures/helpers.jsx index cbaf9233db1..50a2428fb81 100644 --- a/ui/intermittent-failures/helpers.jsx +++ b/ui/intermittent-failures/helpers.jsx @@ -1,8 +1,8 @@ -import moment from 'moment'; -import React from 'react'; + import TextField from '@mui/material/TextField'; import IconButton from '@mui/material/IconButton'; +import dayjs from '../helpers/dayjs'; import { setUrlParam } from '../helpers/location'; // be sure to wrap date arg in a moment() @@ -11,7 +11,7 @@ export const ISODate = function formatISODate(date) { }; export const prettyDate = function formatPrettyDate(date) { - return moment(date).format('ddd MMM D, YYYY'); + return dayjs(date).format('ddd MMM D, YYYY'); }; export const formatBugs = function formatBugsForBugzilla(data) { @@ -51,8 +51,8 @@ export const calculateMetrics = function calculateMetricsForGraphs(data) { const failures = data[i].failure_count; const testRuns = data[i].test_runs; const freq = testRuns < 1 || failures < 1 ? 0 : failures / testRuns; - const date = moment(data[i].date).format('MMM DD'); - const dateObj = moment(data[i].date).toDate(); + const date = dayjs(data[i].date).format('MMM DD'); + const dateObj = dayjs(data[i].date).toDate(); totalFailures += failures; totalRuns += testRuns; @@ -124,7 +124,7 @@ export const validateQueryParams = function validateQueryParams( return messages; }; -export const tableRowStyling = function tableRowStyling(state, bug) { +export const tableRowStyling = function tableRowStyling(_state, bug) { if (bug) { const style = { color: 'rgb(117, 117, 117)', @@ -191,7 +191,7 @@ export const textFilter = ({ filter, onChange, placeholder, columnId }) => ( paddingRight: '0px', }, endAdornment: - filter && filter.value ? ( + filter?.value ? ( { if (columnId) setUrlParam(columnId, ''); diff --git a/ui/job-view/App.jsx b/ui/job-view/App.jsx index e9ed89084e1..89604f39922 100644 --- a/ui/job-view/App.jsx +++ b/ui/job-view/App.jsx @@ -1,7 +1,6 @@ -import React from 'react'; +import React, { createRef } from 'react'; import { Modal } from 'react-bootstrap'; -import { hot } from 'react-hot-loader/root'; -import SplitPane from 'react-split-pane'; +import { PanelGroup, Panel, PanelResizeHandle } from 'react-resizable-panels'; import pick from 'lodash/pick'; import isEqual from 'lodash/isEqual'; import { connect } from 'react-redux'; @@ -78,6 +77,8 @@ class App extends React.Component { const hasSelectedJob = urlParams.has('selectedJob') || urlParams.has('selectedTaskRun'); + this.panelGroupRef = createRef(); + this.state = { repoName: this.getOrSetRepo(), revision: urlParams.get('revision'), @@ -100,7 +101,7 @@ class App extends React.Component { }; } - static getDerivedStateFromProps(props, state) { + static getDerivedStateFromProps(_props, state) { return { ...App.getSplitterDimensions(state.hasSelectedJob), }; @@ -189,14 +190,27 @@ class App extends React.Component { }, MAX_TRANSIENT_AGE); } - componentDidUpdate(prevProps) { + componentDidUpdate(prevProps, prevState) { if ( prevProps.router.location.search !== this.props.router.location.search ) { this.handleUrlChanges(); } + + // Use imperative API to update panel layout when hasSelectedJob changes + if (prevState.hasSelectedJob !== this.state.hasSelectedJob) { + this.updatePanelLayout(); + } } + updatePanelLayout = () => { + const { hasSelectedJob } = this.state; + if (this.panelGroupRef.current) { + const pushListPct = hasSelectedJob ? 100 - DEFAULT_DETAILS_PCT : 100; + this.panelGroupRef.current.setLayout([pushListPct, 100 - pushListPct]); + } + }; + componentWillUnmount() { window.removeEventListener('resize', this.updateDimensions, false); window.removeEventListener('storage', this.handleStorageEvent); @@ -375,9 +389,11 @@ class App extends React.Component { window.location.reload(true); } - handleSplitChange(latestSplitSize) { + handleSplitChange(sizes) { + // react-resizable-panels provides sizes as an array of percentages + // sizes[0] is the top panel (PushList) percentage this.setState({ - latestSplitPct: (latestSplitSize / getWindowHeight()) * 100, + latestSplitPct: sizes[0], }); } @@ -407,11 +423,8 @@ class App extends React.Component { frameworks, } = this.state; - // SplitPane will adjust the CSS height of the top component, but not the - // bottom component. So the scrollbars won't work in the DetailsPanel when - // we resize. Therefore, we must calculate the new - // height of the DetailsPanel based on the current height of the PushList. - // Reported this upstream: https://github.com/tomkp/react-split-pane/issues/282 + // react-resizable-panels handles height calculations automatically + // We still track the percentage for calculating the details panel height const pushListPct = latestSplitPct === undefined || !hasSelectedJob ? defaultPushListPct @@ -419,7 +432,7 @@ class App extends React.Component { const detailsHeight = latestSplitPct === undefined || !hasSelectedJob ? defaultDetailsHeight - : getWindowHeight() * (1 - latestSplitPct / 100); + : getWindowHeight() * (1 - pushListPct / 100); const filterBarFilters = Object.entries(filterModel.urlParams).reduce( (acc, [field, value]) => HIDDEN_URL_PARAMS.includes(field) || matchesDefaults(field, value) @@ -450,46 +463,49 @@ class App extends React.Component { setPushHealthVisibility={this.setPushHealthVisibility} {...this.props} /> - this.handleSplitChange(size)} + this.handleSplitChange(sizes)} > -
- {(isFieldFilterVisible || !!filterBarFilters.length) && ( - - )} - {serverChangedDelayed && ( - - )} - {currentRepo && ( -
- - - -
- )} -
- <> + +
+ {(isFieldFilterVisible || !!filterBarFilters.length) && ( + + )} + {serverChangedDelayed && ( + + )} + {currentRepo && ( +
+ + + +
+ )} +
+ + + {currentRepo && ( )} - - + + { @@ -302,10 +308,6 @@ CustomJobActions.propTypes = { currentRepo: PropTypes.shape({}).isRequired, }; -CustomJobActions.defaultProps = { - job: null, -}; - const mapStateToProps = ({ pushes: { decisionTaskMap } }) => ({ decisionTaskMap, }); diff --git a/ui/job-view/KeyboardShortcuts.jsx b/ui/job-view/KeyboardShortcuts.jsx index e13ccef9cc8..f4c1612d781 100644 --- a/ui/job-view/KeyboardShortcuts.jsx +++ b/ui/job-view/KeyboardShortcuts.jsx @@ -280,13 +280,9 @@ KeyboardShortcuts.propTypes = { selectedJob: PropTypes.shape({}), }; -KeyboardShortcuts.defaultProps = { - selectedJob: null, -}; - const mapStateToProps = ({ notifications: { notifications }, - selectedJob: { selectedJob }, + selectedJob: { selectedJob = null }, pinnedJobs: { pinnedJobs }, }) => ({ notifications, diff --git a/ui/job-view/Notifications.jsx b/ui/job-view/Notifications.jsx index 9d450bace79..de709d8b52a 100644 --- a/ui/job-view/Notifications.jsx +++ b/ui/job-view/Notifications.jsx @@ -1,4 +1,4 @@ -import React from 'react'; + import PropTypes from 'prop-types'; import { connect } from 'react-redux'; diff --git a/ui/job-view/details/DetailsPanel.jsx b/ui/job-view/details/DetailsPanel.jsx index 31383014ab0..7ecc15558dd 100644 --- a/ui/job-view/details/DetailsPanel.jsx +++ b/ui/job-view/details/DetailsPanel.jsx @@ -22,12 +22,17 @@ import TabsPanel from './tabs/TabsPanel'; export const pinboardHeight = 100; +// Debounce delay for loading job details when rapidly switching jobs +const JOB_DETAILS_DEBOUNCE_MS = 200; + class DetailsPanel extends React.Component { constructor(props) { super(props); // used to cancel all the ajax requests triggered by selectJob this.selectJobController = null; + // used to debounce job detail loading when rapidly switching jobs + this.selectJobDebounceTimer = null; this.state = { selectedJobFull: null, @@ -74,9 +79,11 @@ class DetailsPanel extends React.Component { prevResult !== result || prevFci !== fci ) { - this.selectJob(); + this.selectJobDebounced(); } } else if (selectedJob && selectedJob !== prevProps.selectedJob) { + // Initial job selection (from URL or first click) - load immediately without debounce + // This ensures the details panel appears promptly on page load this.selectJob(); } } @@ -86,6 +93,10 @@ class DetailsPanel extends React.Component { thEvents.classificationChanged, this.updateClassifications, ); + // Clean up debounce timer + if (this.selectJobDebounceTimer) { + clearTimeout(this.selectJobDebounceTimer); + } } togglePinBoardVisibility = () => { @@ -148,6 +159,27 @@ class DetailsPanel extends React.Component { return pushList.find((push) => pushId === push.id); }; + // Debounced version of selectJob to prevent loading details too rapidly + // when navigating quickly between jobs with keyboard shortcuts. + // The visual selection updates instantly, but details loading is debounced. + selectJobDebounced = () => { + // Cancel any pending debounce + if (this.selectJobDebounceTimer) { + clearTimeout(this.selectJobDebounceTimer); + } + + // Cancel any in-progress fetch requests immediately + if (this.selectJobController !== null) { + this.selectJobController.abort(); + this.selectJobController = null; + } + + this.selectJobDebounceTimer = setTimeout(() => { + this.selectJobDebounceTimer = null; + this.selectJob(); + }, JOB_DETAILS_DEBOUNCE_MS); + }; + selectJob = () => { const { currentRepo, selectedJob, frameworks } = this.props; const push = this.findPush(selectedJob.push_id); @@ -384,7 +416,7 @@ class DetailsPanel extends React.Component { resizedHeight, classificationMap, classificationTypes, - selectedJob, + selectedJob = null, } = this.props; const { selectedJobFull, @@ -482,10 +514,6 @@ DetailsPanel.propTypes = { selectedJob: PropTypes.shape({}), }; -DetailsPanel.defaultProps = { - selectedJob: null, -}; - const mapStateToProps = ({ selectedJob: { selectedJob }, pushes: { pushList }, diff --git a/ui/job-view/details/PinBoard.jsx b/ui/job-view/details/PinBoard.jsx index 5244a47dd18..ae43cbfb501 100644 --- a/ui/job-view/details/PinBoard.jsx +++ b/ui/job-view/details/PinBoard.jsx @@ -94,7 +94,7 @@ class PinBoard extends React.Component { }; createNewClassification = () => { - const { email } = this.props; + const { email = null } = this.props; const { failureClassificationId, failureClassificationComment, @@ -216,7 +216,8 @@ class PinBoard extends React.Component { }; unclassifyAllPinnedJobsTitle = () => { - if (!this.props.isStaff) { + const { isStaff = false } = this.props; + if (!isStaff) { return 'Must be employee or sheriff'; } @@ -228,9 +229,8 @@ class PinBoard extends React.Component { }; canUnclassifyAllPinnedJobs = () => { - return ( - this.props.isStaff && Object.values(this.props.pinnedJobs).length > 0 - ); + const { isStaff = false } = this.props; + return isStaff && Object.values(this.props.pinnedJobs).length > 0; }; unclassifyAllPinnedJobs = async () => { @@ -398,8 +398,8 @@ class PinBoard extends React.Component { render() { const { - selectedJobFull, - revisionTips, + selectedJobFull = null, + revisionTips = [], isLoggedIn, isPinBoardVisible, classificationTypes, @@ -498,7 +498,7 @@ class PinBoard extends React.Component { pattern="[0-9]*" className="add-related-bugs-input" placeholder="enter bug number" - invalid={!this.isValidBugNumber(newBugNumber)} + isInvalid={!this.isValidBugNumber(newBugNumber)} onKeyPress={this.bugNumberKeyPress} onChange={(ev) => { this.setState({ newBugNumber: ev.target.value }); @@ -530,7 +530,9 @@ class PinBoard extends React.Component { )} {Array.from(pinnedJobBugs).map((bug) => ( - + {!bug.id && ( @@ -716,7 +718,7 @@ PinBoard.propTypes = { jobMap: PropTypes.shape({}).isRequired, classificationTypes: PropTypes.arrayOf(PropTypes.shape({})).isRequired, isLoggedIn: PropTypes.bool.isRequired, - isStaff: PropTypes.bool.isRequired, + isStaff: PropTypes.bool, isPinBoardVisible: PropTypes.bool.isRequired, pinnedJobs: PropTypes.shape({}).isRequired, pinnedJobBugs: PropTypes.arrayOf(PropTypes.shape({})).isRequired, @@ -738,6 +740,7 @@ PinBoard.propTypes = { }; PinBoard.defaultProps = { + isStaff: false, selectedJobFull: null, email: null, revisionTips: [], diff --git a/ui/job-view/details/summary/ActionBar.jsx b/ui/job-view/details/summary/ActionBar.jsx index ea5db7b09fb..d9ccab8bbed 100644 --- a/ui/job-view/details/summary/ActionBar.jsx +++ b/ui/job-view/details/summary/ActionBar.jsx @@ -344,9 +344,9 @@ class ActionBar extends React.PureComponent { render() { const { selectedJobFull, - logViewerUrl, - logViewerFullUrl, - jobLogUrls, + logViewerUrl = null, + logViewerFullUrl = null, + jobLogUrls = [], pinJob, currentRepo, } = this.props; @@ -423,7 +423,7 @@ class ActionBar extends React.PureComponent { title="Scroll to selection" className="actionbar-nav-btn bg-transparent border-0" onClick={() => - findJobInstance(jobLogUrls[0] && jobLogUrls[0].job_id, true) + findJobInstance(jobLogUrls[0]?.job_id, true) } > ({ decisionTaskMap, }); diff --git a/ui/job-view/details/summary/LogItem.jsx b/ui/job-view/details/summary/LogItem.jsx index b5a04a132ad..ea49ade7baf 100644 --- a/ui/job-view/details/summary/LogItem.jsx +++ b/ui/job-view/details/summary/LogItem.jsx @@ -1,4 +1,4 @@ -import React from 'react'; + import PropTypes from 'prop-types'; import { Button, Dropdown } from 'react-bootstrap'; @@ -43,8 +43,8 @@ function getLogUrlProps(logKey, logUrl, logViewerUrl, logViewerFullUrl) { export default function LogItem(props) { const { logUrls, - logViewerUrl, - logViewerFullUrl, + logViewerUrl = null, + logViewerFullUrl = null, logKey, logDescription, } = props; @@ -114,8 +114,3 @@ LogItem.propTypes = { logViewerUrl: PropTypes.string, logViewerFullUrl: PropTypes.string, }; - -LogItem.defaultProps = { - logViewerUrl: null, - logViewerFullUrl: null, -}; diff --git a/ui/job-view/details/summary/LogUrls.jsx b/ui/job-view/details/summary/LogUrls.jsx index 48ee0632821..ccf974c91cc 100644 --- a/ui/job-view/details/summary/LogUrls.jsx +++ b/ui/job-view/details/summary/LogUrls.jsx @@ -8,7 +8,7 @@ import logviewerIcon from '../../../img/logviewerIcon.svg'; import LogItem from './LogItem'; export default function LogUrls(props) { - const { logUrls, logViewerUrl, logViewerFullUrl } = props; + const { logUrls, logViewerUrl = null, logViewerFullUrl = null } = props; const logUrlsUseful = logUrls.filter( (logUrl) => !logUrl.name.includes('perfherder-data'), ); @@ -49,8 +49,3 @@ LogUrls.propTypes = { logViewerUrl: PropTypes.string, logViewerFullUrl: PropTypes.string, }; - -LogUrls.defaultProps = { - logViewerUrl: null, - logViewerFullUrl: null, -}; diff --git a/ui/job-view/details/summary/StatusPanel.jsx b/ui/job-view/details/summary/StatusPanel.jsx index f4e0b138dd0..ae971c28b02 100644 --- a/ui/job-view/details/summary/StatusPanel.jsx +++ b/ui/job-view/details/summary/StatusPanel.jsx @@ -1,4 +1,4 @@ -import React from 'react'; + import PropTypes from 'prop-types'; function StatusPanel(props) { diff --git a/ui/job-view/details/summary/SummaryPanel.jsx b/ui/job-view/details/summary/SummaryPanel.jsx index 1b3a3db093d..fe5ad2008ef 100644 --- a/ui/job-view/details/summary/SummaryPanel.jsx +++ b/ui/job-view/details/summary/SummaryPanel.jsx @@ -13,14 +13,14 @@ class SummaryPanel extends React.PureComponent { render() { const { selectedJobFull, - latestClassification, + latestClassification = null, bugs, - jobLogUrls, - jobDetails, - jobDetailLoading, - logViewerUrl, - logViewerFullUrl, - logParseStatus, + jobLogUrls = [], + jobDetails = [], + jobDetailLoading = false, + logViewerUrl = null, + logViewerFullUrl = null, + logParseStatus = 'pending', user, currentRepo, classificationMap, @@ -129,14 +129,4 @@ SummaryPanel.propTypes = { logViewerFullUrl: PropTypes.string, }; -SummaryPanel.defaultProps = { - latestClassification: null, - jobLogUrls: [], - jobDetails: [], - jobDetailLoading: false, - logParseStatus: 'pending', - logViewerUrl: null, - logViewerFullUrl: null, -}; - export default SummaryPanel; diff --git a/ui/job-view/details/tabs/PerfData.jsx b/ui/job-view/details/tabs/PerfData.jsx index 3d5e4266ef7..2574462c687 100644 --- a/ui/job-view/details/tabs/PerfData.jsx +++ b/ui/job-view/details/tabs/PerfData.jsx @@ -6,7 +6,7 @@ import { notify } from '../../redux/stores/notifications'; class PerfData extends React.PureComponent { render() { - const { perfJobDetail, selectedJobFull } = this.props; + const { perfJobDetail = [], selectedJobFull } = this.props; const sortedDetails = perfJobDetail.slice(); @@ -101,10 +101,6 @@ PerfData.propTypes = { perfJobDetail: PropTypes.arrayOf(PropTypes.shape({})), }; -PerfData.defaultProps = { - perfJobDetail: [], -}; - const mapStateToProps = (state) => ({ decisionTaskMap: state.pushes.decisionTaskMap, }); diff --git a/ui/job-view/details/tabs/PerformanceTab.jsx b/ui/job-view/details/tabs/PerformanceTab.jsx index b427d269eb5..e722fbb9d1a 100644 --- a/ui/job-view/details/tabs/PerformanceTab.jsx +++ b/ui/job-view/details/tabs/PerformanceTab.jsx @@ -85,7 +85,7 @@ class PerformanceTab extends React.PureComponent { ); }; - getProfileRelevance = (jobDetail) => { + getProfileRelevance = (jobDetail = {}) => { const { url, value } = jobDetail; if (!url) { return NO_PROFILE_RELEVANCE; @@ -116,8 +116,9 @@ class PerformanceTab extends React.PureComponent { // Returns profile-related job details, ordered by the relevance. getProfiles = (perfTestOnly) => { + const { jobDetails = [] } = this.props; const profiles = []; - for (const jobDetail of this.props.jobDetails) { + for (const jobDetail of jobDetails) { const relevance = this.getProfileRelevance(jobDetail); if (relevance === NO_PROFILE_RELEVANCE) { continue; @@ -162,10 +163,10 @@ class PerformanceTab extends React.PureComponent { render() { const { repoName, - revision, + revision = '', selectedJobFull, - jobDetails, - perfJobDetail, + jobDetails = [], + perfJobDetail = [], } = this.props; const { triggeredGeckoProfiles, showSideBySide } = this.state; @@ -285,12 +286,6 @@ PerformanceTab.propTypes = { decisionTaskMap: PropTypes.shape({}).isRequired, }; -PerformanceTab.defaultProps = { - jobDetails: [], - perfJobDetail: [], - revision: '', -}; - const mapStateToProps = (state) => ({ decisionTaskMap: state.pushes.decisionTaskMap, }); diff --git a/ui/job-view/details/tabs/SideBySide.jsx b/ui/job-view/details/tabs/SideBySide.jsx index b6217f35615..8ae2430f3c3 100644 --- a/ui/job-view/details/tabs/SideBySide.jsx +++ b/ui/job-view/details/tabs/SideBySide.jsx @@ -32,7 +32,7 @@ class SideBySide extends React.PureComponent { } getSideBySideParams() { - const { jobDetails } = this.props; + const { jobDetails = [] } = this.props; this.setState( { @@ -63,7 +63,7 @@ class SideBySide extends React.PureComponent { } render() { - const { jobDetails } = this.props; + const { jobDetails = [] } = this.props; const { sideBySideLoading, sideBySideParams } = this.state; if (!sideBySideParams) { @@ -189,10 +189,6 @@ SideBySide.propTypes = { jobDetails: PropTypes.arrayOf(PropTypes.shape({})), }; -SideBySide.defaultProps = { - jobDetails: [], -}; - const mapStateToProps = (state) => ({ decisionTaskMap: state.pushes.decisionTaskMap, }); diff --git a/ui/job-view/details/tabs/TabsPanel.jsx b/ui/job-view/details/tabs/TabsPanel.jsx index 22188175d3f..32cb2be1854 100644 --- a/ui/job-view/details/tabs/TabsPanel.jsx +++ b/ui/job-view/details/tabs/TabsPanel.jsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect, useRef, useCallback } from 'react'; +import { useState, useEffect, useRef, useCallback } from 'react'; import PropTypes from 'prop-types'; import { useSelector, useDispatch } from 'react-redux'; import { Button, Dropdown, Nav } from 'react-bootstrap'; @@ -244,7 +244,7 @@ const TabsPanel = ({ useEffect(() => { const timeoutId = setTimeout(() => checkTabOverflow(), 100); return () => clearTimeout(timeoutId); - }, [perfJobDetail?.length, checkTabOverflow]); + }, [checkTabOverflow]); const countPinnedJobs = Object.keys(pinnedJobs).length; const { showPerf } = showTabsFromProps({ perfJobDetail }); @@ -394,7 +394,7 @@ const TabsPanel = ({ Superseded only - - - My pushes only - + + My pushes only - - - Hide code review pushes - + + Hide code review pushes repo.repository_group.name) => ({ + (acc, repo, _idx, _arr, group = (repo) => repo.repository_group.name) => ({ ...acc, [group(repo)]: [...(acc[group(repo)] || []), repo], }), @@ -58,11 +58,11 @@ export default function ReposMenu(props) {