Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update TypeScript and ESLint to latest versions #1256

Merged
merged 1 commit into from
Mar 12, 2023
Merged

Update TypeScript and ESLint to latest versions #1256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ module.exports = {
},
plugins: ['@typescript-eslint'],
rules: {
'no-unused-vars': 0,
'@typescript-eslint/naming-convention': [
'error',
{
Expand All @@ -57,7 +56,16 @@ module.exports = {
prefix: ['I'],
},
],

// Disable ESLint core rules for which @typescript-eslint provides TypeScript-specific equivalents.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, what's the harm in keeping them? if TS flags them first, then fixing the issue would also fix ESLint issue, would it not? (basically, I prefer fewer deviations from defaults if possible)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that these can cause false positives with the new version of @typescript-eslint. E.g. removing the suppression for core's no-shadow rules results in a plethora of bogus reports about TS type definitions. The recommendation of silencing the corresponding core rules comes from the plugin itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way to fix it could be to make the root eslintrc also extend the recommended config from @typescript-eslint, because that config already does this toggling for us. This is what plexus does already. I'm thinking this could be done in a followup PR as it might cause some new errors.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 -- #1258

'no-unused-vars': 0,
'@typescript-eslint/no-unused-vars': 1,
'no-use-before-define': 0,
'@typescript-eslint/no-use-before-define': 1,
'no-redeclare': 0,
'@typescript-eslint/no-redeclare': 1,
'no-shadow': 0,
'@typescript-eslint/no-shadow': 1,

// Disable prop type checks for TSX components, as prop type validation is expected
// to be handled by TypeScript there. Stray prop types in components converted from Flow
Expand All @@ -79,6 +87,7 @@ module.exports = {
/* general */
'arrow-body-style': 0,
'arrow-parens': [1, 'as-needed'],
'class-methods-use-this': 0,
'comma-dangle': 0,
'lines-between-class-members': ['error', 'always', { exceptAfterSingleLine: true }],
'no-continue': 0,
Expand Down
8 changes: 4 additions & 4 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
},
"devDependencies": {
"@babel/eslint-parser": "^7.19.1",
"@typescript-eslint/eslint-plugin": "4.0.0",
"@typescript-eslint/parser": "3.10.1",
"eslint": "7.32.0",
"@typescript-eslint/eslint-plugin": "5.54.1",
"@typescript-eslint/parser": "5.54.1",
"eslint": "8.36.0",
"eslint-config-airbnb": "19.0.4",
"eslint-config-prettier": "8.7.0",
"eslint-plugin-import": "2.27.5",
Expand All @@ -24,7 +24,7 @@
"npm-run-all": "4.1.5",
"prettier": "2.8.4",
"rxjs-compat": "6.6.7",
"typescript": "3.8.3"
"typescript": "4.9.5"
},
"resolutions": {
"**/lodash": "4.17.21"
Expand Down
1 change: 0 additions & 1 deletion packages/jaeger-ui/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@
"tar": "6.1.13",
"ts-key-enum": "^2.0.0",
"tween-functions": "^1.2.0",
"typescript": "3.8.3",
"u-basscss": "2.0.1"
},
"scripts": {
Expand Down
1 change: 1 addition & 0 deletions packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ export function getNodeRenderer({
return function renderNode(vertex: TDdgVertex, _: unknown, lv: TLayoutVertex<any> | null) {
const { isFocalNode, key, operation, service } = vertex;
return (
// eslint-disable-next-line @typescript-eslint/no-use-before-define
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's good to have a comment explaining the reason (I assume it's a false positive)

<DdgNodeContent
focalNodeUrl={isFocalNode ? null : getUrl({ density, operation, service, ...extraUrlArgs }, baseUrl)}
focusPathsThroughVertex={focusPathsThroughVertex}
Expand Down
2 changes: 1 addition & 1 deletion packages/jaeger-ui/src/components/Monitor/ServicesView/serviceGraph.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export class ServiceGraphImpl extends React.PureComponent<TProps> {
return [];
}

generatePlaceholder(placeHolder: string | JSX.Element) {
generatePlaceholder(placeHolder: string | React.ReactNode) {
const { width } = this.props;

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/jaeger-ui/src/components/common/DetailsCard/types.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import type React from 'react';
import { TExample } from '../ExamplesLink';

export type TStyledValue = {
Expand Down
176 changes: 88 additions & 88 deletions packages/jaeger-ui/src/constants/default-config.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,103 +18,103 @@ import { FALLBACK_DAG_MAX_NUM_SERVICES } from './index';
import getVersion from '../utils/version/get-version';

import { version } from '../../package.json';
import { Config } from '../types/config';

export default deepFreeze(
Object.defineProperty(
const defaultConfig: Config = {
archiveEnabled: false,
dependencies: {
dagMaxNumServices: FALLBACK_DAG_MAX_NUM_SERVICES,
menuEnabled: true,
},
menu: [
{
archiveEnabled: false,
dependencies: {
dagMaxNumServices: FALLBACK_DAG_MAX_NUM_SERVICES,
menuEnabled: true,
},
menu: [
label: 'About Jaeger',
items: [
{
label: 'About Jaeger',
items: [
{
label: 'Website/Docs',
url: 'https://www.jaegertracing.io/',
},
{
label: 'Blog',
url: 'https://medium.com/jaegertracing/',
},
{
label: 'Twitter',
url: 'https://twitter.com/JaegerTracing',
},
{
label: 'Discussion Group',
url: 'https://groups.google.com/forum/#!forum/jaeger-tracing',
},
{
label: 'Online Chat',
url: 'https://cloud-native.slack.com/archives/CGG7NFUJ3',
},
{
label: 'GitHub',
url: '/~https://github.com/jaegertracing/',
},
{
label: `Jaeger ${getVersion().gitVersion}`,
},
{
label: `Commit ${getVersion().gitCommit.substring(0, 7)}`,
},
{
label: `Build ${getVersion().buildDate}`,
},
{
label: `Jaeger UI v${version}`,
},
],
label: 'Website/Docs',
url: 'https://www.jaegertracing.io/',
},
],
search: {
maxLookback: {
label: '2 Days',
value: '2d',
{
label: 'Blog',
url: 'https://medium.com/jaegertracing/',
},
maxLimit: 1500,
},
tracking: {
gaID: null,
trackErrors: true,
customWebAnalytics: null,
},
linkPatterns: [],
monitor: {
menuEnabled: true,
emptyState: {
mainTitle: 'Get started with Service Performance Monitoring',
subTitle:
'A high-level monitoring dashboard that helps you cut down the time to identify and resolve anomalies and issues.',
description:
'Service Performance Monitoring aggregates tracing data into RED metrics and visualizes them in service and operation level dashboards.',
button: {
text: 'Read the Documentation',
onClick: () => window.open('https://www.jaegertracing.io/docs/latest/spm/'),
},
alert: {
message: 'Service Performance Monitoring requires a Prometheus-compatible time series database.',
type: 'info',
},
{
label: 'Twitter',
url: 'https://twitter.com/JaegerTracing',
},
docsLink: 'https://www.jaegertracing.io/docs/latest/spm/',
},
deepDependencies: {
menuEnabled: false,
{
label: 'Discussion Group',
url: 'https://groups.google.com/forum/#!forum/jaeger-tracing',
},
{
label: 'Online Chat',
url: 'https://cloud-native.slack.com/archives/CGG7NFUJ3',
},
{
label: 'GitHub',
url: '/~https://github.com/jaegertracing/',
},
{
label: `Jaeger ${getVersion().gitVersion}`,
},
{
label: `Commit ${getVersion().gitCommit.substring(0, 7)}`,
},
{
label: `Build ${getVersion().buildDate}`,
},
{
label: `Jaeger UI v${version}`,
},
],
},
],
search: {
maxLookback: {
label: '2 Days',
value: '2d',
},
maxLimit: 1500,
},
tracking: {
gaID: null,
trackErrors: true,
customWebAnalytics: null,
},
linkPatterns: [],
monitor: {
menuEnabled: true,
emptyState: {
mainTitle: 'Get started with Service Performance Monitoring',
subTitle:
'A high-level monitoring dashboard that helps you cut down the time to identify and resolve anomalies and issues.',
description:
'Service Performance Monitoring aggregates tracing data into RED metrics and visualizes them in service and operation level dashboards.',
button: {
text: 'Read the Documentation',
onClick: () => window.open('https://www.jaegertracing.io/docs/latest/spm/'),
},
qualityMetrics: {
menuEnabled: false,
menuLabel: 'Trace Quality',
alert: {
message: 'Service Performance Monitoring requires a Prometheus-compatible time series database.',
type: 'info',
},
},
// fields that should be individually merged vs wholesale replaced
'__mergeFields',
{ value: ['dependencies', 'search', 'tracking'] }
)
);
docsLink: 'https://www.jaegertracing.io/docs/latest/spm/',
},
deepDependencies: {
menuEnabled: false,
},
qualityMetrics: {
menuEnabled: false,
menuLabel: 'Trace Quality',
},
};

// Fields that should be merged with user-supplied config values rather than overwritten.
type TMergeField = 'dependencies' | 'search' | 'tracking';
export const mergeFields: readonly TMergeField[] = ['dependencies', 'search', 'tracking'];

export default deepFreeze(defaultConfig);

export const deprecations = [
{
Expand Down
18 changes: 9 additions & 9 deletions packages/jaeger-ui/src/types/config.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ import { TNil } from '.';

export type ConfigMenuItem = {
label: string;
url: string;
url?: string;
anchorTarget?: '_self' | '_blank' | '_parent' | '_top';
};

export type ConfigMenuGroup = {
label: string;
items: ConfigMenuItem[];
items: readonly ConfigMenuItem[];
};

export type TScript = {
Expand Down Expand Up @@ -84,15 +84,15 @@ export type Config = {
// menuEnabled enables or disables the System Architecture tab.
menuEnabled?: boolean;

// dagMaxServicesLen defines the maximum number of services allowed
// dagMaxNumServices defines the maximum number of services allowed
// before the DAG dependency view is disabled. Too many services
// cause the DAG view to be non-responsive.
dagMaxServicesLen?: number;
dagMaxNumServices?: number;
};

// menu controls the dropdown menu in the top-right corner of the UI.
// When populated, this element completely overrides the default menu.
menu: (ConfigMenuGroup | ConfigMenuItem)[];
menu: readonly (ConfigMenuGroup | ConfigMenuItem)[];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious why these specific entries need readonly and not all others.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems it's specifically for array type entries. TS is apparently now better at detecting that the object passed to deepFreeze is recursively readonly.


// search section controls some aspects of the Search panel.
search?: {
Expand All @@ -115,14 +115,14 @@ export type Config = {

// scripts is an array of URLs of additional JavaScript files to be loaded.
// TODO when is it useful?
scripts?: TScript[];
scripts?: readonly TScript[];

// topTagPrefixes defines a set of prefixes for span tag names that are considered
// "important" and cause the matching tags to appear higher in the list of tags.
// For example, topTagPrefixes=['http.'] would cause all span tags that begin with
// "http." to be shown above all other tags.
// See /~https://github.com/jaegertracing/jaeger-ui/issues/218 for background.
topTagPrefixes?: string[];
topTagPrefixes?: readonly string[];

// tracking section controls the collection of usage metrics as analytics events.
// By default, Jaeger uses Google Analytics for event tracking (if enabled).
Expand All @@ -149,7 +149,7 @@ export type Config = {
// strings support variable substitution.
// A trace level link is displayed as an icon at the top of the trace view.
// A tag-level link converts the tag value into a hyperlink.
linkPatterns?: LinkPatternsConfig;
linkPatterns?: readonly LinkPatternsConfig[];

// monitor section controls Service Performance Monitoring tab.
monitor?: MonitorConfig;
Expand All @@ -159,7 +159,7 @@ export type Config = {
deepDependencies?: {
menuEnabled?: boolean;
};
pathAgnosticDecorations?: TPathAgnosticDecorationSchema[];
pathAgnosticDecorations?: readonly TPathAgnosticDecorationSchema[];
qualityMetrics?: {
menuEnabled?: boolean;
menuLabel?: string;
Expand Down
1 change: 1 addition & 0 deletions packages/jaeger-ui/src/utils/DraggableManager/DraggableManager.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.

import _get from 'lodash/get';
import type React from 'react';

import EUpdateTypes from './EUpdateTypes';
import { DraggableBounds, DraggingUpdate } from './types';
Expand Down
Loading