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.

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

Bump typescript-eslint from 7.14.1 to 8.12.2 #48275

Merged
merged 8 commits into from
Nov 19, 2024
Merged
175 changes: 119 additions & 56 deletions pnpm-lock.yaml

Large diffs are not rendered by default.

18 changes: 18 additions & 0 deletions web/packages/build/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ module.exports = {
'jest/no-large-snapshots': ['warn', { maxSize: 200 }],
},
},
// Allow require imports in .js files, as migrating our project to ESM modules requires a lot of
// changes.
{
files: ['**/*.js'],
rules: {
'@typescript-eslint/no-require-imports': 'warn',
Copy link
Member

Choose a reason for hiding this comment

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

I tried replacing requires with imports everywhere (it's on r7s/esm), but I couldn't get Babel to work with Jest.

Copy link
Contributor

Choose a reason for hiding this comment

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

I currently have a branch trying to upgrade eslint to v9 and all the dependencies have required a massive change. We can table this one until I'm able to carve out enough time this quarter to finally tackle it.

Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be better if this particular update was already taken care of? Then the final PR will be smaller.

Copy link
Member

Choose a reason for hiding this comment

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

And we wouldn't risk introducing new lint issues.

},
},
],
rules: {
'import/order': [
Expand All @@ -88,6 +96,16 @@ module.exports = {
'import/no-unresolved': 0,
'no-unused-vars': 'off', // disabled to allow the typescript one to take over and avoid errors in reporting
'@typescript-eslint/no-unused-vars': ['error'],
'no-unused-expressions': 'off',
'@typescript-eslint/no-unused-expressions': [
Copy link
Member

Choose a reason for hiding this comment

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

@rudream no-unused-expressions complains about this line and one below it that simply attempts to read the property. Is reading the property alone enough to force reflow?

itemRef.current.offsetHeight; // Force reflow

Copy link
Contributor

@avatus avatus Nov 18, 2024

Choose a reason for hiding this comment

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

Is reading the property alone enough to force reflow?

yes, it causes the recalculation.
i just prepended void (as found recommended online) and it causes the error to go away

'error',
{ allowShortCircuit: true, allowTernary: true, enforceForJSX: true },
],
'@typescript-eslint/no-empty-object-type': [
'error',
// with-single-extends is needed to allow for interface extends like we have in jest.d.ts.
{ allowInterfaces: 'with-single-extends' },
],

// Severity should be one of the following:
// "off" or 0 - turn the rule off
Expand Down
10 changes: 6 additions & 4 deletions web/packages/build/jest/setupTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@

import 'whatwg-fetch';

const crypt = require('crypto');
const path = require('path');
import crypto from 'node:crypto';
import path from 'node:path';

const failOnConsole = require('jest-fail-on-console');
import failOnConsole from 'jest-fail-on-console';

let entFailOnConsoleIgnoreList = [];
try {
// Cannot do `await import` yet here.
// eslint-disable-next-line @typescript-eslint/no-require-imports
entFailOnConsoleIgnoreList = require('../../../../e/web/testsWithIgnoredConsole');
} catch (err) {
// Ignore errors related to teleport.e not being present. This allows OSS users and OSS CI to run
Expand All @@ -36,7 +38,7 @@ try {

Object.defineProperty(globalThis, 'crypto', {
value: {
randomUUID: () => crypt.randomUUID(),
randomUUID: () => crypto.randomUUID(),
},
});

Expand Down
2 changes: 1 addition & 1 deletion web/packages/build/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"jest-fail-on-console": "^3.3.0",
"jsdom": "^25.0.1",
"rollup-plugin-visualizer": "^5.12.0",
"typescript-eslint": "^7.14.1",
"typescript-eslint": "^8.12.2",
"vite-plugin-wasm": "^3.3.0",
"vite-tsconfig-paths": "^5.0.1"
}
Expand Down
2 changes: 1 addition & 1 deletion web/packages/design/src/utils/copyToClipboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function showErrorInBrowser(error: unknown, textToCopy: string): void {
try {
// window.prompt doesn't work in Electron.
window.prompt(message, textToCopy);
} catch (error) {
} catch {
window.alert(message);
}
}
2 changes: 1 addition & 1 deletion web/packages/shared/hooks/useLocalStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export function useLocalStorage<T>(

try {
return JSON.parse(value) as T;
} catch (err) {
} catch {
return initialValue;
}
}, [initialValue, key]);
Expand Down
2 changes: 1 addition & 1 deletion web/packages/shared/redirects/processRedirectUri.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export function processRedirectUri(redirectUri: string | null): string {
}

return `${BASE_PATH}${path.startsWith('/') ? '' : '/'}${path}`;
} catch (error) {
} catch {
// If it's not a valid URL, it might be a relative path
if (redirectUri.startsWith('/')) {
return redirectUri.startsWith(BASE_PATH)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ export function parseRepoAddress(repoAddr: string): {
let url;
try {
url = new URL(repoAddr);
} catch (e) {
} catch {
throw new Error('Must be a valid URL');
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ function getErrorText(response: string | undefined): string {
try {
const json = JSON.parse(response);
return json.error?.message || json.message || badRequest;
} catch (err) {
} catch {
return 'Bad request, failed to parse error message.';
}
}
Expand Down
2 changes: 1 addition & 1 deletion web/packages/teleport/src/LocksV2/Locks/Locks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ export function Locks() {
function getFormattedDate(d: string): string {
try {
return formatRelative(new Date(d), Date.now());
} catch (e) {
} catch {
return '';
}
}
Expand Down
4 changes: 2 additions & 2 deletions web/packages/teleport/src/Navigation/RecentHistory.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ function AnimatedHistoryItem({
const height = item.category ? 60 : 40;
itemRef.current.style.height = `${height}px`;
itemRef.current.style.opacity = '1';
itemRef.current.offsetHeight; // Force reflow
void itemRef.current.offsetHeight; // Force reflow
requestAnimationFrame(() => {
if (itemRef.current) {
itemRef.current.style.height = '0px';
Expand All @@ -189,7 +189,7 @@ function AnimatedHistoryItem({
const height = item.category ? 60 : 40;
itemRef.current.style.height = `0px`;
itemRef.current.style.opacity = '0';
itemRef.current.offsetHeight; // Force reflow
void itemRef.current.offsetHeight; // Force reflow
requestAnimationFrame(() => {
if (itemRef.current) {
itemRef.current.style.height = `${height}px`;
Expand Down
2 changes: 1 addition & 1 deletion web/packages/teleport/src/User/UserContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export function UserContextProvider(props: PropsWithChildren<unknown>) {

setPreferences(preferences);
storageService.setUserPreferences(preferences);
} catch (err) {
} catch {
if (storedPreferences) {
setPreferences(storedPreferences);

Expand Down
4 changes: 2 additions & 2 deletions web/packages/teleport/src/lib/term/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export default class TtyTerminal {
this.fallbackToCanvas();
});
this.term.loadAddon(this._webglAddon);
} catch (err) {
} catch {
this.fallbackToCanvas();
}

Expand Down Expand Up @@ -137,7 +137,7 @@ export default class TtyTerminal {
this._webglAddon = undefined;
try {
this.term.loadAddon(this._canvasAddon);
} catch (err) {
} catch {
logger.error(
'Canvas renderer could not be loaded. Falling back to default'
);
Expand Down
7 changes: 4 additions & 3 deletions web/packages/teleport/src/services/api/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,15 +168,16 @@ const makeFoo = (): { foo: string } => {
// This is a bogus test to satisfy Jest. We don't even need to execute the code that's in the async
// function, we're interested only in the type system checking the code.
test('fetchJson does not return any', () => {
async () => {
const bogusFunction = async () => {
const result = await fooService.doSomething();
// Reading foo is correct. We add a bogus expect to satisfy Jest.
result.foo;
JSON.stringify(result.foo);

// @ts-expect-error If there's no error here, it means that api.fetchJson returns any, which it
// shouldn't.
result.bar;
JSON.stringify(result.bar);
};
bogusFunction.toString(); // Just to satisfy the linter

expect(true).toBe(true);
});
Expand Down
2 changes: 1 addition & 1 deletion web/packages/teleport/src/services/api/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ const api = {
webauthnResponseForRetry = await auth.getWebauthnResponse(
MfaChallengeScope.ADMIN_ACTION
);
} catch (err) {
} catch {
throw new Error(
'Failed to fetch webauthn credentials, please connect a registered hardware key and try again. If you do not have a hardware key registered, you can add one from your account settings page.'
);
Expand Down
2 changes: 1 addition & 1 deletion web/packages/teleport/src/stores/storeNotifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ export class StoreNotifications extends Store<NotificationStoreState> {

try {
return JSON.parse(value) as LocalNotificationStates;
} catch (err) {
} catch {
return defaultLocalNotificationStates;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export function loadInstallationId(filePath: string): string {
let id = '';
try {
id = fs.readFileSync(filePath, 'utf-8');
} catch (error) {
} catch {
return writeInstallationId(filePath);
}
if (!UUID_V4_REGEX.test(id)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export function Status(props: { closeDocument?: () => void }) {
const downloadAndStartAgentAndIgnoreErrors = useCallback(async () => {
try {
await downloadAndStartAgent();
} catch (error) {
} catch {
// Ignore the error, it'll be shown in the UI by inspecting the attempts.
}
}, [downloadAndStartAgent]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ export const ConnectMyComputerContextProvider: FC<
(async () => {
try {
await downloadAndStartAgent();
} catch (error) {
} catch {
// Turn off autostart if it fails, otherwise the user wouldn't be able to turn it off by
// themselves.
workspacesService.setConnectMyComputerAutoStart(
Expand Down
Loading