Skip to content

Commit

Permalink
refactor(deps): migrate from deprecated tslint to eslint
Browse files Browse the repository at this point in the history
- `tslint` had been officially deprecated in favor of `eslint` with `@typescript-eslint` since at least [early 2019](https://blog.palantir.com/tslint-in-2019-1a144c2317a9)
  - it's been completely [archived](https://github.com/palantir/tslint) since at least mid-2021 as well

- modify static analysis docs to mention `eslint` instead of `tslint`
  - also update note about Snyk to include dependency scans and mention that it's used as an SCA (Software Composition Analysis)

- migrate `lint` script to use `eslint`
- migrate all `tslint` deps, including `plugin-react` and `plugin-prettier`
  - **NOTE**: we have an outdated version of `prettier` (v1.x, while v3.x is latest), so equivalently had to use an outdated version of `eslint-plugin-prettier`
    - I imagine this might have been outdated because there was no newer `tslint-plugin-prettier` that supported newer `prettier` versions
      - i.e. this very `eslint` migration was a pre-requisite to upgrade `prettier`
- migrate `tslint.json` to `.eslintrc.json`
  - more or less the same configuration with same plugins
  - use recommended rulesets
  - globally disable `@typescript-eslint/no-explicit-any` and `@typescript-eslint/no-non-null-assertion`
    - these are commonly disabled as both of these are already _explicit_ typings (as opposed to implicit ones, which we should check for)
      - there are valid uses of these two, and while they can be misused, that's somewhat uncommon (in my experience)
    - consistent with `argo-ui`'s configuration

- auto-fix `<a target='_blank'>` elements that were missing a `rel='noreferrer'`, per [`react/jsx-no-target-blank` rule](https://github.com/jsx-eslint/eslint-plugin-react/blob/v7.33.2/docs/rules/jsx-no-target-blank.md)
  - this is a security vuln per [the docs](https://github.com/jsx-eslint/eslint-plugin-react/blob/ca30f77196d410c7cdb1e4fe11f11bfffb46c84f/docs/rules/jsx-no-target-blank.md?plain=1#L9)
    - see also https://mathiasbynens.github.io/rel-noopener and https://html.spec.whatwg.org/multipage/links.html#link-type-noopener
      - browsers have implicitly added `rel='noreferrer'` since at least 2021 as well per the above spec change; this (recommended) rule makes it explicitly added

- auto-fix some unnecessary `!!`, per [`no-extra-boolean-cast` rule](https://eslint.org/docs/latest/rules/no-extra-boolean-cast)

- auto-format `webpack.config.js` as previously `prettier` wasn't run on it (not a TS file)
  - manually add some ESLint comments so that it works with Node & CJS
    - may want to migrate to `webpack.config.mjs` in the future for ESM
      - or [`webpack.config.ts`](https://webpack.js.org/configuration/configuration-languages/#typescript)

- manually fix several unused variables, per [`@typescript-eslint/no-unused-vars` rule](https://typescript-eslint.io/rules/no-unused-vars)
  - remove unused `_`, `e`, `x`, `i`, etc
  - remove unused `onError` prop
  - actually use the previously unused `T` generic type in `object-parser`

- manually fix unnecessary `<T extends any>`, per [`@typescript-eslint/no-unnecessary-type-constraint` rule](https://typescript-eslint.io/rules/no-unnecessary-type-constraint)
  - just use regular `<T>`, which is the same
  - editor had some trouble understanding the syntax, so while doing so, also converted several consts assigned to anonymous functions to named functions
    - and then had a few more inner changes to named functions while at it too
      - and one promise chain -> async/await refactor while at that as well

- manually fix two missing `key`s in React iterators, per [`react/jsx-key` rule](https://github.com/jsx-eslint/eslint-plugin-react/blob/v7.33.2/docs/rules/jsx-key.md)
  - React warns when these are missing at run-time as well, see [React docs on `key`](https://react.dev/learn/rendering-lists#keeping-list-items-in-order-with-key)

- manually replace several single and double quotes inside of JSX with HTML escaped chars instead, per  [`react/no-unescaped-entities` rule](https://github.com/jsx-eslint/eslint-plugin-react/blob/v7.33.2/docs/rules/no-unescaped-entities.md)
  - use `&apos;` and `&quot;` consistently for these

- manually fix a few `require` statements, per [`@typescript-eslint/no-var-requires` rule](https://typescript-eslint.io/rules/no-var-requires/)
  - disable in `webpack.config.js` per above
  - disable in `index.tsx` as I'm not sure if `react-hot-loader` can support async imports
    - `react-hot-loader` is also [deprecated in favor of React Fast Refresh](https://github.com/gaearon/react-hot-loader#moving-towards-next-step) and hasn't received an update in at least a year
      - so eventually we should replace it anyway. rather than trying to get it to work with async imports, just add a disable line
  - remove and uninstall `superagent-promise` as it's not needed nowadays
    - `superagent-promise` hasn't gotten an update in at least 5 years and `superagent` has long natively supported promises
      - heck the types used were from `superagent` itself also
  - convert a few `require('*.scss')` side-effects into `import '*.scss'`
    - `react-datepicker` already had a `.css` `import` as well
    - consistently use ESM syntax where possible
    - also, re-group some imports while at it
      - consistently have external imports -> internal imports -> internal side-effects (e.g. CSS)

- manually remove unnecessary `\` escape chars, per [`no-useless-escape` rule](https://eslint.org/docs/latest/rules/no-useless-escape)
- manually remove an unnecessary `// @ts-ignore` comment, per [`@typescript-eslint/ban-ts-comment` rule](https://typescript-eslint.io/rules/ban-ts-comment)
  - it should be replaced with a `// @ts-expect-error` instead, but this seemed to be unnecessary as there was no error at this time (there may have been in the past?)
- manually remove unused `qeId` prop, per [`react/no-unknown-property` rule](https://github.com/jsx-eslint/eslint-plugin-react/blob/v7.33.2/docs/rules/no-unknown-property.md)
  - this seems to have been for testing purposes, but it is unused and also outdated
    - current testing paradigms tend to use valid HTML [`data-*` attributes](https://developer.mozilla.org/en-US/docs/Learn/HTML/Howto/Use_data_attributes) nowadays, such as Testing Library's [`data-testid`](https://testing-library.com/docs/queries/bytestid)

- manually remove a few of the `tslint:disable` comments
  - `no-console`, `no-bitwise`, and `prefer-for-of`
    - `no-console` and `no-bitwise` are not part of ESLint's [recommended ruleset](https://eslint.org/docs/latest/rules/) at this time, so no need replace with `eslint-disable` comments
    - `prefer-for-of` is [part of](https://github.com/typescript-eslint/typescript-eslint/blob/75c128856b1ce05a4fec799bfa6de03b3dab03d0/packages/eslint-plugin/src/configs/stylistic.ts#L24) the separated `stylistic` package of `typescript-eslint`, which we do not currently use

- also convert a few consts assigned to anonymous functions to named functions for better tracing while refactoring related code

Signed-off-by: Anton Gilgur <[email protected]>
  • Loading branch information
Anton Gilgur committed Nov 25, 2023
1 parent f028ac2 commit e9a4a0e
Show file tree
Hide file tree
Showing 65 changed files with 1,506 additions and 391 deletions.
4 changes: 2 additions & 2 deletions docs/static-code-analysis.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

We use the following static code analysis tools:

* `golangci-lint` and `tslint` for compile time linting.
* [Snyk](https://app.snyk.io/org/argoproj/projects) for image scanning.
* `golangci-lint` and `eslint` for compile time linting.
* [Snyk](https://app.snyk.io/org/argoproj/projects) for dependency and image scanning (SCA).

These are at least run daily or on each pull request.
27 changes: 27 additions & 0 deletions ui/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"extends": [
"eslint:recommended",
"plugin:@typescript-eslint/recommended",
"plugin:react/recommended",
"plugin:prettier/recommended"
],
"parser": "@typescript-eslint/parser",
"parserOptions": {
"ecmaVersion": "latest",
"sourceType": "module"
},
"plugins": [
"@typescript-eslint",
"react",
"prettier"
],
"rules": {
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/no-non-null-assertion": "off"
},
"settings": {
"react": {
"version": "detect"
}
}
}
13 changes: 7 additions & 6 deletions ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"scripts": {
"build": "rm -Rf dist && NODE_OPTIONS='--openssl-legacy-provider' NODE_ENV=production webpack --mode production --config ./src/app/webpack.config.js",
"start": "NODE_OPTIONS='--no-experimental-fetch --openssl-legacy-provider' webpack-dev-server --config ./src/app/webpack.config.js",
"lint": "tslint --fix -p ./src/app",
"lint": "eslint --fix ./src/app",
"test": "jest",
"deduplicate": "yarn-deduplicate -s fewer yarn.lock"
},
Expand All @@ -35,7 +35,6 @@
"react-monaco-editor": "^0.50.1",
"react-router-dom": "^4.2.2",
"superagent": "^8.1.2",
"superagent-promise": "^1.1.0",
"swagger-ui-react": "^4.19.1"
},
"devDependencies": {
Expand All @@ -59,9 +58,15 @@
"@types/superagent": "^4.1.22",
"@types/swagger-ui-react": "^4.11.0",
"@types/uuid": "^9.0.7",
"@typescript-eslint/eslint-plugin": "^6.10.0",
"@typescript-eslint/parser": "^6.10.0",
"babel-jest": "^29.6.4",
"babel-loader": "^9.1.3",
"copy-webpack-plugin": "^11.0.0",
"eslint": "^8.53.0",
"eslint-config-prettier": "^9.0.0",
"eslint-plugin-prettier": "^3.4.1",
"eslint-plugin-react": "^7.33.2",
"html-webpack-plugin": "^5.5.3",
"jest": "^26.6.3",
"monaco-editor-webpack-plugin": "^7.1.0",
Expand All @@ -75,10 +80,6 @@
"ts-jest": "^26.4.4",
"ts-loader": "^9.5.1",
"ts-node": "^9.1.1",
"tslint": "^5.20.1",
"tslint-config-prettier": "^1.18.0",
"tslint-plugin-prettier": "^2.3.0",
"tslint-react": "^3.4.0",
"typescript": "^4.6.4",
"webpack": "^5.89.0",
"webpack-cli": "^5.1.4",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {Utils} from '../../../shared/utils';
import {SubmitWorkflowPanel} from '../../../workflows/components/submit-workflow-panel';
import {ClusterWorkflowTemplateEditor} from '../cluster-workflow-template-editor';

require('../../../workflows/components/workflow-details/workflow-details.scss');
import '../../../workflows/components/workflow-details/workflow-details.scss';

export function ClusterWorkflowTemplateDetails({history, location, match}: RouteComponentProps<any>) {
// boiler-plate
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import {useQueryParams} from '../../../shared/use-query-params';
import {Footnote} from '../../../shared/footnote';
import {services} from '../../../shared/services';
import {ClusterWorkflowTemplateCreator} from '../cluster-workflow-template-creator';
require('./cluster-workflow-template-list.scss');

import './cluster-workflow-template-list.scss';

export function ClusterWorkflowTemplateList({history, location}: RouteComponentProps<any>) {
const {navigation} = useContext(Context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import {WidgetGallery} from '../../../widgets/widget-gallery';
import {WorkflowsRow} from '../../../workflows/components/workflows-row/workflows-row';
import {CronWorkflowEditor} from '../cron-workflow-editor';

require('../../../workflows/components/workflow-details/workflow-details.scss');
require('./cron-workflow-details.scss');
import '../../../workflows/components/workflow-details/workflow-details.scss';
import './cron-workflow-details.scss';

export function CronWorkflowDetails({match, location, history}: RouteComponentProps<any>) {
// boiler-plate
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {CheckboxFilter} from '../../../shared/components/checkbox-filter/checkbo
import {NamespaceFilter} from '../../../shared/components/namespace-filter';
import {TagsInput} from '../../../shared/components/tags-input/tags-input';

require('./cron-workflow-filters.scss');
import './cron-workflow-filters.scss';

interface WorkflowFilterProps {
cronWorkflows: models.WorkflowTemplate[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {CronWorkflowCreator} from '../cron-workflow-creator';
import {CronWorkflowFilters} from '../cron-workflow-filters/cron-workflow-filters';
import {PrettySchedule} from '../pretty-schedule';

require('./cron-workflow-list.scss');
import './cron-workflow-list.scss';

const learnMore = <a href='https://argoproj.github.io/argo-workflows/cron-workflows/'>Learn more</a>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ function status(r: {status?: {conditions?: Condition[]}}) {
if (!r.status || !r.status.conditions) {
return '';
}
if (!!r.status.conditions.find(c => c.status === 'False')) {
if (r.status.conditions.find(c => c.status === 'False')) {
return 'Failed';
}
return !!r.status.conditions.find(c => c.status !== 'True') ? 'Pending' : 'Ready';
return r.status.conditions.find(c => c.status !== 'True') ? 'Pending' : 'Ready';
}

const numNodesToHide = 2;
export function buildGraph(eventSources: EventSource[], sensors: Sensor[], workflows: Workflow[], flow: {[p: string]: {count: number; timeout?: any}}, expanded: boolean) {
const edgeLabel = (id: Node, label?: string) => (!!flow[id] ? (label || '') + ' (' + flow[id].count + ')' : label);
const edgeLabel = (id: Node, label?: string) => (flow[id] ? (label || '') + ' (' + flow[id].count + ')' : label);
const edgeClassNames = (id: Node) => (!!flow[id] && flow[id].timeout ? 'flow' : '');
const graph = new Graph();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {buildGraph} from './build-graph';
import {genres} from './genres';
import {ID} from './id';

require('./event-flow-page.scss');
import './event-flow-page.scss';

export function EventFlowPage({history, location, match}: RouteComponentProps<any>) {
// boiler-plate
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {ObjectEditor} from '../../shared/components/object-editor/object-editor'

export function EventSourceEditor({
onChange,
onError,
onTabSelected,
selectedTabKey,
eventSource
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ export function EventSourceList({match, location, history}: RouteComponentProps<
</div>
<div className='columns small-2'>
<div
onClick={e => {
onClick={() => {
setSelectedNode(`${es.metadata.namespace}/event-sources/${es.metadata.name}`);
}}>
<i className='fa fa-bars' />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ export function EventSourceLogsViewer({
return () => subscription.unsubscribe();
}, [namespace, eventSource, selectedEvent]);

// @ts-ignore
return (
<div>
<div className='row'>
Expand All @@ -68,9 +67,9 @@ export function EventSourceLogsViewer({
</div>
{!!eventSource &&
Object.entries(eventSource.spec).map(([type, value]) => (
<div>
<div key={type}>
<span title={type}>&nbsp;{type}</span>
{Object.entries(value).map(([name, eventValue]) => (
{Object.entries(value).map(([name]) => (
<div
key={`${type}-${name}`}
onClick={() => {
Expand Down
12 changes: 7 additions & 5 deletions ui/src/app/help/components/help.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import {Page} from 'argo-ui';
import * as React from 'react';

import {uiUrl} from '../../shared/base';
import {useCollectEvent} from '../../shared/components/use-collect-event';
require('./help.scss');

import './help.scss';

export function Help() {
useCollectEvent('openedHelp');
Expand All @@ -13,10 +15,10 @@ export function Help() {
<div className='help-box'>
<div className='help-box__ico help-box__ico--manual' />
<h3>Documentation</h3>
<a href='https://argoproj.github.io/argo-workflows' target='_blank' className='help-box__link'>
<a href='https://argoproj.github.io/argo-workflows' target='_blank' className='help-box__link' rel='noreferrer'>
Online Help
</a>
<a className='help-box__link' target='_blank' href={uiUrl('apidocs')}>
<a className='help-box__link' target='_blank' href={uiUrl('apidocs')} rel='noreferrer'>
API Docs
</a>
</div>
Expand All @@ -25,7 +27,7 @@ export function Help() {
<div className='help-box'>
<div className='help-box__ico help-box__ico--email' />
<h3>Contact</h3>
<a className='help-box__link' target='_blank' href='https://argoproj.github.io/community/join-slack/'>
<a className='help-box__link' target='_blank' href='https://argoproj.github.io/community/join-slack/' rel='noreferrer'>
Slack
</a>
</div>
Expand All @@ -34,7 +36,7 @@ export function Help() {
<div className='help-box'>
<div className='help-box__ico help-box__ico--download' />
<h3>Argo CLI</h3>
<a className='help-box__link' target='_blank' href='https://github.com/argoproj/argo-workflows/releases/latest'>
<a className='help-box__link' target='_blank' href='https://github.com/argoproj/argo-workflows/releases/latest' rel='noreferrer'>
Releases
</a>
</div>
Expand Down
2 changes: 1 addition & 1 deletion ui/src/app/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ ReactDOM.render(<App />, document.getElementById('app'));
const mdl = module as any;
if (mdl.hot) {
mdl.hot.accept('./app.tsx', () => {
const UpdatedApp = require('./app.tsx').App;
const UpdatedApp = require('./app.tsx').App; // eslint-disable-line @typescript-eslint/no-var-requires
ReactDOM.render(<UpdatedApp />, document.getElementById('app'));
});
}
3 changes: 2 additions & 1 deletion ui/src/app/login/components/login.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import {Page} from 'argo-ui';
import * as React from 'react';

import {uiUrl, uiUrlWithParams} from '../../shared/base';
import {useCollectEvent} from '../../shared/components/use-collect-event';

require('./login.scss');
import './login.scss';

function logout() {
document.cookie = 'authorization=;Max-Age=0';
Expand Down
6 changes: 3 additions & 3 deletions ui/src/app/modals/feedback/feedback-modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import {BigButton} from '../../shared/components/big-button';
import {Modal} from '../../shared/components/modal/modal';
import {SurveyButton} from '../../shared/components/survey-button';

export const FeedbackModal = ({dismiss}: {dismiss: () => void}) => {
export function FeedbackModal({dismiss}: {dismiss: () => void}) {
const [choice, setChoice] = useState(0);
return (
<Modal dismiss={dismiss}>
<h3 style={{textAlign: 'center'}}>How's it going so far?</h3>
<h3 style={{textAlign: 'center'}}>How&apos;s it going so far?</h3>
<div style={{textAlign: 'center'}}>
<BigButton icon='smile-beam' title='Great' onClick={() => setChoice(1)} />
<BigButton icon='frown-open' title='Not so good' href='#' onClick={() => setChoice(2)} />
Expand All @@ -20,4 +20,4 @@ export const FeedbackModal = ({dismiss}: {dismiss: () => void}) => {
)}
</Modal>
);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {SurveyButton} from '../../shared/components/survey-button';
*/
export const FirstTimeUserModal = ({dismiss}: {dismiss: () => void}) => (
<Modal dismiss={dismiss}>
<h3 style={{textAlign: 'center'}}>Tell us what you want to use Argo for - we'll tell you how to do it. </h3>
<h3 style={{textAlign: 'center'}}>Tell us what you want to use Argo for - we&apos;ll tell you how to do it. </h3>
<div style={{textAlign: 'center'}}>
{[
{key: 'machine-learning', icon: 'brain', title: 'Machine Learning'},
Expand Down
Loading

0 comments on commit e9a4a0e

Please sign in to comment.