♻️ [RUMF-1267] remove circular dependencies part 1 (#1559)

* 🚨 [RUMF-1267] enable import/no-cycle as a warning

* ♻️ [RUMF-1267] split some imports

By splitting imports, we import less stuff, which removes a few cycles

* ♻️ [RUMF-1267] remove cycle in recorder

Move `shouldIgnoreNode` to `privacy` module. `shouldIgnoreNode` is only
used in `privacy`, so it makes sense to put it there.

* ♻️ [RUMF-1267] remove cycle in session

Move session constants to a separate module

* ♻️ [RUMF-1267] remove cycle in performances script

Move the utility function to check if an URL is a SDK bundle URL into a
new utility module.

* ♻️ [RUMF-1267] remove cycle in e2e tests

Move `flushEvents` in the `framework`, as it is both using `framework`
functions and is used by the `framework`.

.eslintrc.js

@@ -164,6 +164,7 @@ module.exports = {
     '@typescript-eslint/no-unused-vars': ['error', { args: 'all', argsIgnorePattern: '^_', vars: 'all' }],
     '@typescript-eslint/triple-slash-reference': ['error', { path: 'always', types: 'prefer-import', lib: 'always' }],
 
+    'import/no-cycle': ['warn'],
     'import/no-default-export': 'error',
     'import/no-duplicates': 'error',
     'import/no-extraneous-dependencies': 'error',

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

@@ -6,7 +6,7 @@ import {
   OLD_SESSION_COOKIE_NAME,
   tryOldCookiesMigration,
 } from './oldCookiesMigration'
-import { SESSION_EXPIRATION_DELAY } from './sessionStore'
+import { SESSION_EXPIRATION_DELAY } from './sessionConstants'
 import { SESSION_COOKIE_NAME } from './sessionCookieStore'
 
 describe('old cookies migration', () => {

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

@@ -0,0 +1,4 @@
+import { ONE_HOUR, ONE_MINUTE } from '../../tools/utils'
+
+export const SESSION_TIME_OUT_DELAY = 4 * ONE_HOUR
+export const SESSION_EXPIRATION_DELAY = 15 * ONE_MINUTE

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

@@ -3,8 +3,8 @@ import { getCookie, setCookie } from '../../browser/cookie'
 import { isChromium } from '../../tools/browserDetection'
 import * as utils from '../../tools/utils'
 import { monitor } from '../internalMonitoring'
+import { SESSION_EXPIRATION_DELAY } from './sessionConstants'
 import type { SessionState } from './sessionStore'
-import { SESSION_EXPIRATION_DELAY } from './sessionStore'
 
 const SESSION_ENTRY_REGEXP = /^([a-z]+)=([a-z0-9-]+)$/
 const SESSION_ENTRY_SEPARATOR = '&'

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

@@ -7,8 +7,8 @@ import type { RelativeTime } from '../../tools/timeUtils'
 import { isIE } from '../../tools/browserDetection'
 import type { SessionManager } from './sessionManager'
 import { startSessionManager, stopSessionManager, VISIBILITY_CHECK_DELAY } from './sessionManager'
-import { SESSION_TIME_OUT_DELAY, SESSION_EXPIRATION_DELAY } from './sessionStore'
 import { SESSION_COOKIE_NAME } from './sessionCookieStore'
+import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants'
 
 const enum FakeTrackingType {
   NOT_TRACKED = 'not-tracked',

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

@@ -7,7 +7,8 @@ import { ContextHistory } from '../../tools/contextHistory'
 import type { RelativeTime } from '../../tools/timeUtils'
 import { relativeNow, clocksOrigin } from '../../tools/timeUtils'
 import { tryOldCookiesMigration } from './oldCookiesMigration'
-import { startSessionStore, SESSION_TIME_OUT_DELAY } from './sessionStore'
+import { startSessionStore } from './sessionStore'
+import { SESSION_TIME_OUT_DELAY } from './sessionConstants'
 
 export interface SessionManager<TrackingType extends string> {
   findActiveSession: (startTime?: RelativeTime) => SessionContext<TrackingType> | undefined

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

@@ -3,8 +3,9 @@ import { mockClock } from '../../../test/specHelper'
 import type { CookieOptions } from '../../browser/cookie'
 import { getCookie, setCookie, COOKIE_ACCESS_DELAY } from '../../browser/cookie'
 import type { SessionStore } from './sessionStore'
-import { startSessionStore, SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionStore'
+import { startSessionStore } from './sessionStore'
 import { SESSION_COOKIE_NAME } from './sessionCookieStore'
+import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants'
 
 const enum FakeTrackingType {
   TRACKED = 'tracked',

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

@@ -3,6 +3,7 @@ import { COOKIE_ACCESS_DELAY } from '../../browser/cookie'
 import { Observable } from '../../tools/observable'
 import * as utils from '../../tools/utils'
 import { monitor } from '../internalMonitoring'
+import { SESSION_TIME_OUT_DELAY } from './sessionConstants'
 import { retrieveSession, withCookieLockAccess } from './sessionCookieStore'
 
 export interface SessionStore {
@@ -23,9 +24,6 @@ export interface SessionState {
   [key: string]: string | undefined
 }
 
-export const SESSION_EXPIRATION_DELAY = 15 * utils.ONE_MINUTE
-export const SESSION_TIME_OUT_DELAY = 4 * utils.ONE_HOUR
-
 /**
  * Different session concepts:
  * - tracked, the session has an id and is updated along the user navigation

packages/core/src/index.ts

@@ -37,7 +37,7 @@ export {
 } from './domain/session/sessionManager'
 export {
   SESSION_TIME_OUT_DELAY, // Exposed for tests
-} from './domain/session/sessionStore'
+} from './domain/session/sessionConstants'
 export {
   HttpRequest,
   Batch,

packages/core/src/tools/createEventRateLimiter.ts

@@ -1,5 +1,7 @@
-import type { RawError } from '..'
-import { clocksNow, ErrorSource, ONE_MINUTE } from '..'
+import type { RawError } from './error'
+import { ErrorSource } from './error'
+import { clocksNow } from './timeUtils'
+import { ONE_MINUTE } from './utils'
 
 export type EventRateLimiter = ReturnType<typeof createEventRateLimiter>
 

packages/core/src/transport/startBatchWithReplica.ts

@@ -1,6 +1,7 @@
 import type { Configuration, EndpointBuilder } from '../domain/configuration'
 import type { Context } from '../tools/context'
-import { Batch, HttpRequest } from './index'
+import { Batch } from './batch'
+import { HttpRequest } from './httpRequest'
 
 export function startBatchWithReplica<T extends Context>(
   configuration: Configuration,

packages/rum/src/domain/record/privacy.ts

@@ -15,8 +15,6 @@ import {
 
 export const MAX_ATTRIBUTE_VALUE_CHAR_LENGTH = 100_000
 
-import { shouldIgnoreElement } from './serialize'
-
 const TEXT_MASKING_CHAR = 'x'
 
 /**
@@ -215,3 +213,76 @@ export function getTextContent(
   }
   return textContent
 }
+
+/**
+ * TODO: Preserve CSS element order, and record the presence of the tag, just don't render
+ * We don't need this logic on the recorder side.
+ * For security related meta's, customer can mask themmanually given they
+ * are easy to identify in the HEAD tag.
+ */
+export function shouldIgnoreElement(element: Element): boolean {
+  if (element.nodeName === 'SCRIPT') {
+    return true
+  }
+
+  if (element.nodeName === 'LINK') {
+    const relAttribute = getLowerCaseAttribute('rel')
+    return (
+      // Scripts
+      (relAttribute === 'preload' && getLowerCaseAttribute('as') === 'script') ||
+      // Favicons
+      relAttribute === 'shortcut icon' ||
+      relAttribute === 'icon'
+    )
+  }
+
+  if (element.nodeName === 'META') {
+    const nameAttribute = getLowerCaseAttribute('name')
+    const relAttribute = getLowerCaseAttribute('rel')
+    const propertyAttribute = getLowerCaseAttribute('property')
+    return (
+      // Favicons
+      /^msapplication-tile(image|color)$/.test(nameAttribute) ||
+      nameAttribute === 'application-name' ||
+      relAttribute === 'icon' ||
+      relAttribute === 'apple-touch-icon' ||
+      relAttribute === 'shortcut icon' ||
+      // Description
+      nameAttribute === 'keywords' ||
+      nameAttribute === 'description' ||
+      // Social
+      /^(og|twitter|fb):/.test(propertyAttribute) ||
+      /^(og|twitter):/.test(nameAttribute) ||
+      nameAttribute === 'pinterest' ||
+      // Robots
+      nameAttribute === 'robots' ||
+      nameAttribute === 'googlebot' ||
+      nameAttribute === 'bingbot' ||
+      // Http headers. Ex: X-UA-Compatible, Content-Type, Content-Language, cache-control,
+      // X-Translated-By
+      element.hasAttribute('http-equiv') ||
+      // Authorship
+      nameAttribute === 'author' ||
+      nameAttribute === 'generator' ||
+      nameAttribute === 'framework' ||
+      nameAttribute === 'publisher' ||
+      nameAttribute === 'progid' ||
+      /^article:/.test(propertyAttribute) ||
+      /^product:/.test(propertyAttribute) ||
+      // Verification
+      nameAttribute === 'google-site-verification' ||
+      nameAttribute === 'yandex-verification' ||
+      nameAttribute === 'csrf-token' ||
+      nameAttribute === 'p:domain_verify' ||
+      nameAttribute === 'verify-v1' ||
+      nameAttribute === 'verification' ||
+      nameAttribute === 'shopify-checkout-api-token'
+    )
+  }
+
+  function getLowerCaseAttribute(name: string) {
+    return (element.getAttribute(name) || '').toLowerCase()
+  }
+
+  return false
+}

packages/rum/src/domain/record/serialize.ts

@@ -173,79 +173,6 @@ export function serializeElementNode(element: Element, options: SerializeOptions
   }
 }
 
-/**
- * TODO: Preserve CSS element order, and record the presence of the tag, just don't render
- * We don't need this logic on the recorder side.
- * For security related meta's, customer can mask themmanually given they
- * are easy to identify in the HEAD tag.
- */
-export function shouldIgnoreElement(element: Element): boolean {
-  if (element.nodeName === 'SCRIPT') {
-    return true
-  }
-
-  if (element.nodeName === 'LINK') {
-    const relAttribute = getLowerCaseAttribute('rel')
-    return (
-      // Scripts
-      (relAttribute === 'preload' && getLowerCaseAttribute('as') === 'script') ||
-      // Favicons
-      relAttribute === 'shortcut icon' ||
-      relAttribute === 'icon'
-    )
-  }
-
-  if (element.nodeName === 'META') {
-    const nameAttribute = getLowerCaseAttribute('name')
-    const relAttribute = getLowerCaseAttribute('rel')
-    const propertyAttribute = getLowerCaseAttribute('property')
-    return (
-      // Favicons
-      /^msapplication-tile(image|color)$/.test(nameAttribute) ||
-      nameAttribute === 'application-name' ||
-      relAttribute === 'icon' ||
-      relAttribute === 'apple-touch-icon' ||
-      relAttribute === 'shortcut icon' ||
-      // Description
-      nameAttribute === 'keywords' ||
-      nameAttribute === 'description' ||
-      // Social
-      /^(og|twitter|fb):/.test(propertyAttribute) ||
-      /^(og|twitter):/.test(nameAttribute) ||
-      nameAttribute === 'pinterest' ||
-      // Robots
-      nameAttribute === 'robots' ||
-      nameAttribute === 'googlebot' ||
-      nameAttribute === 'bingbot' ||
-      // Http headers. Ex: X-UA-Compatible, Content-Type, Content-Language, cache-control,
-      // X-Translated-By
-      element.hasAttribute('http-equiv') ||
-      // Authorship
-      nameAttribute === 'author' ||
-      nameAttribute === 'generator' ||
-      nameAttribute === 'framework' ||
-      nameAttribute === 'publisher' ||
-      nameAttribute === 'progid' ||
-      /^article:/.test(propertyAttribute) ||
-      /^product:/.test(propertyAttribute) ||
-      // Verification
-      nameAttribute === 'google-site-verification' ||
-      nameAttribute === 'yandex-verification' ||
-      nameAttribute === 'csrf-token' ||
-      nameAttribute === 'p:domain_verify' ||
-      nameAttribute === 'verify-v1' ||
-      nameAttribute === 'verification' ||
-      nameAttribute === 'shopify-checkout-api-token'
-    )
-  }
-
-  function getLowerCaseAttribute(name: string) {
-    return (element.getAttribute(name) || '').toLowerCase()
-  }
-
-  return false
-}
-
 /**
  * Text Nodes are dependant on Element nodes
  * Privacy levels are set on elements so we check the parentElement of a text node

performances/src/profilers/startCpuProfiling.ts

@@ -1,6 +1,6 @@
 import type { CDPSession } from 'puppeteer'
 import type { ProfilingOptions } from '../types'
-import { isSdkBundleUrl } from './startProfiling'
+import { isSdkBundleUrl } from '../utils'
 
 export async function startCPUProfiling(options: ProfilingOptions, client: CDPSession) {
   await client.send('Profiler.enable')

performances/src/profilers/startMemoryProfiling.ts

@@ -1,6 +1,6 @@
 import type { CDPSession } from 'puppeteer'
+import { isSdkBundleUrl } from '../utils'
 import type { ProfilingOptions } from '../types'
-import { isSdkBundleUrl } from './startProfiling'
 
 export async function startMemoryProfiling(options: ProfilingOptions, client: CDPSession) {
   await client.send('HeapProfiler.enable')

performances/src/profilers/startNetworkProfiling.ts

@@ -1,6 +1,6 @@
 import type { CDPSession, Protocol } from 'puppeteer'
 import type { ProfilingOptions } from '../types'
-import { isSdkBundleUrl } from './startProfiling'
+import { isSdkBundleUrl } from '../utils'
 
 export async function startNetworkProfiling(options: ProfilingOptions, client: CDPSession) {
   await client.send('Network.enable')

performances/src/profilers/startProfiling.ts

@@ -19,7 +19,3 @@ export async function startProfiling(options: ProfilingOptions, page: Page) {
     }),
   }
 }
-
-export function isSdkBundleUrl(options: ProfilingOptions, url: string) {
-  return url === options.bundleUrl
-}

performances/src/utils.ts

@@ -0,0 +1,5 @@
+import type { ProfilingOptions } from './types'
+
+export function isSdkBundleUrl(options: ProfilingOptions, url: string) {
+  return url === options.bundleUrl
+}

test/e2e/lib/framework/createTest.ts

@@ -1,9 +1,9 @@
 import type { LogsInitConfiguration } from '@datadog/browser-logs'
 import type { RumInitConfiguration } from '@datadog/browser-rum-core'
 import { deleteAllCookies, getBrowserName, withBrowserLogs } from '../helpers/browser'
-import { flushEvents } from '../helpers/flushEvents'
 import { validateRumFormat } from '../helpers/validation'
 import { EventRegistry } from './eventsRegistry'
+import { flushEvents } from './flushEvents'
 import type { Servers } from './httpServers'
 import { getTestServers, waitForServersIdle } from './httpServers'
 import { log } from './logger'

test/e2e/lib/helpers/flushEvents.ts → test/e2e/lib/framework/flushEvents.ts

@@ -1,5 +1,5 @@
-import { getTestServers, waitForServersIdle } from '../framework'
-import { browserExecuteAsync } from './browser'
+import { browserExecuteAsync } from '../helpers/browser'
+import { getTestServers, waitForServersIdle } from './httpServers'
 
 export async function flushEvents() {
   // wait to process actions + event loop before switching page

test/e2e/lib/framework/index.ts

@@ -2,3 +2,4 @@ export { createTest } from './createTest'
 export { bundleSetup, html } from './pageSetups'
 export { EventRegistry } from './eventsRegistry'
 export { getTestServers, waitForServersIdle } from './httpServers'
+export { flushEvents } from './flushEvents'

test/e2e/scenario/eventBridge.scenario.ts

@@ -1,6 +1,5 @@
 import { browserExecute, flushBrowserLogs } from '../lib/helpers/browser'
-import { createTest, html } from '../lib/framework'
-import { flushEvents } from '../lib/helpers/flushEvents'
+import { createTest, flushEvents, html } from '../lib/framework'
 
 describe('bridge present', () => {
   createTest('send action')

test/e2e/scenario/internalMonitoring.scenario.ts

@@ -1,7 +1,6 @@
 import type { TelemetryErrorEvent } from '@datadog/browser-core/src/domain/internalMonitoring/telemetryEvent.types'
-import { bundleSetup, createTest } from '../lib/framework'
+import { bundleSetup, createTest, flushEvents } from '../lib/framework'
 import { browserExecute } from '../lib/helpers/browser'
-import { flushEvents } from '../lib/helpers/flushEvents'
 
 describe('internal monitoring', () => {
   createTest('send errors for logs')

test/e2e/scenario/logs.scenario.ts

@@ -1,8 +1,7 @@
 import { DEFAULT_REQUEST_ERROR_RESPONSE_LENGTH_LIMIT } from '@datadog/browser-logs/cjs/domain/configuration'
-import { createTest } from '../lib/framework'
+import { createTest, flushEvents } from '../lib/framework'
 import { UNREACHABLE_URL } from '../lib/helpers/constants'
 import { browserExecute, browserExecuteAsync, flushBrowserLogs, withBrowserLogs } from '../lib/helpers/browser'
-import { flushEvents } from '../lib/helpers/flushEvents'
 
 describe('logs', () => {
   createTest('send logs')

test/e2e/scenario/recorder/recorder.scenario.ts

@@ -18,9 +18,8 @@ import {
 } from '@datadog/browser-rum/test/utils'
 import { renewSession } from '../../lib/helpers/session'
 import type { EventRegistry } from '../../lib/framework'
-import { createTest, bundleSetup, html } from '../../lib/framework'
+import { flushEvents, createTest, bundleSetup, html } from '../../lib/framework'
 import { browserExecute } from '../../lib/helpers/browser'
-import { flushEvents } from '../../lib/helpers/flushEvents'
 
 const INTEGER_RE = /^\d+$/
 const TIMESTAMP_RE = /^\d{13}$/

test/e2e/scenario/recorder/viewports.scenario.ts

@@ -4,9 +4,8 @@ import type { RumInitConfiguration } from '@datadog/browser-rum-core'
 
 import { findAllIncrementalSnapshots, findAllVisualViewports } from '@datadog/browser-rum/test/utils'
 import type { EventRegistry } from '../../lib/framework'
-import { createTest, bundleSetup, html } from '../../lib/framework'
+import { flushEvents, createTest, bundleSetup, html } from '../../lib/framework'
 import { browserExecute, getBrowserName, getPlatformName } from '../../lib/helpers/browser'
-import { flushEvents } from '../../lib/helpers/flushEvents'
 
 const NAVBAR_HEIGHT_CHANGE_UPPER_BOUND = 30
 const VIEWPORT_META_TAGS = `

test/e2e/scenario/rum/actions.scenario.ts

@@ -1,6 +1,5 @@
 import { withBrowserLogs } from '../../lib/helpers/browser'
-import { createTest, html, waitForServersIdle } from '../../lib/framework'
-import { flushEvents } from '../../lib/helpers/flushEvents'
+import { createTest, flushEvents, html, waitForServersIdle } from '../../lib/framework'
 
 describe('action collection', () => {
   createTest('track a click action')