diff --git a/core/src/utils/config.ts b/core/src/utils/config.ts index 0ad796a4ad5..cca58752ee7 100644 --- a/core/src/utils/config.ts +++ b/core/src/utils/config.ts @@ -217,6 +217,7 @@ export interface IonicConfig { * - `'OFF'`: No errors or warnings are logged. * - `'ERROR'`: Logs only errors. * - `'WARN'`: Logs errors and warnings. + * - `'DEBUG'`: Logs errors, warnings, and Ionic's internal diagnostics. */ logLevel?: LogLevel; diff --git a/core/src/utils/logging/index.ts b/core/src/utils/logging/index.ts index d13d6d8d0d1..48158d13bcb 100644 --- a/core/src/utils/logging/index.ts +++ b/core/src/utils/logging/index.ts @@ -4,8 +4,31 @@ export enum LogLevel { OFF = 'OFF', ERROR = 'ERROR', WARN = 'WARN', + DEBUG = 'DEBUG', } +/** + * Ranks each level so an enabled check is a numeric comparison. A configured + * level logs anything whose rank is less than or equal to its own: `OFF` (0) + * logs nothing, `ERROR` (1) logs errors, `WARN` (2) logs errors and warnings, + * `DEBUG` (3) logs all of the above plus internal diagnostics. + */ +const LOG_LEVEL_RANK: Record = { + [LogLevel.OFF]: 0, + [LogLevel.ERROR]: 1, + [LogLevel.WARN]: 2, + [LogLevel.DEBUG]: 3, +}; + +/** + * Whether the configured level is verbose enough to log `minimum`. Levels set + * through a query parameter arrive as raw strings, hence the uppercasing. + */ +const isLogLevelEnabled = (minimum: LogLevel): boolean => { + const configured = String(config.get('logLevel', LogLevel.WARN)).toUpperCase() as LogLevel; + return LOG_LEVEL_RANK[configured] >= LOG_LEVEL_RANK[minimum]; +}; + /** * Logs a warning to the console with an Ionic prefix * to indicate the library that is warning the developer. @@ -13,8 +36,7 @@ export enum LogLevel { * @param message - The string message to be logged to the console. */ export const printIonWarning = (message: string, ...params: any[]) => { - const logLevel = config.get('logLevel', LogLevel.WARN); - if ([LogLevel.WARN].includes(logLevel)) { + if (isLogLevelEnabled(LogLevel.WARN)) { return console.warn(`[Ionic Warning]: ${message}`, ...params); } }; @@ -27,8 +49,7 @@ export const printIonWarning = (message: string, ...params: any[]) => { * @param params - Additional arguments to supply to the console.error. */ export const printIonError = (message: string, ...params: any[]) => { - const logLevel = config.get('logLevel', LogLevel.ERROR); - if ([LogLevel.ERROR, LogLevel.WARN].includes(logLevel)) { + if (isLogLevelEnabled(LogLevel.ERROR)) { return console.error(`[Ionic Error]: ${message}`, ...params); } }; diff --git a/core/src/utils/logging/test/logging.spec.ts b/core/src/utils/logging/test/logging.spec.ts index bcaaa0b9632..986c95ae083 100644 --- a/core/src/utils/logging/test/logging.spec.ts +++ b/core/src/utils/logging/test/logging.spec.ts @@ -37,6 +37,16 @@ describe('Logging', () => { }); }); + describe("when the logLevel configuration is set to 'DEBUG'", () => { + it('logs a warning to the console', () => { + config.set('logLevel', LogLevel.DEBUG); + + printIonWarning('This is a warning message'); + + expect(consoleWarnSpy).toHaveBeenCalledWith('[Ionic Warning]: This is a warning message'); + }); + }); + describe("when the logLevel configuration is set to 'ERROR'", () => { it('does not log a warning to the console', () => { config.set('logLevel', LogLevel.ERROR); @@ -101,6 +111,16 @@ describe('Logging', () => { }); }); + describe("when the logLevel configuration is set to 'DEBUG'", () => { + it('logs an error to the console', () => { + config.set('logLevel', LogLevel.DEBUG); + + printIonError('This is an error message'); + + expect(consoleErrorSpy).toHaveBeenCalledWith('[Ionic Error]: This is an error message'); + }); + }); + describe("when the logLevel configuration is set to 'OFF'", () => { it('does not log an error to the console', () => { config.set('logLevel', LogLevel.OFF); diff --git a/docs/react-router/README.md b/docs/react-router/README.md index 4ef96223a68..36c3d714fbe 100644 --- a/docs/react-router/README.md +++ b/docs/react-router/README.md @@ -9,3 +9,21 @@ See our [Contributing Guide](/docs/CONTRIBUTING.md). ## Testing Refer to the [React Router Testing documentation](./testing.md) for testing the React Router package. + +## Debug Logging + +The `StackManager` logs the decisions behind the swipe-to-go-back gesture: whether it can start, which views are entering and leaving, and whether the entering page ends up visible. These logs are off in every build, dev included. Ionic's `logLevel` config turns them on, either through the URL: + +``` +http://localhost:3000/routing?ionic:logLevel=DEBUG +``` + +or before the app renders: + +```tsx +import { LogLevel, setupIonicReact } from '@ionic/react'; + +setupIonicReact({ logLevel: LogLevel.DEBUG }); +``` + +Refer to [the testing docs](./testing.md#debug-logging-in-e2e-runs) for how to read them in a failing e2e run. diff --git a/docs/react-router/testing.md b/docs/react-router/testing.md index 68b8c6236ce..9d12b32dd91 100644 --- a/docs/react-router/testing.md +++ b/docs/react-router/testing.md @@ -42,6 +42,15 @@ Useful flags: | `--app ` | Pick a different app variant from `packages/react-router/test/apps/` (default: `reactrouter6-react18`; use `reactrouter6-react19` for the latest supported React version) | | `--serve` | Start the dev server only and open the browser | +## Debug Logging in E2E Runs + +The test app starts with `setupIonicReact({ logLevel: LogLevel.DEBUG })`, so the `StackManager` swipe-back diagnostics are on for every spec. + +- Cypress prints the browser console to the terminal on failure, via `cypress-terminal-report`. +- Playwright records a trace on the first retry, so CI failures come with one. Open it with `npx playwright show-trace ` and read the console tab. Retries are off locally, so pass `--trace on` when you want the same thing from a local run. Don't turn tracing on by default: the recording overhead is enough to destabilize the tab lifecycle specs on React 19. + +A passing run collects the same logs in the browser and throws them away, so nothing reaches your terminal. Refer to [Debug Logging](./README.md#debug-logging) for turning them on in your own app. + ## Test App Build Structure Unlike other test applications, these test apps are broken up into multiple directories. These directories are then combined to create a single application. This allows us to share common application code, tests, etc so that each app is being tested the same way. Below details the different pieces that help create a single test application. diff --git a/packages/react-router/src/ReactRouter/StackManager.tsx b/packages/react-router/src/ReactRouter/StackManager.tsx index 7c47492a11a..a3eb9da2cfb 100644 --- a/packages/react-router/src/ReactRouter/StackManager.tsx +++ b/packages/react-router/src/ReactRouter/StackManager.tsx @@ -5,7 +5,7 @@ */ import type { RouteInfo, StackContextState, ViewItem } from '@ionic/react'; -import { IonRoute, RouteManagerContext, StackContext, generateId } from '@ionic/react'; +import { IonRoute, RouteManagerContext, StackContext, createDebugLogger, generateId } from '@ionic/react'; import React from 'react'; import type { RouteObject } from 'react-router-dom'; import { Route, UNSAFE_RouteContext as RouteContext, matchRoutes } from 'react-router-dom'; @@ -35,6 +35,9 @@ const VIEW_UNMOUNT_DELAY_MS = 250; */ const ION_PAGE_WAIT_TIMEOUT_MS = 300; +/** Off unless the app sets `logLevel: 'DEBUG'`. */ +const debug = createDebugLogger('react-router'); + interface StackManagerProps { routeInfo: RouteInfo; id?: string; @@ -47,19 +50,6 @@ const isViewVisible = (el: HTMLElement) => const hideIonPageElement = (element: HTMLElement | undefined): void => { if (element) { - if (element.id === 'section-a' || element.id === 'section-b') { - // eslint-disable-next-line no-console - console.log( - '[HideIonPageElement]', - JSON.stringify({ - id: element.id, - stack: new Error().stack - ?.split('\n') - .slice(1, 6) - .map((s) => s.trim()), - }) - ); - } element.classList.add('ion-page-hidden'); element.setAttribute('aria-hidden', 'true'); } @@ -91,32 +81,9 @@ const showIonPageElement = (element: HTMLElement | undefined): void => { */ const revealIonPageForSwipeBack = (element: HTMLElement | undefined): void => { if (element) { - const before = { - id: element.id, - inlineDisplay: element.style.display, - hasHiddenClass: element.classList.contains('ion-page-hidden'), - ariaHidden: element.getAttribute('aria-hidden'), - computedDisplay: getComputedStyle(element).display, - }; element.style.removeProperty('display'); element.classList.remove('ion-page-hidden'); element.removeAttribute('aria-hidden'); - // eslint-disable-next-line no-console - console.log( - '[SwipeBackReveal]', - JSON.stringify({ - before, - after: { - inlineDisplay: element.style.display, - hasHiddenClass: element.classList.contains('ion-page-hidden'), - ariaHidden: element.getAttribute('aria-hidden'), - computedDisplay: getComputedStyle(element).display, - }, - }) - ); - } else { - // eslint-disable-next-line no-console - console.log('[SwipeBackReveal] element is undefined'); } }; @@ -1433,20 +1400,16 @@ export class StackManager extends React.PureComponent { enteringViewItem.routeData.match.pattern.path !== routeInfo.pathname && enteringViewItem.routeData.match.pathname !== routeInfo.pathname; - // eslint-disable-next-line no-console - console.log( - '[SwipeBackCanStart]', - JSON.stringify({ - outletId: this.id, - routePathname: routeInfo.pathname, - swipeBackPathname: swipeBackRouteInfo?.pathname, - enteringViewId: enteringViewItem?.id, - enteringViewPath: enteringViewItem?.reactElement?.props?.path, - enteringMount: enteringViewItem?.mount, - ionPageInDocument, - canStartSwipe, - }) - ); + debug('SwipeBackCanStart', () => ({ + outletId: this.id, + routePathname: routeInfo.pathname, + swipeBackPathname: swipeBackRouteInfo?.pathname, + enteringViewId: enteringViewItem?.id, + enteringViewPath: enteringViewItem?.reactElement?.props?.path, + enteringMount: enteringViewItem?.mount, + ionPageInDocument, + canStartSwipe, + })); return canStartSwipe; }; @@ -1457,20 +1420,16 @@ export class StackManager extends React.PureComponent { const enteringViewItem = this.findEnteringViewForSwipe(swipeBackRouteInfo); const leavingViewItem = this.context.findViewItemByRouteInfo(routeInfo, this.id, false); - // eslint-disable-next-line no-console - console.log( - '[SwipeBackOnStart:entry]', - JSON.stringify({ - outletId: this.id, - routePathname: routeInfo.pathname, - swipeBackPathname: swipeBackRouteInfo?.pathname, - enteringViewId: enteringViewItem?.id, - enteringViewPath: enteringViewItem?.reactElement?.props?.path, - enteringMount: enteringViewItem?.mount, - hasEnteringIonPageElement: !!enteringViewItem?.ionPageElement, - leavingViewId: leavingViewItem?.id, - }) - ); + debug('SwipeBackOnStart:entry', () => ({ + outletId: this.id, + routePathname: routeInfo.pathname, + swipeBackPathname: swipeBackRouteInfo?.pathname, + enteringViewId: enteringViewItem?.id, + enteringViewPath: enteringViewItem?.reactElement?.props?.path, + enteringMount: enteringViewItem?.mount, + hasEnteringIonPageElement: !!enteringViewItem?.ionPageElement, + leavingViewId: leavingViewItem?.id, + })); // Ensure the entering view is mounted so React keeps rendering it during the gesture. // This is important when the view was previously marked for unmount but its @@ -1489,18 +1448,14 @@ export class StackManager extends React.PureComponent { await this.transitionPage(routeInfo, enteringViewItem, leavingViewItem, 'back', true); } - // eslint-disable-next-line no-console - console.log( - '[SwipeBackOnStart:exit]', - JSON.stringify({ - outletId: this.id, - enteringFinalComputedDisplay: enteringViewItem?.ionPageElement - ? getComputedStyle(enteringViewItem.ionPageElement).display - : null, - enteringFinalInlineDisplay: enteringViewItem?.ionPageElement?.style.display ?? null, - enteringFinalHiddenClass: enteringViewItem?.ionPageElement?.classList.contains('ion-page-hidden') ?? null, - }) - ); + debug('SwipeBackOnStart:exit', () => ({ + outletId: this.id, + enteringFinalComputedDisplay: enteringViewItem?.ionPageElement + ? getComputedStyle(enteringViewItem.ionPageElement).display + : null, + enteringFinalInlineDisplay: enteringViewItem?.ionPageElement?.style.display ?? null, + enteringFinalHiddenClass: enteringViewItem?.ionPageElement?.classList.contains('ion-page-hidden') ?? null, + })); return Promise.resolve(); }; diff --git a/packages/react-router/test/base/src/App.tsx b/packages/react-router/test/base/src/App.tsx index 09e61e940bd..1e26697edeb 100644 --- a/packages/react-router/test/base/src/App.tsx +++ b/packages/react-router/test/base/src/App.tsx @@ -1,4 +1,4 @@ -import { IonApp, setupIonicReact, IonRouterOutlet } from '@ionic/react'; +import { IonApp, setupIonicReact, LogLevel, IonRouterOutlet } from '@ionic/react'; import React from 'react'; import { Route, Navigate } from 'react-router-dom'; @@ -72,7 +72,8 @@ import SuspenseOutlet from './pages/suspense-outlet/SuspenseOutlet'; import { PropsUpdateDirect, PropsUpdateRoutesWrapper } from './pages/props-update/PropsUpdate'; import DisabledButton from './pages/disabled-button/DisabledButton'; -setupIonicReact(); +// Debug logs on so failing specs include the navigation diagnostics. +setupIonicReact({ logLevel: LogLevel.DEBUG }); const App: React.FC = () => { return ( diff --git a/packages/react/src/components/IonIcon.tsx b/packages/react/src/components/IonIcon.tsx index cf02827e838..b255e043a97 100644 --- a/packages/react/src/components/IonIcon.tsx +++ b/packages/react/src/components/IonIcon.tsx @@ -1,10 +1,11 @@ import React from 'react'; import { NavContext } from '../contexts/NavContext'; +import { getConfig } from '../utils/config'; import type { IonicReactProps } from './IonicReactProps'; import { IonIconInner } from './inner-proxies'; -import { createForwardRef, getConfig } from './utils'; +import { createForwardRef } from './utils'; interface IonIconProps { color?: string; diff --git a/packages/react/src/components/index.ts b/packages/react/src/components/index.ts index 7b079159962..cec98f1789a 100644 --- a/packages/react/src/components/index.ts +++ b/packages/react/src/components/index.ts @@ -11,6 +11,7 @@ export { getTimeGivenProgression, getIonPageElement, openURL, + LogLevel, // TYPES Animation, @@ -124,7 +125,8 @@ export * from './IonRoute'; export * from './IonRouterContext'; // Utils -export { isPlatform, getPlatforms, getConfig } from './utils'; +export { isPlatform, getPlatforms } from './utils'; +export { getConfig } from '../utils/config'; export * from './hrefprops'; // Ionic Animations diff --git a/packages/react/src/components/utils/index.tsx b/packages/react/src/components/utils/index.tsx index 2cd3c99bb70..358494c776c 100644 --- a/packages/react/src/components/utils/index.tsx +++ b/packages/react/src/components/utils/index.tsx @@ -1,4 +1,4 @@ -import type { Config as CoreConfig, Platforms } from '@ionic/core/components'; +import type { Platforms } from '@ionic/core/components'; import { getPlatforms as getPlatformsCore, isPlatform as isPlatformCore } from '@ionic/core/components'; import React from 'react'; @@ -39,13 +39,3 @@ export const isPlatform = (platform: Platforms) => { export const getPlatforms = () => { return getPlatformsCore(window); }; - -export const getConfig = (): CoreConfig | null => { - if (typeof (window as any) !== 'undefined') { - const Ionic = (window as any).Ionic; - if (Ionic && Ionic.config) { - return Ionic.config; - } - } - return null; -}; diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index 54495d0ae5b..69ebe615bd9 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -5,3 +5,4 @@ export * from './components'; export * from './routing'; export * from './models'; export * from './utils/generateId'; +export * from './utils/debug'; diff --git a/packages/react/src/utils/__tests__/debug.spec.ts b/packages/react/src/utils/__tests__/debug.spec.ts new file mode 100644 index 00000000000..c2b277c5c4f --- /dev/null +++ b/packages/react/src/utils/__tests__/debug.spec.ts @@ -0,0 +1,113 @@ +import { createDebugLogger } from '../debug'; + +/** Core swaps `Ionic.config` for a `Config` instance in `initialize()`, so a `get` stub is enough here. */ +const setLogLevel = (logLevel?: string) => { + (window as any).Ionic = { config: { get: (key: string) => (key === 'logLevel' ? logLevel : undefined) } }; +}; + +describe('debug logging', () => { + let consoleLogSpy: jest.SpyInstance; + let debug: ReturnType; + + beforeEach(() => { + consoleLogSpy = jest.spyOn(console, 'log').mockImplementation(() => undefined); + delete (window as any).Ionic; + debug = createDebugLogger('react-router'); + }); + + afterEach(() => { + consoleLogSpy.mockRestore(); + delete (window as any).Ionic; + }); + + describe('gating', () => { + it('should stay silent before Ionic has initialized', () => { + debug('SomeEvent'); + + expect(consoleLogSpy).not.toHaveBeenCalled(); + }); + + it('should stay silent at the default log level', () => { + setLogLevel(undefined); + + debug('SomeEvent'); + + expect(consoleLogSpy).not.toHaveBeenCalled(); + }); + + it('should stay silent at log levels below DEBUG', () => { + ['OFF', 'ERROR', 'WARN'].forEach((logLevel) => { + setLogLevel(logLevel); + + debug('SomeEvent'); + + expect(consoleLogSpy).not.toHaveBeenCalled(); + }); + }); + + it('should log at the DEBUG log level', () => { + setLogLevel('DEBUG'); + + debug('SomeEvent'); + + expect(consoleLogSpy).toHaveBeenCalled(); + }); + + it('should accept a log level in any casing, since query parameters are raw strings', () => { + setLogLevel('debug'); + + debug('SomeEvent'); + + expect(consoleLogSpy).toHaveBeenCalled(); + }); + + it('should track the log level changing after the logger is created', () => { + setLogLevel('WARN'); + debug('SomeEvent'); + expect(consoleLogSpy).not.toHaveBeenCalled(); + + setLogLevel('DEBUG'); + debug('SomeEvent'); + expect(consoleLogSpy).toHaveBeenCalled(); + }); + }); + + describe('output', () => { + it('should not build a payload while disabled', () => { + const getData = jest.fn(() => ({ some: 'data' })); + + debug('SomeEvent', getData); + + expect(getData).not.toHaveBeenCalled(); + }); + + it('should log a namespaced event with its serialized payload', () => { + setLogLevel('DEBUG'); + + debug('SomeEvent', () => ({ some: 'data' })); + + expect(consoleLogSpy).toHaveBeenCalledWith('[Ionic Debug]: [react-router] - SomeEvent', '{"some":"data"}'); + }); + + it('should log events that carry no payload', () => { + setLogLevel('DEBUG'); + + debug('SomeEvent'); + + expect(consoleLogSpy).toHaveBeenCalledWith('[Ionic Debug]: [react-router] - SomeEvent'); + }); + + it('should keep logging when a payload cannot be serialized', () => { + setLogLevel('DEBUG'); + const circular: any = {}; + circular.self = circular; + + debug('SomeEvent', () => circular); + + expect(consoleLogSpy).toHaveBeenCalledWith( + '[Ionic Debug]: [react-router] - SomeEvent', + '[unserializable payload]' + ); + }); + }); +}); diff --git a/packages/react/src/utils/config.ts b/packages/react/src/utils/config.ts new file mode 100644 index 00000000000..5dfbd09e35e --- /dev/null +++ b/packages/react/src/utils/config.ts @@ -0,0 +1,16 @@ +import type { Config as CoreConfig } from '@ionic/core/components'; + +/** + * Ionic's global config, or `null` before core has initialized. Kept here rather + * than alongside the component helpers so that utils reaching for config don't + * pull core's runtime bundle in with it. + */ +export const getConfig = (): CoreConfig | null => { + if (typeof (window as any) !== 'undefined') { + const Ionic = (window as any).Ionic; + if (Ionic && Ionic.config) { + return Ionic.config; + } + } + return null; +}; diff --git a/packages/react/src/utils/debug.ts b/packages/react/src/utils/debug.ts new file mode 100644 index 00000000000..3d8d1e792b7 --- /dev/null +++ b/packages/react/src/utils/debug.ts @@ -0,0 +1,49 @@ +import type { LogLevel } from '@ionic/core/components'; + +import { getConfig } from './config'; + +/** + * Spelled out instead of imported as a value so this util doesn't pull core's + * runtime into unit tests. The template type still breaks the build on a rename. + */ +const DEBUG: `${LogLevel.DEBUG}` = 'DEBUG'; + +/** Whether the app opted into debug logging with `logLevel: 'DEBUG'`. */ +const isDebugEnabled = (): boolean => { + return String(getConfig()?.get('logLevel') ?? '').toUpperCase() === DEBUG; +}; + +/** @internal */ +export type DebugLogger = (event: string, getData?: () => unknown) => void; + +/** + * Logger namespaced to a package, e.g. `react-router`. The payload is a function + * so nothing it collects runs while logging is off. + * + * @internal + */ +export const createDebugLogger = (namespace: string): DebugLogger => { + return (event, getData) => { + if (!isDebugEnabled()) { + return; + } + + const prefix = `[Ionic Debug]: [${namespace}] - ${event}`; + const data = getData?.(); + + if (data === undefined) { + console.log(prefix); + return; + } + + let serialized: string; + try { + serialized = JSON.stringify(data); + } catch { + // A circular reference in the payload must not break navigation. + serialized = '[unserializable payload]'; + } + + console.log(prefix, serialized); + }; +};