From 6fbef5eeebe8a73cf1d1666fbeed37b82f644bd1 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Thu, 29 Aug 2024 12:49:25 -0400 Subject: [PATCH] create `useWarningLogger` hook (#2213) --- .changeset/mighty-spiders-repair.md | 5 ++ .../src/core/Breadcrumbs/Breadcrumbs.tsx | 6 +- .../src/core/Buttons/IconButton.tsx | 6 +- .../itwinui-react/src/core/Menu/MenuItem.tsx | 6 +- .../itwinui-react/src/core/Table/Table.tsx | 6 +- .../itwinui-react/src/utils/functions/dev.ts | 29 +------- .../itwinui-react/src/utils/hooks/index.ts | 1 + .../src/utils/hooks/useWarningLogger.ts | 69 +++++++++++++++++++ 8 files changed, 88 insertions(+), 40 deletions(-) create mode 100644 .changeset/mighty-spiders-repair.md create mode 100644 packages/itwinui-react/src/utils/hooks/useWarningLogger.ts diff --git a/.changeset/mighty-spiders-repair.md b/.changeset/mighty-spiders-repair.md new file mode 100644 index 00000000000..fa720164c37 --- /dev/null +++ b/.changeset/mighty-spiders-repair.md @@ -0,0 +1,5 @@ +--- +'@itwin/itwinui-react': patch +--- + +Dev-only warnings have been improved so that they are correctly shown for every individual instance of a component. Additionally, these warnings are now logged using `console.error`, which results in a better stack trace. diff --git a/packages/itwinui-react/src/core/Breadcrumbs/Breadcrumbs.tsx b/packages/itwinui-react/src/core/Breadcrumbs/Breadcrumbs.tsx index 6f2c4ab1d52..82b09fe528a 100644 --- a/packages/itwinui-react/src/core/Breadcrumbs/Breadcrumbs.tsx +++ b/packages/itwinui-react/src/core/Breadcrumbs/Breadcrumbs.tsx @@ -9,14 +9,12 @@ import { useOverflow, SvgChevronRight, Box, - createWarningLogger, + useWarningLogger, } from '../../utils/index.js'; import type { PolymorphicForwardRefComponent } from '../../utils/index.js'; import { Button } from '../Buttons/Button.js'; import { Anchor } from '../Typography/Anchor.js'; -const logWarning = createWarningLogger(); - type BreadcrumbsProps = { /** * Index of the currently active breadcrumb. @@ -198,6 +196,8 @@ const ListItem = ({ }) => { let children = item as any; + const logWarning = useWarningLogger(); + if ( children?.type === 'span' || children?.type === 'a' || diff --git a/packages/itwinui-react/src/core/Buttons/IconButton.tsx b/packages/itwinui-react/src/core/Buttons/IconButton.tsx index 49c33f4c61a..cab108a92ce 100644 --- a/packages/itwinui-react/src/core/Buttons/IconButton.tsx +++ b/packages/itwinui-react/src/core/Buttons/IconButton.tsx @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import cx from 'classnames'; import * as React from 'react'; -import { Box, ButtonBase, createWarningLogger } from '../../utils/index.js'; +import { Box, ButtonBase, useWarningLogger } from '../../utils/index.js'; import { Tooltip } from '../Tooltip/Tooltip.js'; import type { ButtonProps } from './Button.js'; import type { PolymorphicForwardRefComponent } from '../../utils/index.js'; @@ -12,8 +12,6 @@ import { VisuallyHidden } from '../VisuallyHidden/VisuallyHidden.js'; import { ButtonGroupContext } from '../ButtonGroup/ButtonGroup.js'; import { PopoverOpenContext } from '../Popover/Popover.js'; -const logWarning = createWarningLogger(); - export type IconButtonProps = { /** * Button gets active style. @@ -74,6 +72,8 @@ export const IconButton = React.forwardRef((props, ref) => { const buttonGroupOrientation = React.useContext(ButtonGroupContext); const hasPopoverOpen = React.useContext(PopoverOpenContext); + const logWarning = useWarningLogger(); + if ( process.env.NODE_ENV === 'development' && !label && diff --git a/packages/itwinui-react/src/core/Menu/MenuItem.tsx b/packages/itwinui-react/src/core/Menu/MenuItem.tsx index a6c51494f04..e8cada2c510 100644 --- a/packages/itwinui-react/src/core/Menu/MenuItem.tsx +++ b/packages/itwinui-react/src/core/Menu/MenuItem.tsx @@ -7,7 +7,7 @@ import { SvgCaretRightSmall, useMergedRefs, useId, - createWarningLogger, + useWarningLogger, } from '../../utils/index.js'; import type { PolymorphicForwardRefComponent } from '../../utils/index.js'; import { Menu, MenuContext } from './Menu.js'; @@ -15,8 +15,6 @@ import { ListItem } from '../List/ListItem.js'; import type { ListItemOwnProps } from '../List/ListItem.js'; import cx from 'classnames'; -const logWarning = createWarningLogger(); - export type MenuItemProps = { /** * Item is selected. @@ -100,6 +98,8 @@ export const MenuItem = React.forwardRef((props, forwardedRef) => { ...rest } = props; + const logWarning = useWarningLogger(); + if ( process.env.NODE_ENV === 'development' && onClickProp != null && diff --git a/packages/itwinui-react/src/core/Table/Table.tsx b/packages/itwinui-react/src/core/Table/Table.tsx index e862db488c3..db6f65e1dea 100644 --- a/packages/itwinui-react/src/core/Table/Table.tsx +++ b/packages/itwinui-react/src/core/Table/Table.tsx @@ -32,7 +32,7 @@ import { useResizeObserver, useLayoutEffect, Box, - createWarningLogger, + useWarningLogger, ShadowRoot, useMergedRefs, useLatestRef, @@ -71,8 +71,6 @@ const shiftRowSelectedAction = 'shiftRowSelected'; export const tableResizeStartAction = 'tableResizeStart'; const tableResizeEndAction = 'tableResizeEnd'; -const logWarning = createWarningLogger(); - export type TablePaginatorRendererProps = { /** * The zero-based index of the current page. @@ -631,6 +629,8 @@ export const Table = < let headerGroups = _headerGroups; + const logWarning = useWarningLogger(); + if (columns.length === 1 && 'columns' in columns[0]) { headerGroups = _headerGroups.slice(1); if (process.env.NODE_ENV === 'development') { diff --git a/packages/itwinui-react/src/utils/functions/dev.ts b/packages/itwinui-react/src/utils/functions/dev.ts index 0fe7c0271c5..32a5fc3a066 100644 --- a/packages/itwinui-react/src/utils/functions/dev.ts +++ b/packages/itwinui-react/src/utils/functions/dev.ts @@ -14,31 +14,4 @@ const isVitest = typeof (globalThis as any).__vitest_index__ !== 'undefined'; const isUnitTest = isJest || isVitest || isMocha; -/** - * Returns a function that can be used to log one-time warnings in dev environments. - * - * **Note**: The actual log call should be wrapped in a check against `process.env.NODE_ENV === 'development'` - * to ensure that it is removed from the production build output (by SWC). - * Read more about the [`NODE_ENV` convention](https://nodejs.org/en/learn/getting-started/nodejs-the-difference-between-development-and-production). - * - * @example - * const logWarning = createWarningLogger(); - * - * if (process.env.NODE_ENV === 'development') { - * logWarning("please don't use this") - * } - */ -const createWarningLogger = - process.env.NODE_ENV === 'development' && !isUnitTest - ? () => { - let logged = false; - return (message: string) => { - if (!logged) { - console.warn(message); - logged = true; - } - }; - } - : () => () => {}; - -export { isUnitTest, createWarningLogger }; +export { isUnitTest }; diff --git a/packages/itwinui-react/src/utils/hooks/index.ts b/packages/itwinui-react/src/utils/hooks/index.ts index c6d0da43656..03d8878ed1d 100644 --- a/packages/itwinui-react/src/utils/hooks/index.ts +++ b/packages/itwinui-react/src/utils/hooks/index.ts @@ -18,3 +18,4 @@ export * from './useId.js'; export * from './useControlledState.js'; export * from './useSyncExternalStore.js'; export * from './useVirtualScroll.js'; +export * from './useWarningLogger.js'; diff --git a/packages/itwinui-react/src/utils/hooks/useWarningLogger.ts b/packages/itwinui-react/src/utils/hooks/useWarningLogger.ts new file mode 100644 index 00000000000..51fe53f6764 --- /dev/null +++ b/packages/itwinui-react/src/utils/hooks/useWarningLogger.ts @@ -0,0 +1,69 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Bentley Systems, Incorporated. All rights reserved. + * See LICENSE.md in the project root for license terms and full copyright notice. + *--------------------------------------------------------------------------------------------*/ + +import * as React from 'react'; +import { isUnitTest } from '../functions/dev.js'; + +const _React = React as any; +const ReactInternals = + _React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED; + +/** + * Returns a function that can be used to log one-time warnings in dev environments. + * + * **Note**: The actual log call should be wrapped in a check against `process.env.NODE_ENV === 'development'` + * to ensure that it is removed from the production build output (by SWC). + * Read more about the [`NODE_ENV` convention](https://nodejs.org/en/learn/getting-started/nodejs-the-difference-between-development-and-production). + * + * @example + * const logWarning = useWarningLogger(); + * + * if (process.env.NODE_ENV === 'development') { + * logWarning("please don't use this") + * } + */ +export const useWarningLogger = + process.env.NODE_ENV === 'development' && !isUnitTest + ? function () { + const loggedRef = React.useRef(false); + const timeoutRef = React.useRef(null); + + // https://stackoverflow.com/a/71685253 + const stack = + ReactInternals?.ReactDebugCurrentFrame?.getCurrentStack?.(); + + // Second line in the stack is the component name. + const componentName = stack?.trim().split('\n')[1]?.trim(); + + const prefix = componentName + ? `Warning (${componentName}):` + : 'Warning:'; + + const logWarning = React.useCallback( + (message: string) => { + // Using setTimeout to delay execution until after rendering is complete. + timeoutRef.current = window.setTimeout(() => { + if (!loggedRef.current) { + console.error(prefix, message); + loggedRef.current = true; + } + }); + }, + [prefix], + ); + + React.useEffect(() => { + // Clearing timeout on unmount to avoid double execution in StrictMode. + // The warning should be logged only once per component instance. + return () => { + if (timeoutRef.current) { + window.clearTimeout(timeoutRef.current); + } + }; + }, []); + + return logWarning; + } + : () => () => {};