From 2b65bf755f5fd0613cd14e7b18c02bd2e27541d0 Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Thu, 20 Jul 2023 15:00:37 +0300
Subject: [PATCH] Fix passing invalid props to Icon and Link (#32527)
---
frontend/src/metabase/components/Badge/Badge.jsx | 2 +-
frontend/src/metabase/components/Badge/Badge.styled.tsx | 7 ++-----
.../src/metabase/components/tree/TreeNode.styled.tsx | 5 +----
.../components/BookmarkToggle/BookmarkToggle.styled.tsx | 5 +----
frontend/src/metabase/core/components/Icon/Icon.tsx | 8 ++++++--
.../src/metabase/core/components/Link/Link.styled.tsx | 5 ++---
frontend/src/metabase/lib/styling/emotion.ts | 5 -----
.../nav/components/AppBar/AppBarToggle.styled.tsx | 5 +----
.../src/metabase/nav/components/SearchBar.styled.tsx | 5 +----
.../MainNavbar/SidebarItems/SidebarItems.styled.tsx | 9 +++------
package.json | 1 +
11 files changed, 19 insertions(+), 38 deletions(-)
delete mode 100644 frontend/src/metabase/lib/styling/emotion.ts
diff --git a/frontend/src/metabase/components/Badge/Badge.jsx b/frontend/src/metabase/components/Badge/Badge.jsx
index 85735265525..f14a6f34b1f 100644
--- a/frontend/src/metabase/components/Badge/Badge.jsx
+++ b/frontend/src/metabase/components/Badge/Badge.jsx
@@ -42,7 +42,7 @@ function Badge({
isSingleLine={isSingleLine}
{...props}
>
- {icon && <BadgeIcon {...getIconProps(icon)} $hasMargin={!!children} />}
+ {icon && <BadgeIcon {...getIconProps(icon)} hasMargin={!!children} />}
{children && (
<BadgeText className="text-wrap" isSingleLine={isSingleLine}>
{children}
diff --git a/frontend/src/metabase/components/Badge/Badge.styled.tsx b/frontend/src/metabase/components/Badge/Badge.styled.tsx
index 8656e2d9454..a91a136992d 100644
--- a/frontend/src/metabase/components/Badge/Badge.styled.tsx
+++ b/frontend/src/metabase/components/Badge/Badge.styled.tsx
@@ -4,7 +4,6 @@ import { HTMLAttributes } from "react";
import { color } from "metabase/lib/colors";
import { Icon } from "metabase/core/components/Icon";
import Link, { LinkProps } from "metabase/core/components/Link";
-import { shouldForwardNonTransientProp } from "metabase/lib/styling/emotion";
interface RawMaybeLinkProps {
to?: string;
@@ -41,10 +40,8 @@ export const MaybeLink = styled(RawMaybeLink)`
}
`;
-export const BadgeIcon = styled(Icon, {
- shouldForwardProp: shouldForwardNonTransientProp,
-})<{ $hasMargin: boolean }>`
- margin-right: ${props => (props.$hasMargin ? "5px" : 0)};
+export const BadgeIcon = styled(Icon)<{ hasMargin: boolean }>`
+ margin-right: ${props => (props.hasMargin ? "5px" : 0)};
`;
export const BadgeText = styled.span<{ isSingleLine: boolean }>`
diff --git a/frontend/src/metabase/components/tree/TreeNode.styled.tsx b/frontend/src/metabase/components/tree/TreeNode.styled.tsx
index 5a78b404595..92869c4a5ee 100644
--- a/frontend/src/metabase/components/tree/TreeNode.styled.tsx
+++ b/frontend/src/metabase/components/tree/TreeNode.styled.tsx
@@ -2,7 +2,6 @@ import styled from "@emotion/styled";
import { css } from "@emotion/react";
import { color, lighten } from "metabase/lib/colors";
import { Icon, IconProps } from "metabase/core/components/Icon";
-import { shouldForwardNonTransientProp } from "metabase/lib/styling/emotion";
interface TreeNodeRootProps {
isSelected: boolean;
@@ -38,9 +37,7 @@ interface ExpandToggleIconProps {
isExpanded: boolean;
}
-export const ExpandToggleIcon = styled(Icon, {
- shouldForwardProp: shouldForwardNonTransientProp,
-})<ExpandToggleIconProps & IconProps>`
+export const ExpandToggleIcon = styled(Icon)<ExpandToggleIconProps & IconProps>`
transition: transform 200ms;
${props =>
diff --git a/frontend/src/metabase/core/components/BookmarkToggle/BookmarkToggle.styled.tsx b/frontend/src/metabase/core/components/BookmarkToggle/BookmarkToggle.styled.tsx
index f1209a3da10..971496afbeb 100644
--- a/frontend/src/metabase/core/components/BookmarkToggle/BookmarkToggle.styled.tsx
+++ b/frontend/src/metabase/core/components/BookmarkToggle/BookmarkToggle.styled.tsx
@@ -3,7 +3,6 @@ import { keyframes } from "@emotion/react";
import { color } from "metabase/lib/colors";
import { Icon } from "metabase/core/components/Icon";
import Button from "metabase/core/components/Button";
-import { shouldForwardNonTransientProp } from "metabase/lib/styling/emotion";
const expandKeyframes = keyframes`
50% {
@@ -23,9 +22,7 @@ export interface BookmarkIconProps {
onAnimationEnd: () => void;
}
-export const BookmarkIcon = styled(Icon, {
- shouldForwardProp: shouldForwardNonTransientProp,
-})<BookmarkIconProps>`
+export const BookmarkIcon = styled(Icon)<BookmarkIconProps>`
color: ${props => (props.isBookmarked ? color("brand") : "")};
animation-name: ${props =>
props.isBookmarked ? expandKeyframes : shrinkKeyframes};
diff --git a/frontend/src/metabase/core/components/Icon/Icon.tsx b/frontend/src/metabase/core/components/Icon/Icon.tsx
index f9c3d2626f8..aa6194c34ca 100644
--- a/frontend/src/metabase/core/components/Icon/Icon.tsx
+++ b/frontend/src/metabase/core/components/Icon/Icon.tsx
@@ -1,4 +1,5 @@
import React, { SVGAttributes, forwardRef } from "react";
+import isPropValid from "@emotion/is-prop-valid";
import cx from "classnames";
import Tooltip from "../Tooltip";
import { Icons } from "./icons";
@@ -15,10 +16,13 @@ export interface IconProps extends SVGAttributes<SVGSVGElement> {
}
export const Icon = forwardRef<SVGSVGElement, IconProps>(function Icon(
- { name, className, size = defaultSize, tooltip, ...rest }: IconProps,
+ { name, className, size = defaultSize, tooltip, ...restProps }: IconProps,
ref,
) {
const IconComponent = (Icons[name] ?? Icons["unknown"]).component;
+ const validProps = Object.fromEntries(
+ Object.entries(restProps).filter(([key]) => isPropValid(key)),
+ );
const icon = (
<IconComponent
@@ -28,7 +32,7 @@ export const Icon = forwardRef<SVGSVGElement, IconProps>(function Icon(
className={cx(`Icon Icon-${name}`, className)}
width={size}
height={size}
- {...rest}
+ {...validProps}
/>
);
diff --git a/frontend/src/metabase/core/components/Link/Link.styled.tsx b/frontend/src/metabase/core/components/Link/Link.styled.tsx
index 9ce7312c50e..78a8b4ac56b 100644
--- a/frontend/src/metabase/core/components/Link/Link.styled.tsx
+++ b/frontend/src/metabase/core/components/Link/Link.styled.tsx
@@ -1,14 +1,13 @@
import styled from "@emotion/styled";
import { css } from "@emotion/react";
+import isPropValid from "@emotion/is-prop-valid";
import { Link } from "react-router";
import { color as metabaseColor } from "metabase/lib/colors";
-import { shouldForwardNonTransientProp } from "metabase/lib/styling/emotion";
import { focusOutlineStyle } from "metabase/core/style/input";
-
import type { LinkProps } from "./types";
export const LinkRoot = styled(Link, {
- shouldForwardProp: shouldForwardNonTransientProp,
+ shouldForwardProp: isPropValid,
})<LinkProps>`
opacity: ${props => (props.disabled ? "0.4" : "")};
pointer-events: ${props => (props.disabled ? "none" : "")};
diff --git a/frontend/src/metabase/lib/styling/emotion.ts b/frontend/src/metabase/lib/styling/emotion.ts
deleted file mode 100644
index 9fd3588ba7b..00000000000
--- a/frontend/src/metabase/lib/styling/emotion.ts
+++ /dev/null
@@ -1,5 +0,0 @@
-import isPropValid from "@emotion/is-prop-valid";
-
-export function shouldForwardNonTransientProp(propName: string): boolean {
- return !propName.startsWith("$") && isPropValid(propName);
-}
diff --git a/frontend/src/metabase/nav/components/AppBar/AppBarToggle.styled.tsx b/frontend/src/metabase/nav/components/AppBar/AppBarToggle.styled.tsx
index 78e7c4adfea..46362fd17aa 100644
--- a/frontend/src/metabase/nav/components/AppBar/AppBarToggle.styled.tsx
+++ b/frontend/src/metabase/nav/components/AppBar/AppBarToggle.styled.tsx
@@ -2,7 +2,6 @@ import styled from "@emotion/styled";
import { css } from "@emotion/react";
import { color } from "metabase/lib/colors";
import { Icon } from "metabase/core/components/Icon";
-import { shouldForwardNonTransientProp } from "metabase/lib/styling/emotion";
import { AppBarLeftContainer } from "./AppBarLarge.styled";
interface SidebarButtonProps {
@@ -33,9 +32,7 @@ interface SidebarIconProps {
isLogoVisible?: boolean;
}
-export const SidebarIcon = styled(Icon, {
- shouldForwardProp: shouldForwardNonTransientProp,
-})<SidebarIconProps>`
+export const SidebarIcon = styled(Icon)<SidebarIconProps>`
color: ${color("brand")};
display: block;
transform: translateY(2px) translateX(2px);
diff --git a/frontend/src/metabase/nav/components/SearchBar.styled.tsx b/frontend/src/metabase/nav/components/SearchBar.styled.tsx
index 739c5c3b5eb..9aebeaef39b 100644
--- a/frontend/src/metabase/nav/components/SearchBar.styled.tsx
+++ b/frontend/src/metabase/nav/components/SearchBar.styled.tsx
@@ -11,7 +11,6 @@ import {
breakpointMaxSmall,
breakpointMinSmall,
} from "metabase/styled-components/theme";
-import { shouldForwardNonTransientProp } from "metabase/lib/styling/emotion";
const activeInputCSS = css`
border-radius: 6px;
@@ -106,9 +105,7 @@ export const SearchInput = styled.input<{ isActive: boolean }>`
const ICON_MARGIN = "10px";
-export const SearchIcon = styled(Icon, {
- shouldForwardProp: shouldForwardNonTransientProp,
-})<{ isActive: boolean }>`
+export const SearchIcon = styled(Icon)<{ isActive: boolean }>`
${breakpointMaxSmall} {
transition: margin 0.3s;
diff --git a/frontend/src/metabase/nav/containers/MainNavbar/SidebarItems/SidebarItems.styled.tsx b/frontend/src/metabase/nav/containers/MainNavbar/SidebarItems/SidebarItems.styled.tsx
index 273d0b1e625..9ae72d50a3a 100644
--- a/frontend/src/metabase/nav/containers/MainNavbar/SidebarItems/SidebarItems.styled.tsx
+++ b/frontend/src/metabase/nav/containers/MainNavbar/SidebarItems/SidebarItems.styled.tsx
@@ -8,12 +8,9 @@ import Link from "metabase/core/components/Link";
import { NAV_SIDEBAR_WIDTH } from "metabase/nav/constants";
-import { darken, color, alpha } from "metabase/lib/colors";
-import { shouldForwardNonTransientProp } from "metabase/lib/styling/emotion";
+import { alpha, color, darken } from "metabase/lib/colors";
-export const SidebarIcon = styled(Icon, {
- shouldForwardProp: shouldForwardNonTransientProp,
-})<{
+export const SidebarIcon = styled(Icon)<{
color?: string | null;
isSelected: boolean;
}>`
@@ -91,8 +88,8 @@ const itemContentStyle = css`
export const FullWidthButton = styled.button<{ isSelected: boolean }>`
cursor: pointer;
- ${itemContentStyle}
+ ${itemContentStyle}
${TreeNode.NameContainer} {
font-weight: 700;
color: ${props => getTextColor(props.isSelected)};
diff --git a/package.json b/package.json
index 91602956455..f074fbb1926 100644
--- a/package.json
+++ b/package.json
@@ -13,6 +13,7 @@
"@dnd-kit/core": "^6.0.8",
"@dnd-kit/modifiers": "^6.0.1",
"@dnd-kit/sortable": "^7.0.2",
+ "@emotion/is-prop-valid": "^1.1.1",
"@emotion/react": "^11.7.1",
"@emotion/styled": "^11.6.0",
"@mantine/core": "^6.0.13",
--
GitLab