chore: start fixing up MenuItem types

src/components/sidebar/components/sidebar-nav-menu-item/index.tsx

@@ -18,7 +18,7 @@ import { ProductSlug } from 'types/products'
 import isAbsoluteUrl from 'lib/is-absolute-url'
 import Badge from 'components/badge'
 import Link from 'components/link'
-import { MenuItem } from 'components/sidebar'
+import { MenuItem, MenuItemOptionalProperties } from 'components/sidebar'
 import ProductIcon from 'components/product-icon'
 import {
 	SidebarHorizontalRule,
@@ -31,7 +31,6 @@ import {
 	SidebarNavLinkItemProps,
 	SidebarNavMenuButtonProps,
 	SidebarNavMenuItemBadgeProps,
-	SidebarNavMenuItemProps,
 	SupportedIconName,
 } from './types'
 import s from './sidebar-nav-menu-item.module.css'
@@ -198,18 +197,47 @@ export function SidebarNavMenuButton({ item }: SidebarNavMenuButtonProps) {
 	)
 }
 
+/**
+ * Given a MenuItem,
+ * Return `true` if the item should be shown "open" by default,
+ * or `false` otherwise.
+ *
+ * TODO: update input `item` type to be the "submenu" item type specifically.
+ * Can't do this yet cause MenuItem still needs more work before we can.
+ * Task: https://app.asana.com/0/1202097197789424/1202405210286689/f
+ */
+function getDefaultOpen(item: MenuItemOptionalProperties): boolean {
+	const defaultOpenProps = [
+		'isOpen',
+		'hasActiveChild',
+		'hasChildrenMatchingFilter',
+		'matchesFilter',
+	]
+	const isDefaultOpen = defaultOpenProps.reduce((acc, prop) => {
+		if (item[prop]) {
+			return true
+		}
+		return acc
+	}, false)
+	return isDefaultOpen
+}
+
 /**
  * Handles rendering a collapsible/expandable submenu item and its child menu
  * items in the Sidebar.
  */
-const SidebarNavSubmenuItem = ({ item }: SidebarNavMenuItemProps) => {
+const SidebarNavSubmenuItem = ({
+	item,
+}: {
+	/**
+	 * TODO: update this `item` type to be the "submenu" item type specifically.
+	 * Can't do this yet cause MenuItem still needs more work before we can.
+	 * Task: https://app.asana.com/0/1202097197789424/1202405210286689/f
+	 */
+	item: MenuItemOptionalProperties
+}) => {
 	const buttonRef = useRef<HTMLButtonElement>()
-	const [isOpen, setIsOpen] = useState(
-		item.isOpen ||
-			item.hasActiveChild ||
-			item.hasChildrenMatchingFilter ||
-			item.matchesFilter
-	)
+	const [isOpen, setIsOpen] = useState(getDefaultOpen(item))
 	const hasBadge = !!(item as $TSFixMe).badge
 
 	/**
@@ -218,18 +246,8 @@ const SidebarNavSubmenuItem = ({ item }: SidebarNavMenuItemProps) => {
 	 * if we pass the props needed instead of just the item object?
 	 */
 	useEffect(() => {
-		setIsOpen(
-			item.isOpen ||
-				item.hasActiveChild ||
-				item.hasChildrenMatchingFilter ||
-				item.matchesFilter
-		)
-	}, [
-		item.isOpen,
-		item.hasActiveChild,
-		item.hasChildrenMatchingFilter,
-		item.matchesFilter,
-	])
+		setIsOpen(getDefaultOpen(item))
+	}, [item])
 
 	const handleKeyDown: KeyboardEventHandler<HTMLUListElement> = (e) => {
 		if (e.key === 'Escape') {
@@ -272,7 +290,19 @@ const SidebarNavSubmenuItem = ({ item }: SidebarNavMenuItemProps) => {
 			{isOpen && (
 				<ul id={listId} onKeyDown={handleKeyDown}>
 					{item.routes.map((route: MenuItem, i) => {
-						const key = `${route.id || route.fullPath || route.title}-${i}`
+						/**
+						 * Note: these items _aren't_ stable since we filter them
+						 * client-side... perhaps `useId` would be appropriate here?
+						 * Or we could do that server-side before passing props to the
+						 * client? `heading` and `divider` items do _not_ have a
+						 * meaningful identifier; and some other items could potentially
+						 * have duplicate identifiers. We need to better account for that.
+						 */
+						const uniqueIshId =
+							'heading' in route
+								? route.heading
+								: route.id || route.fullPath || route.title
+						const key = `${uniqueIshId}-${i}`
 						return <SidebarNavMenuItem key={key} item={route} />
 					})}
 				</ul>
@@ -289,15 +319,15 @@ const SidebarNavSubmenuItem = ({ item }: SidebarNavMenuItemProps) => {
  *  - SidebarNavSubmenu
  *  - SidebarNavLink
  */
-const SidebarNavMenuItem = ({ item }: SidebarNavMenuItemProps) => {
+const SidebarNavMenuItem = ({ item }: { item: MenuItem }) => {
 	let itemContent
-	if (item.divider) {
+	if ('divider' in item) {
 		itemContent = <SidebarHorizontalRule />
-	} else if (item.heading) {
+	} else if ('heading' in item) {
 		itemContent = <SidebarSectionHeading text={item.heading} />
-	} else if (item.routes) {
+	} else if ('routes' in item) {
 		itemContent = <SidebarNavSubmenuItem item={item} />
-	} else if (item.theme) {
+	} else if ('theme' in item) {
 		itemContent = (
 			<SidebarNavHighlightItem
 				theme={item.theme}
@@ -310,7 +340,7 @@ const SidebarNavMenuItem = ({ item }: SidebarNavMenuItemProps) => {
 		itemContent = <SidebarNavLinkItem item={item} />
 	}
 
-	return <li id={item.id}>{itemContent}</li>
+	return <li>{itemContent}</li>
 }
 
 export { SidebarNavLinkItem, SidebarNavSubmenuItem }

src/components/sidebar/components/sidebar-nav-menu-item/types.ts

@@ -6,7 +6,7 @@
 import { ReactElement } from 'react'
 import { IconChevronDown16 } from '@hashicorp/flight-icons/svg-react/chevron-down-16'
 import Badge, { BadgeProps } from 'components/badge'
-import { MenuItem } from 'components/sidebar'
+import { MenuItemOptionalProperties } from 'components/sidebar'
 import { ProductSlug } from 'types/products'
 
 interface RightIconsContainerProps {
@@ -23,12 +23,8 @@ interface SidebarNavMenuItemBadgeProps {
 	type?: BadgeProps['type']
 }
 
-interface SidebarNavMenuItemProps {
-	item: MenuItem
-}
-
 type SupportedIconName = 'home' & 'guide' & ProductSlug
-interface SidebarNavLinkItem extends MenuItem {
+interface SidebarNavLinkItem extends MenuItemOptionalProperties {
 	leadingIconName?: SupportedIconName
 }
 
@@ -45,6 +41,5 @@ export type {
 	SidebarNavMenuButtonProps,
 	SidebarNavLinkItemProps,
 	SidebarNavMenuItemBadgeProps,
-	SidebarNavMenuItemProps,
 	SupportedIconName,
 }

src/components/sidebar/index.tsx

@@ -22,7 +22,12 @@ import {
 } from 'components/sidebar/components'
 
 // Local imports
-import { FilteredNavItem, MenuItem, SidebarProps } from './types'
+import {
+	FilteredNavItem,
+	MenuItem,
+	MenuItemOptionalProperties,
+	SidebarProps,
+} from './types'
 import {
 	addNavItemMetaData,
 	getFilteredNavItems,
@@ -146,5 +151,5 @@ const Sidebar = ({
 	)
 }
 
-export type { MenuItem, SidebarProps }
+export type { MenuItem, MenuItemOptionalProperties, SidebarProps }
 export default Sidebar

src/components/sidebar/types.ts

@@ -124,7 +124,7 @@ export interface NavHighlightItem {
  * with optional properties to cover all possible menu item types.
  * ref: https://app.asana.com/0/1202097197789424/1202405210286689/f
  */
-interface MenuItem {
+interface MenuItemOptionalProperties {
 	divider?: boolean
 	fullPath?: string
 	hasActiveChild?: boolean
@@ -136,7 +136,6 @@ interface MenuItem {
 	path?: string
 	routes?: MenuItem[]
 	title?: string
-	heading?: string
 	isOpen?: boolean
 	/**
 	 * Optional icon to display at right of the menu item.
@@ -150,6 +149,15 @@ interface MenuItem {
 	theme?: NavHighlightItem['theme']
 }
 
+/**
+ * TODO: build out this revised MenuItem type to capture all of the
+ * different things `MenuItemOptionalProperties`, but with a union
+ * of more concrete and distinguishable types instead.
+ *
+ * Task: https://app.asana.com/0/1202097197789424/1202405210286689/f
+ */
+type MenuItem = MenuItemOptionalProperties | HeadingNavItem
+
 interface SidebarBaseProps {
 	/**
 	 * Optional props to send to `SidebarBackToLink` which is displayed at the top
@@ -219,6 +227,7 @@ export type {
 	EnrichedSubmenuNavItem,
 	FilteredNavItem,
 	LinkNavItemWithMetaData,
+	MenuItemOptionalProperties,
 	MenuItem,
 	NavItemWithMetaData,
 	SidebarProps,

src/lib/docs/add-branded-overview-sidebar-item.ts

@@ -4,12 +4,17 @@
  */
 
 import { ProductSlug } from 'types/products'
-import { MenuItem, EnrichedNavItem } from 'components/sidebar/types'
+import { MenuItem } from 'components/sidebar/types'
 
 /**
  * Determine whether a `menuItem` is an "overview" item.
  */
 function isOverviewItem(item: MenuItem) {
+	// Item must have a `path`, otherwise definitely not an overview item
+	if (!('path' in item)) {
+		return false
+	}
+	// Path must match certain criteria for "overview" items
 	const isPathMatch =
 		item.path === '' ||
 		item.path === '/' ||
@@ -22,7 +27,7 @@ function isOverviewItem(item: MenuItem) {
  * Determine whether a `menuItem` is an "heading" item.
  */
 function isHeadingItem(item: MenuItem) {
-	return typeof item.heading == 'string'
+	return 'heading' in item && typeof item.heading == 'string'
 }
 
 /**

src/views/onboarding/tutorial-view/server.ts

@@ -102,7 +102,7 @@ export async function getOnboardingTutorialProps(
 	// handle next sidebar collection logic
 	if (isLastTutorial) {
 		const currentIndex = tutorialNavLevelMenuItems.findIndex(
-			(item: MenuItem) => item.fullPath === `/${currentCollection.slug}`
+			(item) => item.fullPath === `/${currentCollection.slug}`
 		)
 		const nextSidebarItem: MenuItem =
 			tutorialNavLevelMenuItems[currentIndex + 1]

src/views/well-architected-framework/tutorial-view/server.ts

@@ -17,7 +17,7 @@ import {
 	TutorialLite as ClientTutorialLite,
 } from 'lib/learn-client/types'
 import { generateTopLevelSidebarNavData } from 'components/sidebar/helpers'
-import { MenuItem, SidebarProps } from 'components/sidebar'
+import { SidebarProps } from 'components/sidebar'
 import { EnrichedNavItem } from 'components/sidebar/types'
 import { generateWafCollectionSidebar } from 'views/well-architected-framework/utils/generate-collection-sidebar'
 import { getNextPrevious } from 'views/tutorial-view/components'
@@ -97,13 +97,31 @@ export async function getWafTutorialViewProps(
 
 	if (isLastTutorial) {
 		const filteredSidebarItems = categorizedWafCollectionSidebarItems.filter(
-			(item: MenuItem) => !item.divider && !item.heading
+			(item) => {
+				/**
+				 * Filter out divider and heading items, these are definitely
+				 * not the "next" item.
+				 *
+				 * Note that tutorials sidebars will never have
+				 * divider or heading items, so this is probably unnecessary.
+				 * But it's here for consistency with other sidebar types.
+				 * Types could potentially be refactored to reflect that
+				 * tutorial sidebars only use a subset of possible `MenuItem` types.
+				 */
+				const isDivider = 'divider' in item
+				const isHeading = 'heading' in item
+				return !isDivider && !isHeading
+			}
 		)
 		const currentIndex = filteredSidebarItems.findIndex(
-			(item: MenuItem) => item.fullPath === `/${currentCollection.slug}`
+			(item) =>
+				'fullPath' in item && item.fullPath === `/${currentCollection.slug}`
 		)
-		const nextSidebarItem: MenuItem = filteredSidebarItems[currentIndex + 1]
-		nextCollection = allWafCollections.find((c) => nextSidebarItem?.id === c.id)
+		const nextSidebarItem = filteredSidebarItems[currentIndex + 1]
+		nextCollection = allWafCollections.find((c) => {
+			const hasId = nextSidebarItem && 'id' in nextSidebarItem
+			return hasId && nextSidebarItem.id === c.id
+		})
 	}
 
 	/**