Skip to content

Commit

Permalink
create useWarningLogger hook (#2213)
Browse files Browse the repository at this point in the history
  • Loading branch information
mayank99 authored Aug 29, 2024
1 parent f58ae5b commit 6fbef5e
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 40 deletions.
5 changes: 5 additions & 0 deletions .changeset/mighty-spiders-repair.md
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 3 additions & 3 deletions packages/itwinui-react/src/core/Breadcrumbs/Breadcrumbs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -198,6 +196,8 @@ const ListItem = ({
}) => {
let children = item as any;

const logWarning = useWarningLogger();

if (
children?.type === 'span' ||
children?.type === 'a' ||
Expand Down
6 changes: 3 additions & 3 deletions packages/itwinui-react/src/core/Buttons/IconButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,14 @@
*--------------------------------------------------------------------------------------------*/
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';
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.
Expand Down Expand Up @@ -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 &&
Expand Down
6 changes: 3 additions & 3 deletions packages/itwinui-react/src/core/Menu/MenuItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,14 @@ import {
SvgCaretRightSmall,
useMergedRefs,
useId,
createWarningLogger,
useWarningLogger,
} from '../../utils/index.js';
import type { PolymorphicForwardRefComponent } from '../../utils/index.js';
import { Menu, MenuContext } from './Menu.js';
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.
Expand Down Expand Up @@ -100,6 +98,8 @@ export const MenuItem = React.forwardRef((props, forwardedRef) => {
...rest
} = props;

const logWarning = useWarningLogger();

if (
process.env.NODE_ENV === 'development' &&
onClickProp != null &&
Expand Down
6 changes: 3 additions & 3 deletions packages/itwinui-react/src/core/Table/Table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
useResizeObserver,
useLayoutEffect,
Box,
createWarningLogger,
useWarningLogger,
ShadowRoot,
useMergedRefs,
useLatestRef,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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') {
Expand Down
29 changes: 1 addition & 28 deletions packages/itwinui-react/src/utils/functions/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
1 change: 1 addition & 0 deletions packages/itwinui-react/src/utils/hooks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ export * from './useId.js';
export * from './useControlledState.js';
export * from './useSyncExternalStore.js';
export * from './useVirtualScroll.js';
export * from './useWarningLogger.js';
69 changes: 69 additions & 0 deletions packages/itwinui-react/src/utils/hooks/useWarningLogger.ts
Original file line number Diff line number Diff line change
@@ -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<number | null>(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;
}
: () => () => {};

0 comments on commit 6fbef5e

Please sign in to comment.