Fix and improve the types of <Button> (#36680)

This modifies the prop types of `<Button>` so that:
- The types for links formatted as buttons are treated separately than the ones for standard button. In particular I renamed `onClick` to `onClickOfAnchorLink` for links to avoid people setting both `href` and `onClick` by mistake.
- We avoid exposing all HTML button attributes and use an allowlist instead. This avoids some annoying cases (e.g. we were setting the `color` property somewhere which is valid HTML but definitely now that we want to do)
- We avoid writing `{...(props as any)}` which breaks type safety (this was causing the passHref type bugs!)

GitOrigin-RevId: 004fd1b2fcd9730e5f940507d020b8c880f77c10

Author: Nicolapps

npm-packages/@convex-dev/design-system/src/Button.tsx

@@ -16,14 +16,34 @@ export type ButtonProps = {
   disabled?: boolean;
   tip?: React.ReactNode;
   tipSide?: TooltipSide;
-} & (
-  | React.ButtonHTMLAttributes<HTMLButtonElement>
-  | {
-      href: React.AnchorHTMLAttributes<HTMLAnchorElement>["href"] | UrlObject;
-      onClick?: React.AnchorHTMLAttributes<HTMLAnchorElement>["onClick"];
-      target?: React.AnchorHTMLAttributes<HTMLAnchorElement>["target"];
-    }
-);
+} & Pick<
+  React.HTMLProps<HTMLElement>,
+  | "tabIndex"
+  | "role"
+  | "aria-label"
+  | "style"
+  | "title"
+  | "onMouseOver"
+  | "onKeyDown"
+> &
+  (
+    | {
+        href?: never;
+        onClick?: React.ButtonHTMLAttributes<HTMLButtonElement>["onClick"];
+        onClickOfAnchorLink?: never;
+        type?: React.ButtonHTMLAttributes<HTMLButtonElement>["type"];
+        target?: never;
+      }
+    | {
+        href: React.AnchorHTMLAttributes<HTMLAnchorElement>["href"] | UrlObject;
+        onClick?: never;
+        // In most cases you shouldn’t use this. This is only useful when you
+        // need the event sent before the native link behavior is handled.
+        onClickOfAnchorLink?: React.AnchorHTMLAttributes<HTMLAnchorElement>["onClick"];
+        type?: never;
+        target?: React.AnchorHTMLAttributes<HTMLAnchorElement>["target"];
+      }
+  );
 
 export const Button = forwardRef<HTMLElement, ButtonProps>(function Button(
   {
@@ -42,7 +62,7 @@ export const Button = forwardRef<HTMLElement, ButtonProps>(function Button(
   ref,
 ) {
   const Link = useContext(UIContext);
-  const { href, onClick, target, type } =
+  const { href, onClick, target, type, onClickOfAnchorLink, ...htmlProps } =
     "href" in props
       ? { ...props, type: undefined }
       : { ...props, href: undefined, target: undefined };
@@ -64,13 +84,14 @@ export const Button = forwardRef<HTMLElement, ButtonProps>(function Button(
         <Link
           passHref
           href={href}
+          // There is something weird here with `forwardRef`, I’d expect this to work without `any`
           ref={ref as any}
           role="link"
           className={buttonClassName}
           tabIndex={0}
           target={target}
-          onClick={onClick}
-          {...(props as any)}
+          onClick={onClickOfAnchorLink}
+          {...htmlProps}
         >
           {icon && <div>{icon}</div>}
           {children}
@@ -86,11 +107,12 @@ export const Button = forwardRef<HTMLElement, ButtonProps>(function Button(
         // eslint-disable-next-line react/button-has-type
         type={type ?? "button"}
         tabIndex={0}
-        onClick={onClick as any}
+        onClick={onClick}
         className={buttonClassName}
         disabled={disabled}
+        // There is something weird here with `forwardRef`, I’d expect this to work without `any`
         ref={ref as any}
-        {...(props as any)}
+        {...htmlProps}
       >
         {/* This needs to be wrapped in a dom element to 
           fix an issue with the google translate extension

npm-packages/@convex-dev/design-system/src/Menu.tsx

@@ -73,13 +73,22 @@ export function MenuItem({
 }: {
   variant?: "default" | "danger";
   children: ReactNode;
-  action?: () => void;
-  href?: string;
   disabled?: boolean;
   tip?: ReactNode;
   tipSide?: TooltipSide;
   shortcut?: Key[];
-}) {
+} & (
+  | {
+      action: () => void;
+      href?: never;
+    }
+  | {
+      action?: never;
+      href: string;
+    }
+)) {
+  const actionProp = href ? { href } : { onClick: action };
+
   return (
     <HeadlessMenu.Item>
       {({ active }) => (
@@ -98,8 +107,7 @@ export function MenuItem({
               : "text-content-primary",
           )}
           disabled={disabled}
-          onClick={action}
-          href={href}
+          {...actionProp}
         >
           {children}
           {shortcut && (

npm-packages/@convex-dev/design-system/src/UIContext.tsx

@@ -2,6 +2,7 @@ import { omit } from "lodash-es";
 import {
   ComponentType,
   createContext,
+  forwardRef,
   PropsWithChildren,
   useContext,
 } from "react";
@@ -14,7 +15,8 @@ type LinkProps = {
   tabIndex?: number;
   target?: string;
   onClick?: (event: React.MouseEvent<HTMLAnchorElement>) => void;
-};
+  passHref?: boolean;
+} & React.RefAttributes<HTMLAnchorElement>;
 
 function encodeQuery(query: UrlObject["query"]): string {
   if (!query) return "";
@@ -34,24 +36,23 @@ function encodeQuery(query: UrlObject["query"]): string {
 }
 
 // Default link component that just renders an anchor tag
-function DefaultLink({
-  href,
-  children,
-  ...props
-}: PropsWithChildren<LinkProps>) {
-  return (
-    <a
-      href={
-        typeof href === "string"
-          ? href
-          : `${href.pathname}${encodeQuery(href.query)}${href.hash || ""}`
-      }
-      {...omit(props, "passHref")}
-    >
-      {children}
-    </a>
-  );
-}
+const DefaultLink = forwardRef<HTMLAnchorElement, PropsWithChildren<LinkProps>>(
+  function DefaultLink({ href, children, ...props }, ref) {
+    return (
+      <a
+        href={
+          typeof href === "string"
+            ? href
+            : `${href.pathname}${encodeQuery(href.query)}${href.hash || ""}`
+        }
+        {...omit(props, "passHref")}
+        ref={ref}
+      >
+        {children}
+      </a>
+    );
+  },
+);
 
 // Create context with the default link component
 export const UIContext =

npm-packages/dashboard-common/.eslintrc.cjs

@@ -78,7 +78,19 @@ module.exports = {
       },
     ],
 
-    "no-restricted-imports": [2, { paths: ["lodash"] }],
+    "no-restricted-imports": [
+      2,
+      {
+        paths: ["lodash"],
+        patterns: [
+          {
+            group: ["react-day-picker"],
+            importNames: ["Button"],
+            message: "You probably mean to import from @ui/Button.",
+          },
+        ],
+      },
+    ],
     // http://eslint.org/docs/rules/no-restricted-syntax
     "no-restricted-syntax": [
       "error",
@@ -175,7 +187,7 @@ module.exports = {
     "tailwindcss/no-custom-classname": [
       "error",
       {
-        whitelist: ["bottom-four"],
+        whitelist: ["bottom-four", "js-launch-kapa-ai"],
       },
     ],
   },

npm-packages/dashboard-common/src/elements/DateRangePicker.tsx

@@ -33,7 +33,6 @@ export function DateRangePicker({
       placement="bottom-start"
       button={
         <Button
-          id="date"
           variant="neutral"
           className="w-fit justify-start text-left font-normal"
           icon={<CalendarIcon className="size-4" />}

npm-packages/dashboard-common/src/elements/EmptySection.tsx

@@ -18,7 +18,7 @@ export function EmptySection({
   header: string;
   body: React.ReactNode;
   action?: React.ReactNode;
-  learnMoreButton?: ButtonProps;
+  learnMoreButton?: ButtonProps & { href: string };
   sheet?: boolean;
 }) {
   const Parent = sheet ? Sheet : "div";

npm-packages/dashboard-common/src/features/functionRunner/components/RunHistory.tsx

@@ -46,7 +46,7 @@ export function RunHistory({
         variant="neutral"
         tip="Jump to Code"
         href={`${url}#code`}
-        onClick={() => log("jump to code from function runner")}
+        onClickOfAnchorLink={() => log("jump to code from function runner")}
       />
       <Button
         onClick={() => {

npm-packages/dashboard-common/src/features/functions/components/FunctionItem.tsx

@@ -82,26 +82,24 @@ export function DirectoryItem({
   children: React.ReactNode[];
   disclosure?: boolean;
 }) {
-  const Btn = disclosure ? Disclosure.Button : Button;
-  return (
-    <Btn
-      variant="unstyled"
-      className={cn(
-        sidebarLinkClassNames({
-          isActive,
-          font: "mono",
-          small: true,
-        }),
-        "px-0 py-0 w-full min-w-full max-w-full truncate h-[30px] pr-2",
-        "rounded-none",
-        isActive &&
-          "outline outline-util-accent/40 bg-util-accent/30 hover:bg-util-accent/30 font-normal",
-        !isActive && "hover:bg-util-accent/20",
-        "focus-visible:outline-none focus-visible:ring-0 focus-visible:bg-util-accent/20",
-      )}
-      href={href}
-      onClick={onClick}
-    >
+  const { captureMessage } = useContext(DeploymentInfoContext);
+
+  const className = cn(
+    sidebarLinkClassNames({
+      isActive,
+      font: "mono",
+      small: true,
+    }),
+    "px-0 py-0 w-full min-w-full max-w-full truncate h-[30px] pr-2",
+    "rounded-none",
+    isActive &&
+      "outline outline-util-accent/40 bg-util-accent/30 hover:bg-util-accent/30 font-normal",
+    !isActive && "hover:bg-util-accent/20",
+    "focus-visible:outline-none focus-visible:ring-0 focus-visible:bg-util-accent/20",
+  );
+
+  const buttonChildren = (
+    <>
       <div
         className="flex h-full items-center gap-4"
         style={{
@@ -117,6 +115,29 @@ export function DirectoryItem({
           ))}
       </div>
       {children}
-    </Btn>
+    </>
+  );
+
+  if (disclosure) {
+    if (href) {
+      captureMessage("DirectoryItem with href and disclosure");
+    }
+
+    return (
+      <Disclosure.Button className={className} onClick={onClick}>
+        {buttonChildren}
+      </Disclosure.Button>
+    );
+  }
+
+  return (
+    <Button
+      variant="unstyled"
+      className={className}
+      href={href}
+      onClickOfAnchorLink={onClick}
+    >
+      {buttonChildren}
+    </Button>
   );
 }

npm-packages/dashboard-common/src/features/health/components/IntegrationStatus.tsx

@@ -113,7 +113,7 @@ function IntegrationStatusCard({
               tip={`Go to ${integrationName(configuredIntegrations[0].config.type)}`}
               icon={<ExternalLinkIcon />}
               href={configToUrl(configuredIntegrations[0].config)}
-              onClick={() => log("go to integration via health")}
+              onClickOfAnchorLink={() => log("go to integration via health")}
               target="_blank"
               size="xs"
               inline
@@ -124,7 +124,7 @@ function IntegrationStatusCard({
             tip="Configure Integrations"
             icon={<GearIcon />}
             href={integrationsPageLink}
-            onClick={() => log("configure integrations via health")}
+            onClickOfAnchorLink={() => log("configure integrations via health")}
             target="_blank"
             size="xs"
             inline

npm-packages/dashboard/src/components/deploymentSettings/BackupListItem.tsx

@@ -178,7 +178,7 @@ export function BackupListItem({
                   href={
                     backup.state === "complete"
                       ? getZipExportUrl(backup.snapshotId)
-                      : undefined
+                      : "" // only when disabled
                   }
                   tipSide="left"
                   tip={

npm-packages/dashboard/src/components/header/ProjectSelector/ProjectSelector.tsx

@@ -226,7 +226,7 @@ function ProjectSelectorPanel({
               <Button
                 size="xs"
                 href={`/t/${team.slug}/settings`}
-                onClick={close}
+                onClickOfAnchorLink={close}
                 inline
                 variant="neutral"
                 icon={<GearIcon />}
@@ -287,7 +287,7 @@ function ProjectSelectorPanel({
               <Button
                 size="xs"
                 href={`/t/${team.slug}/${lastHoveredProject.slug}/settings`}
-                onClick={close}
+                onClickOfAnchorLink={close}
                 inline
                 variant="neutral"
                 icon={<GearIcon />}

npm-packages/dashboard/src/components/projectSettings/CustomDomains.tsx

@@ -447,7 +447,6 @@ function VanityDomainForm({
       <Button
         className="flex w-fit"
         type="submit"
-        color="primary"
         disabled={
           disabled ||
           formState.isSubmitting ||

npm-packages/dashboard/src/components/referral/ReferralsBanner.tsx

@@ -11,7 +11,7 @@ import { useId, useState } from "react";
 interface ReferralsBannerProps {
   team: Team;
   referralState: ReferralState;
-  onHide?: () => void;
+  onHide: () => void;
   className?: string;
 }
 

npm-packages/dashboard/src/elements/AskAI.tsx

@@ -9,7 +9,7 @@ export function AskAI() {
         src="https://widget.kapa.ai/kapa-widget.bundle.js"
         data-modal-title="Convex AI"
         data-button-hide="true"
-        data-modal-override-open-id="js-launch-kapa-ai"
+        data-modal-override-open-class="js-launch-kapa-ai"
         data-website-id="8dfb3aad-6006-4f56-b2ed-75fa8051db22"
         data-project-name="Convex"
         data-project-color="#3F5295"
@@ -20,9 +20,7 @@ export function AskAI() {
       />
       <Button
         inline
-        id="js-launch-kapa-ai"
-        type="button"
-        className="flex items-center gap-1 px-2.5 text-sm text-content-primary"
+        className="js-launch-kapa-ai flex items-center gap-1 px-2.5 text-sm text-content-primary"
       >
         <SparklesIcon className="size-4" />
         <span className="hidden md:block">Ask AI</span>