📦️ update typescript-eslint (#3192)

* 📦️ update typescript-eslint and adjust config

We still use the ("legacy setup")[1] on purpose: we'll upgrade to a flat
config in a separate PR, when we upgrade ESLint.

[1]: https://typescript-eslint.io/getting-started/legacy-eslint-setup

Rule changes:

* @typescript-eslint/ban-types have been split in multiple rules, most
  of them are enabled by default. The configuration we used has been
  ported to @typescript-eslint/no-restricted-types

* no-throw-literal isn't needed anymore as the new rule
  @typescript-eslint/only-throw-error is covering the same thing and
  more.

* @typescript-eslint/no-var-requires was replaced with
  @typescript-eslint/no-require-imports.

* @typescript-eslint/no-unused-experessions was turned on in the
  recommended rules, but we have a few cases where we avoid using an
  `if` like `foo && bar()` that raise that error, so I disabled the rule
  for now.

* 🚨 fix lint issues related to typescript-eslint update

* The new @typescript-eslint/only-throw-error reports quite a few (valid)
  cases where we don't throw errors. I ignored those as issues as this was
  generally what we wanted.

* Some eslint-disable comments were not needed, so I removed them

* Optional properties and arguments that also accept `undefined` now
  raise an error

* typescript-eslint is also better at finding unused variables in catch
  clauses

* And other minor things...

* Turn on rule no-unused-expressions

* Update developer-extension/src/panel/components/tabs/infosTab.tsx

Co-authored-by: Thomas Lebeau <[email protected]>

---------

Co-authored-by: zcy <[email protected]>
Co-authored-by: Thomas Lebeau <[email protected]>

Author: 3 people

.eslintrc.js

@@ -20,6 +20,26 @@ module.exports = {
       './performances/tsconfig.json',
     ],
     sourceType: 'module',
+
+    // Without this option, typescript-eslint fails to lint .js files because we don't use
+    // `allowJs: true` in the TypeScript configuration. Same issue as
+    // https://github.com/typescript-eslint/typescript-eslint/issues/9749.
+    //
+    // Enabling `allowJs` would be a better solution, but right now it's not possible because of a
+    // pretty small reason with big implications: `webpack.base.js` includes
+    // `tsconfig-paths-webpack-plugin`, and this file includes Node.js types via a <reference>
+    // directive (see https://unpkg.com/browse/[email protected]/lib/plugin.d.ts).
+    //
+    // Because of this, Node.js types are included in the whole project, and because some of them
+    // are slightly different from the DOM/Browser types, some annoying typecheck errors are raised.
+    //
+    // So, ideally, we should:
+    // * add `allowJs: true` in the TypeScript configuration
+    // * have different tsconfig.json configurations for packages and scripts
+    // * when typechecking, run `tsc` multiple time with each configuration (like we do for the
+    //   developer-extension)
+    // * then remove this option
+    disallowAutomaticSingleRunInference: true,
   },
   plugins: [
     'eslint-plugin-import',
@@ -60,7 +80,6 @@ module.exports = {
     'no-return-await': 'error',
     'no-sequences': 'error',
     'no-template-curly-in-string': 'error',
-    'no-throw-literal': 'error',
     'no-undef-init': 'error',
     'no-unreachable': 'error',
     'no-useless-concat': 'error',
@@ -94,14 +113,12 @@ module.exports = {
         'ts-check': 'allow-with-description',
       },
     ],
-    '@typescript-eslint/ban-types': [
+    '@typescript-eslint/no-restricted-types': [
       'error',
       {
         types: {
           /* eslint-disable id-denylist */
           Object: { message: 'Avoid using the `Object` type. Did you mean `object`?' },
-          object: false,
-          Function: { message: 'Avoid using the `Function` type. Prefer a specific function type, like `() => void`.' },
           Boolean: { message: 'Avoid using the `Boolean` type. Did you mean `boolean`?' },
           Number: { message: 'Avoid using the `Number` type. Did you mean `number`?' },
           String: { message: 'Avoid using the `String` type. Did you mean `string`?' },
@@ -288,7 +305,7 @@ module.exports = {
         node: true,
       },
       rules: {
-        '@typescript-eslint/no-var-requires': 'off',
+        '@typescript-eslint/no-require-imports': 'off',
       },
     },
     {

developer-extension/src/panel/components/panel.tsx

@@ -30,7 +30,7 @@ export function Panel() {
   const [activeTab, setActiveTab] = useState<string | null>(DEFAULT_PANEL_TAB)
   function updateActiveTab(activeTab: string | null) {
     setActiveTab(activeTab)
-    activeTab && datadogRum.startView(activeTab)
+    datadogRum.startView(activeTab!)
   }
 
   return (

developer-extension/src/panel/components/tabs/infosTab.tsx

@@ -169,7 +169,7 @@ function Entry({
   }
 
   const handleClearClick = () => {
-    onChange && onChange(null)
+    onChange?.(null)
     reloadPage()
   }
 

developer-extension/src/panel/hooks/useSettings.ts

@@ -57,6 +57,7 @@ export function useSettings() {
   // If we don't have settings yet, it means that we are still loading them from the storage. Throw
   // the promise so it'll be caught by the Suspense boundary.
   if (!settings) {
+    // eslint-disable-next-line @typescript-eslint/only-throw-error
     throw storageLoadingPromise
   }
 

developer-extension/src/panel/index.tsx

@@ -1,7 +1,6 @@
 // The default eslint-plugin-import resolver does not support "exports" fields in package.json yet.
 // Ignore the error until the default resolver supports it, or we switch to a different resolver.
 // https://github.com/import-js/eslint-plugin-import/issues/1810
-// eslint-disable-next-line import/no-unresolved
 import '@mantine/core/styles.layer.css'
 
 import './global.css'

package.json

@@ -37,8 +37,8 @@
     "@types/cors": "2.8.17",
     "@types/express": "4.17.21",
     "@types/jasmine": "3.10.18",
-    "@typescript-eslint/eslint-plugin": "7.18.0",
-    "@typescript-eslint/parser": "7.18.0",
+    "@typescript-eslint/eslint-plugin": "8.16.0",
+    "@typescript-eslint/parser": "8.16.0",
     "@wdio/browserstack-service": "8.40.6",
     "@wdio/cli": "8.40.6",
     "@wdio/jasmine-framework": "8.40.6",

packages/core/src/boot/init.spec.ts

@@ -34,6 +34,7 @@ describe('defineGlobal', () => {
   it('catches the errors thrown by the queued callbacks', () => {
     const myError = 'Ooops!'
     const onReady = () => {
+      // eslint-disable-next-line @typescript-eslint/only-throw-error
       throw myError
     }
     const myGlobal: any = {

packages/core/src/browser/browser.types.ts

@@ -53,6 +53,7 @@ export interface WeakRefConstructor {
 
 // Those are native API types that are not official supported by TypeScript yet
 
+// eslint-disable-next-line @typescript-eslint/no-empty-object-type
 export interface CookieStore extends EventTarget {}
 
 export interface CookieStoreEventMap {
@@ -65,5 +66,3 @@ export type CookieChangeEvent = Event & {
   changed: CookieChangeItem[]
   deleted: CookieChangeItem[]
 }
-
-export interface CookieStore extends EventTarget {}

packages/core/src/domain/configuration/configuration.spec.ts

@@ -155,6 +155,7 @@ describe('validateAndBuildConfiguration', () => {
     it('should catch errors and log them', () => {
       const myError = 'Ooops!'
       const beforeSend = () => {
+        // eslint-disable-next-line @typescript-eslint/only-throw-error
         throw myError
       }
       const configuration = validateAndBuildConfiguration({ clientToken, beforeSend })!

packages/core/src/domain/console/consoleObservable.spec.ts

@@ -118,7 +118,6 @@ describe('console error observable', () => {
     const error = new Error('foo')
     ;(error as DatadogError).dd_fingerprint = 'my-fingerprint'
 
-    // eslint-disable-next-line no-console
     console.error(error)
 
     const consoleLog = notifyLog.calls.mostRecent().args[0]
@@ -129,7 +128,6 @@ describe('console error observable', () => {
     const error = new Error('foo')
     ;(error as any).dd_fingerprint = 2
 
-    // eslint-disable-next-line no-console
     console.error(error)
 
     const consoleLog = notifyLog.calls.mostRecent().args[0]

packages/core/src/domain/context/contextManager.ts

@@ -17,7 +17,7 @@ export function createContextManager(customerDataTracker?: CustomerDataTracker)
     setContext: (newContext: Context) => {
       if (getType(newContext) === 'object') {
         context = sanitize(newContext)
-        customerDataTracker && customerDataTracker.updateCustomerData(context)
+        customerDataTracker?.updateCustomerData(context)
       } else {
         contextManager.clearContext()
       }
@@ -26,19 +26,19 @@ export function createContextManager(customerDataTracker?: CustomerDataTracker)
 
     setContextProperty: (key: string, property: any) => {
       context[key] = sanitize(property)
-      customerDataTracker && customerDataTracker.updateCustomerData(context)
+      customerDataTracker?.updateCustomerData(context)
       changeObservable.notify()
     },
 
     removeContextProperty: (key: string) => {
       delete context[key]
-      customerDataTracker && customerDataTracker.updateCustomerData(context)
+      customerDataTracker?.updateCustomerData(context)
       changeObservable.notify()
     },
 
     clearContext: () => {
       context = {}
-      customerDataTracker && customerDataTracker.resetCustomerData()
+      customerDataTracker?.resetCustomerData()
       changeObservable.notify()
     },
 

packages/core/src/domain/error/trackRuntimeError.spec.ts

@@ -113,7 +113,6 @@ describe('instrumentOnError', () => {
   it('should notify unhandled string', (done) => {
     const error = 'foo' as any
     setTimeout(() => {
-      // eslint-disable-next-line no-throw-literal
       throw error
     })
     collectAsyncCalls(onErrorSpy, 1, () => {
@@ -127,7 +126,6 @@ describe('instrumentOnError', () => {
   it('should notify unhandled object', (done) => {
     const error = { a: 'foo' } as any
     setTimeout(() => {
-      // eslint-disable-next-line no-throw-literal
       throw error
     })
     collectAsyncCalls(onErrorSpy, 1, () => {

packages/core/src/domain/session/sessionStoreOperations.spec.ts

@@ -47,7 +47,9 @@ const EXPIRED_SESSION: SessionState = { isExpired: '1' }
 
     describe('with lock access disabled', () => {
       beforeEach(() => {
-        sessionStoreStrategy.isLockEnabled && pending('lock-access required')
+        if (sessionStoreStrategy.isLockEnabled) {
+          pending('lock-access required')
+        }
       })
 
       it('should persist session when process returns a value', () => {
@@ -99,7 +101,9 @@ const EXPIRED_SESSION: SessionState = { isExpired: '1' }
 
     describe('with lock access enabled', () => {
       beforeEach(() => {
-        !sessionStoreStrategy.isLockEnabled && pending('lock-access not enabled')
+        if (!sessionStoreStrategy.isLockEnabled) {
+          pending('lock-access not enabled')
+        }
       })
 
       it('should persist session when process returns a value', () => {

packages/core/src/domain/session/sessionStoreOperations.ts

@@ -79,7 +79,11 @@ export function processSessionStoreOperations(
       expireSession(processedSession)
     } else {
       expandSessionState(processedSession)
-      isLockEnabled ? persistWithLock(processedSession) : persistSession(processedSession)
+      if (isLockEnabled) {
+        persistWithLock(processedSession)
+      } else {
+        persistSession(processedSession)
+      }
     }
   }
   if (isLockEnabled) {

packages/core/src/domain/session/storeStrategies/sessionInLocalStorage.ts

@@ -14,7 +14,7 @@ export function selectLocalStorageStrategy(): SessionStoreStrategyType | undefin
     const retrievedId = localStorage.getItem(testKey)
     localStorage.removeItem(testKey)
     return id === retrievedId ? { type: 'LocalStorage' } : undefined
-  } catch (e) {
+  } catch {
     return undefined
   }
 }

packages/core/src/tools/catchUserErrors.spec.ts

@@ -11,6 +11,7 @@ describe('catchUserErrors', () => {
     const displaySpy = spyOn(display, 'error')
     const myError = 'Ooops!'
     const wrappedFn = catchUserErrors(() => {
+      // eslint-disable-next-line @typescript-eslint/only-throw-error
       throw myError
     }, 'Error during callback')
     expect(wrappedFn()).toBe(undefined)

packages/core/src/tools/experimentalFeatures.ts

@@ -24,7 +24,7 @@ export enum ExperimentalFeature {
 
 const enabledExperimentalFeatures: Set<ExperimentalFeature> = new Set()
 
-export function initFeatureFlags(enableExperimentalFeatures?: string[] | undefined) {
+export function initFeatureFlags(enableExperimentalFeatures: string[] | undefined) {
   if (Array.isArray(enableExperimentalFeatures)) {
     addExperimentalFeatures(
       enableExperimentalFeatures.filter((flag): flag is ExperimentalFeature =>

packages/core/src/tools/monitor.spec.ts

@@ -20,13 +20,13 @@ describe('monitor', () => {
 
       @monitored
       monitoredStringErrorThrowing() {
-        // eslint-disable-next-line no-throw-literal
+        // eslint-disable-next-line @typescript-eslint/only-throw-error
         throw 'string error'
       }
 
       @monitored
       monitoredObjectErrorThrowing() {
-        // eslint-disable-next-line no-throw-literal
+        // eslint-disable-next-line @typescript-eslint/only-throw-error
         throw { foo: 'bar' }
       }
 

packages/core/src/tools/serialisation/context.ts

@@ -4,5 +4,5 @@ export interface Context {
 
 export type ContextValue = string | number | boolean | Context | ContextArray | undefined | null
 
-// eslint-disable-next-line @typescript-eslint/no-empty-interface
+// eslint-disable-next-line @typescript-eslint/no-empty-object-type
 export interface ContextArray extends Array<ContextValue> {}

packages/core/src/tools/serialisation/sanitize.ts

@@ -4,7 +4,7 @@ import type { Context, ContextArray, ContextValue } from './context'
 import type { ObjectWithToJsonMethod } from './jsonStringify'
 import { detachToJsonMethod } from './jsonStringify'
 
-// eslint-disable-next-line @typescript-eslint/ban-types
+// eslint-disable-next-line @typescript-eslint/no-unsafe-function-type
 type PrimitivesAndFunctions = string | number | boolean | undefined | null | symbol | bigint | Function
 type ExtendedContextValue = PrimitivesAndFunctions | object | ExtendedContext | ExtendedContextArray
 type ExtendedContext = { [key: string]: ExtendedContextValue }

packages/core/src/tools/stackTrace/handlingStack.ts

@@ -24,7 +24,7 @@ export function createHandlingStack(): string {
   if (!error.stack) {
     try {
       throw error
-    } catch (e) {
+    } catch {
       noop()
     }
   }

packages/core/src/tools/utils/responseUtils.ts

@@ -5,7 +5,7 @@ export function isServerError(status: number) {
 export function tryToClone(response: Response): Response | undefined {
   try {
     return response.clone()
-  } catch (e) {
+  } catch {
     // clone can throw if the response has already been used by another instrumentation or is disturbed
     return
   }

packages/core/src/transport/httpRequest.spec.ts

@@ -57,7 +57,7 @@ describe('httpRequest', () => {
       }
       let notQueuedFetch: Promise<never>
       interceptor.withFetch(() => {
-        notQueuedFetch = Promise.reject()
+        notQueuedFetch = Promise.reject(new Error())
         return notQueuedFetch
       })
 
@@ -118,7 +118,7 @@ describe('httpRequest', () => {
         pending('no fetch keepalive support')
       }
 
-      interceptor.withFetch(() => Promise.reject())
+      interceptor.withFetch(() => Promise.reject(new Error()))
       interceptor.withMockXhr((xhr) => {
         setTimeout(() => {
           xhr.complete(429)

packages/logs/src/boot/logsPublicApi.spec.ts

@@ -18,8 +18,8 @@ describe('logs entry', () => {
     (
       logsMessage: LogsMessage,
       logger: Logger,
-      commonContext?: CommonContext | undefined,
-      date?: TimeStamp | undefined
+      commonContext: CommonContext | undefined,
+      date: TimeStamp | undefined
     ) => void
   >
   let startLogs: jasmine.Spy<StartLogs>

packages/logs/src/boot/logsPublicApi.ts

@@ -119,7 +119,7 @@ export interface LogsPublicApi extends PublicApi {
    *
    * See [Access internal context](https://docs.datadoghq.com/logs/log_collection/javascript/#access-internal-context) for further information.
    */
-  getInternalContext: (startTime?: number | undefined) => InternalContext | undefined
+  getInternalContext: (startTime?: number) => InternalContext | undefined
 
   /**
    * Set user information to all events, stored in `@usr`

packages/logs/src/domain/runtimeError/runtimeErrorCollection.spec.ts

@@ -70,6 +70,7 @@ describe('runtime error collection', () => {
     error.cause = nestedError
     ;({ stop: stopRuntimeErrorCollection } = startRuntimeErrorCollection(configuration, lifeCycle))
     setTimeout(() => {
+      // eslint-disable-next-line @typescript-eslint/only-throw-error
       throw error
     })
 

packages/rum-core/src/browser/htmlDomUtils.spec.ts

@@ -21,6 +21,7 @@ describe('isTextNode', () => {
   ]
 
   parameters.forEach(([element, result]) => {
+    // eslint-disable-next-line @typescript-eslint/no-base-to-string
     it(`should return ${String(result)} for "${String(element)}"`, () => {
       expect(isTextNode(element)).toBe(result)
     })
@@ -37,6 +38,7 @@ describe('isCommentNode', () => {
   ]
 
   parameters.forEach(([element, result]) => {
+    // eslint-disable-next-line @typescript-eslint/no-base-to-string
     it(`should return ${String(result)} for "${String(element)}"`, () => {
       expect(isCommentNode(element)).toBe(result)
     })
@@ -53,6 +55,7 @@ describe('isElementNode', () => {
   ]
 
   parameters.forEach(([element, result]) => {
+    // eslint-disable-next-line @typescript-eslint/no-base-to-string
     it(`should return ${String(result)} for "${String(element)}"`, () => {
       expect(isElementNode(element)).toBe(result)
     })
@@ -119,6 +122,7 @@ if (!isIE()) {
     ]
 
     parameters.forEach(([element, result]) => {
+      // eslint-disable-next-line @typescript-eslint/no-base-to-string
       it(`should return ${String(result)} for "${String(element)}"`, () => {
         expect(isNodeShadowHost(element)).toBe(result)
       })

packages/rum-core/src/domain/view/viewMetrics/trackInteractionToNextPaint.spec.ts

@@ -52,7 +52,7 @@ describe('trackInteractionToNextPaint', () => {
     setViewEnd = interactionToNextPaintTracking.setViewEnd
 
     registerCleanupTask(() => {
-      interactionToNextPaintTracking.stop
+      interactionToNextPaintTracking.stop()
       resetExperimentalFeatures()
       interactionCountMock.clear()
     })