PM-2749 - upgrade major node modules #393
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| export interface AuthConfig { | ||
| user: AuthUser | ||
| profileCompletionData: ProfileCompletionData | ||
| user?: AuthUser |
There was a problem hiding this comment.
[❗❗ correctness]
Making user optional may lead to runtime errors if the rest of the codebase assumes user is always defined. Ensure that all usages of user are updated to handle the case where it might be undefined.
| user: AuthUser | ||
| profileCompletionData: ProfileCompletionData | ||
| user?: AuthUser | ||
| profileCompletionData?: ProfileCompletionData |
There was a problem hiding this comment.
[❗❗ correctness]
Making profileCompletionData optional could cause issues if the code assumes it is always present. Verify that all references to profileCompletionData handle the possibility of it being undefined.
| export let activeRoute: NavMenuItem = undefined; | ||
| export let items: NavMenuItem[]; | ||
| export let activeRoute: NavMenuItem | undefined = undefined; | ||
| export let items: NavMenuItem[] = []; |
There was a problem hiding this comment.
[correctness]
Initializing items with an empty array is a good practice, but ensure that the rest of the codebase handles the case where items might be empty. If there are any assumptions about items always having elements, they should be revisited.
| role="button" | ||
| tabindex="0" | ||
| on:click={() => toggleItem(item)} | ||
| on:keydown={(ev) => ev.key === 'Enter' && toggleItem(item)} |
There was a problem hiding this comment.
[accessibility]
The on:keydown handler now checks for the 'Enter' key to toggle the item, which is a good accessibility improvement. However, consider also handling the 'Space' key, as it is commonly used for activating buttons via keyboard.
| let isPopupMenuActive: boolean = false; | ||
|
|
||
| function isActiveMenu(menuItem: NavMenuItem, activeMenuItem: NavMenuItem) { | ||
| function isActiveMenu(menuItem: NavMenuItem, activeMenuItem?: NavMenuItem) { |
There was a problem hiding this comment.
[correctness]
The change to make activeMenuItem optional in isActiveMenu is good for flexibility, but ensure that all calls to isActiveMenu handle the case where activeMenuItem is undefined to avoid potential runtime errors.
|
|
||
| function itemHasHoverMenu(menuItem: NavMenuItem) { | ||
| return menuItem.children?.length || menuItem.description; | ||
| return !!(menuItem.children?.length) || !!menuItem.description; |
There was a problem hiding this comment.
[💡 readability]
The use of double negation !! is correct for converting values to boolean, but it can be less readable. Consider using explicit comparisons for clarity, especially for menuItem.children?.length.
| } | ||
|
|
||
| hoveredElement = ev.target; | ||
| hoveredElement = ev.currentTarget as HTMLElement ?? undefined; |
There was a problem hiding this comment.
[correctness]
The use of ev.currentTarget as HTMLElement ?? undefined assumes currentTarget is always an HTMLElement. Ensure this assumption is valid in all contexts where handleMouseover is used, or add type checks to prevent potential runtime errors.
| <div class={styles.mobileMenuWrap} transition:fade={{duration: 200}}> | ||
| <TopNavbar style="primary" showLogo={false}> | ||
| <div class={styles.closeIcon} slot="right" on:click={handleClose} on:keydown={() => {}}> | ||
| <div class={styles.closeIcon} role="button" tabindex="0" slot="right" on:click={handleClose} on:keydown={() => {}}> |
There was a problem hiding this comment.
[accessibility]
The on:keydown handler is currently an empty function. If the intention is to handle keyboard events for accessibility, consider implementing the necessary logic to handle key events like Enter or Space to trigger handleClose. This will improve accessibility for keyboard users.
| } | ||
|
|
||
| el.dataset.targetKey = undefined | ||
| delete el.dataset.targetKey |
There was a problem hiding this comment.
[💡 design]
Using delete el.dataset.targetKey is correct for removing the attribute, but it might be worth considering setting it to an empty string instead if you want to ensure the attribute remains present but empty. This can sometimes be useful for CSS selectors or other logic that checks for the presence of the attribute.
| @import 'lib/styles/mixins.scss'; | ||
| @import 'lib/styles/fonts.scss'; | ||
| @import 'lib/styles/colors.scss'; | ||
| @use 'lib/styles/mixins.scss' as *; |
There was a problem hiding this comment.
[maintainability]
Switching from @import to @use is a good practice for better modularity and to avoid namespace conflicts. However, using as * imports all members into the global scope, which can lead to potential conflicts and makes it harder to track where variables and mixins are coming from. Consider importing only the necessary members or using a namespace to improve maintainability.
|
|
||
| function handleNavigation(ev: MouseEvent, menuItem: NavMenuItem) { | ||
| if (typeof navigationHandler === 'function') { | ||
| if (typeof navigationHandler === 'function' && menuItem.url) { |
There was a problem hiding this comment.
[maintainability]
The addition of menuItem.url in the condition ensures that navigationHandler is only called when a valid URL is present, which is a good improvement for preventing potential errors. However, consider logging or handling the case where menuItem.url is not present to aid in debugging and provide feedback to the user or developer.
| @@ -1,4 +1,4 @@ | |||
| @import 'lib/styles/mixins.scss'; | |||
| @use 'lib/styles/mixins.scss' as *; | |||
There was a problem hiding this comment.
[maintainability]
The change from @import to @use is correct for modern Sass, but using as * can lead to namespace collisions. Consider using a specific namespace to avoid potential conflicts with other modules.
| @@ -1,5 +1,5 @@ | |||
| @import 'lib/styles/fonts.scss'; | |||
| @import 'lib/styles/mixins.scss'; | |||
| @use 'lib/styles/fonts.scss' as *; | |||
There was a problem hiding this comment.
[maintainability]
Switching from @import to @use is a good practice for better modularity and avoiding namespace conflicts. However, using as * imports all members into the global namespace, which can lead to potential conflicts and makes it harder to track where variables and mixins are coming from. Consider importing only the necessary members or using a namespace to maintain clarity.
| <div class={classnames(styles.modalWrap, $$props.class, size && `size-${size}`)}> | ||
| <div class={styles.modalContainer}> | ||
| <div class={styles.modalOverlay} transition:fade={{duration: 200}} on:click={() => isVisible = false} on:keydown={() => {}} /> | ||
| <div class={styles.modalOverlay} role="button" tabindex="0" transition:fade={{duration: 200}} on:click={() => isVisible = false} on:keydown={() => {}} /> |
There was a problem hiding this comment.
[💡 maintainability]
The on:keydown={() => {}} handler is currently a no-op. If this is intentional, consider removing it to avoid confusion. If it's meant to handle a specific key event, implement the necessary logic.
| class={styles.closeBtn} | ||
| on:click={() => isVisible = ''} | ||
| on:click={() => isVisible = false} | ||
| on:keydown={() => {}} |
There was a problem hiding this comment.
[💡 maintainability]
The on:keydown={() => {}} handler is currently a no-op. If this is intentional, consider removing it to avoid confusion. If it's meant to handle a specific key event, implement the necessary logic.
| let elYOffset = 0; | ||
|
|
||
| function handleScroll() { | ||
| if (!elRef) { |
There was a problem hiding this comment.
[maintainability]
The check if (!elRef) ensures that elRef is defined before accessing its properties. However, since elRef is used in multiple places, consider initializing it with a default value or handling its potential undefined state more centrally to improve maintainability.
| } | ||
|
|
||
| onMount(() => { | ||
| if (!elRef) { |
There was a problem hiding this comment.
[correctness]
The repeated check if (!elRef) in both handleScroll and onMount suggests that elRef might not be reliably set. Consider reviewing the logic that assigns elRef to ensure it is correctly initialized before these functions are called.
| @import 'lib/styles/mixins.scss'; | ||
| @import 'lib/styles/fonts.scss'; | ||
| @import 'lib/styles/colors.scss'; | ||
| @use 'lib/styles/mixins.scss' as *; |
There was a problem hiding this comment.
[maintainability]
Switching from @import to @use is a good practice for better encapsulation and avoiding global namespace pollution. However, using as * defeats this purpose by exposing all variables and mixins globally. Consider using specific imports or aliasing to maintain encapsulation and avoid potential conflicts.
| import styles from './ToolMenu.module.scss'; | ||
|
|
||
| let navMenuItems = getToolSelectorItems(); | ||
| let navMenuItems: NavMenuItem[] = getToolSelectorItems() ?? []; |
There was a problem hiding this comment.
[correctness]
The use of ?? [] ensures that navMenuItems is always an array, which is good for preventing runtime errors. However, if getToolSelectorItems() returns null or undefined, it might indicate a deeper issue with data fetching that should be addressed elsewhere.
| {#each navMenuItems as section, sectionIndex} | ||
| <div class={classnames(styles.toolSection, styles[section.label?.toLowerCase()])}> | ||
| {#if section} | ||
| <div class={classnames(styles.toolSection, section.label ? styles[section.label.toLowerCase()] : undefined)}> |
There was a problem hiding this comment.
[💡 maintainability]
The conditional check {#if section} is a good addition to prevent errors when section is null or undefined. However, consider ensuring that navMenuItems never contains null or undefined values at the source to simplify the template logic.
|
|
||
| <div class={styles.toolGroups}> | ||
| {#each section.children as group} | ||
| {#each section.children ?? [] as group} |
There was a problem hiding this comment.
[correctness]
Using ?? [] for section.children ensures that the each block does not fail if children is null or undefined. This is a good defensive programming practice, but it might be better to ensure that children is always an array at the data source.
|
|
||
| <div class={styles.toolNavItems}> | ||
| {#each group.children as navItem} | ||
| {#each group.children ?? [] as navItem} |
There was a problem hiding this comment.
[correctness]
The use of ?? [] for group.children is a safe guard against null or undefined values. However, consider ensuring that children is always an array at the data source to reduce the need for such checks.
| tabindex="0" | ||
| bind:this={elRef} | ||
| on:click={() => popupIsVisible = true} | ||
| on:keydown={() => {}} |
There was a problem hiding this comment.
[accessibility]
The on:keydown event handler is currently an empty function. If this is intended to handle keyboard interactions for accessibility, it should be implemented to manage specific key events like 'Enter' or 'Space' to trigger the button action.
| $ctx.auth = {...$ctx.auth, ready: false}; | ||
| const authUser = await fetchUserProfile(); | ||
| $ctx.auth = {...$ctx.auth, ready: true, user: authUser}; | ||
| $ctx.auth = {...$ctx.auth, ready: true, user: authUser ?? undefined}; |
There was a problem hiding this comment.
[💡 correctness]
Using the nullish coalescing operator (??) to default authUser to undefined is unnecessary here. If fetchUserProfile() returns null, setting user to null might be more informative than undefined for debugging purposes.
| let initials: string = ''; | ||
| $: initials = user['initials'] ?? ( | ||
| $: initials = ( | ||
| (('initials' in user ? (user as any).initials : undefined) as string | undefined) ?? |
There was a problem hiding this comment.
[maintainability]
The use of (user as any).initials introduces a type safety issue by bypassing TypeScript's type checking. Consider refining the AuthUser type to include initials if it's a valid property, or use a safer type assertion method.
| tabindex="0" | ||
| bind:this={elRef} | ||
| on:click={() => popupIsVisible = true} | ||
| on:keydown={() => {}} |
There was a problem hiding this comment.
[💡 readability]
The on:keydown={() => {}} handler is currently a no-op. If this is intended for accessibility (e.g., handling 'Enter' or 'Space' for button-like behavior), it should be implemented. Otherwise, consider removing it to avoid confusion.
| @import 'lib/styles/mixins.scss'; | ||
| @import 'lib/styles/fonts.scss'; | ||
| @import 'lib/styles/colors.scss'; | ||
| @use 'lib/styles/mixins.scss' as *; |
There was a problem hiding this comment.
[maintainability]
Switching from @import to @use is a good practice for better encapsulation and avoiding global namespace pollution. However, using as * negates some of these benefits by exposing all variables and mixins globally. Consider using specific namespaces instead to maintain encapsulation.
| * @returns {Authorization: 'Bearer'} or {} | ||
| */ | ||
| export function getRequestAuthHeaders() { | ||
| export function getRequestAuthHeaders(): Partial<{ Authorization : string }> { |
There was a problem hiding this comment.
[💡 maintainability]
The return type Partial<{ Authorization : string }> is technically correct, but consider using a more explicit type like Record<string, string> if you expect more headers in the future. This would improve maintainability by making it easier to extend the function.
|
|
||
| const filtered = filterRoutesByUserRole(menu); | ||
| return filtered?.children | ||
| return filtered?.children ?? []; |
There was a problem hiding this comment.
[correctness]
The use of the nullish coalescing operator (??) to default to an empty array is a good approach for handling cases where filtered.children might be null or undefined. However, ensure that the rest of the codebase correctly handles an empty array as a valid return value, as this could potentially change the behavior if previously null or undefined was expected and handled differently.
|
|
||
| // store fetched data in a local cache (de-duplicate immediate api calls) | ||
| const localCache = {}; | ||
| const localCache: any = {}; |
There was a problem hiding this comment.
[maintainability]
Using any for localCache type can lead to potential type safety issues. Consider defining a more specific type for localCache to ensure type safety and improve maintainability.
|
|
||
| // get the user roles that match the config/auth's user roles definitions | ||
| export const getUserAppRoles = (): AuthUser['roles'] | undefined => { | ||
| export const getUserAppRoles = (): AuthUser['roles'] => { |
There was a problem hiding this comment.
[❗❗ correctness]
The return type of getUserAppRoles was changed from AuthUser['roles'] | undefined to AuthUser['roles']. This change assumes that getJwtUserRoles will never return undefined, which may not be the case. Ensure that getJwtUserRoles always returns a valid array or handle the undefined case appropriately.
| * @returns Promise<AuthUser> | ||
| */ | ||
| export const fetchUserProfile = async (): Promise<AuthUser> => { | ||
| export const fetchUserProfile = async (): Promise<AuthUser | undefined> => { |
There was a problem hiding this comment.
[❗❗ correctness]
The return type of fetchUserProfile was changed to Promise<AuthUser | undefined>. Ensure that all calling code handles the possibility of undefined being returned, as this could lead to unexpected behavior if not properly managed.
|
|
||
|
|
||
| let resolve: (value: AuthUser) => void; | ||
| let resolve: (value: AuthUser) => void = () => {}; |
There was a problem hiding this comment.
[design]
The initialization of resolve with a no-op function () => {} is unnecessary and could mask potential issues if resolve is used before being properly assigned. Consider initializing resolve only when the Promise is created.
| export let menuItems: NavMenuItem[] = []; | ||
| export let activeRoutePath: NavMenuItem[] = []; | ||
| export let activeRoute: NavMenuItem = null; | ||
| export let activeRoute: NavMenuItem | undefined = undefined; |
There was a problem hiding this comment.
[💡 maintainability]
Consider using null instead of undefined for activeRoute to maintain consistency with common TypeScript practices, unless there's a specific reason to use undefined. This can help avoid potential issues with type checks and improve code clarity.
| </script> | ||
|
|
||
| <div class={styles.menuBtn} on:click={() => menuIsVisible = true} on:keydown={() => {}}> | ||
| <div class={styles.menuBtn} role="button" tabindex="0" on:click={() => menuIsVisible = true} on:keydown={() => {}}> |
There was a problem hiding this comment.
[💡 readability]
The on:keydown handler is currently an empty function. If this is intentional, consider removing it to avoid confusion. If it's a placeholder for future functionality, it might be better to add a comment explaining its purpose.
|
|
||
| @function color($base, $shade: default) { | ||
| $color: map-get(map-get($colors, $base), $shade); | ||
| $color: map.get(map.get($colors, $base), $shade); |
There was a problem hiding this comment.
[maintainability]
The change from map-get to map.get is correct for using the sass:map module. However, ensure that all environments where this code runs support the @use syntax, as it requires Dart Sass 1.23.0 or later.
| role="button" | ||
| tabindex="0" | ||
| on:click={toggleMainMenu} | ||
| on:keydown={() => {}} |
There was a problem hiding this comment.
[accessibility]
The on:keydown handler is currently an empty function. If this is intended to handle keyboard interactions for accessibility, it should be implemented to handle specific keys (e.g., Enter or Space) to trigger the toggleMainMenu function. Otherwise, it could be removed to avoid confusion.
| const tokens: string[] = []; | ||
|
|
||
| for (const arg of args) { | ||
| if (typeof arg !== 'string' || !arg.trim()) { |
There was a problem hiding this comment.
[correctness]
The check typeof arg !== 'string' || !arg.trim() could be simplified to !arg || typeof arg !== 'string' || !arg.trim() to ensure that non-string falsy values are also filtered out early, improving readability and correctness.
| ]; | ||
|
|
||
| let previousLoadPromise; | ||
| let previousLoadPromise: Promise<void> | undefined; |
There was a problem hiding this comment.
[correctness]
The type annotation Promise<void> | undefined for previousLoadPromise is a good addition for clarity. However, ensure that all usages of previousLoadPromise handle the undefined case appropriately to avoid potential runtime errors.
| // append it before the first <link" we foind in the document | ||
| const firstLink = document.getElementsByTagName('link')[0]; | ||
| firstLink.parentNode.insertBefore(link, firstLink); | ||
| const parent = firstLink?.parentNode ?? document.head; |
There was a problem hiding this comment.
[correctness]
The use of optional chaining and nullish coalescing is a good approach to handle cases where firstLink might be undefined. However, consider adding a check to ensure that document.head is not null before using it as a fallback, as this could theoretically lead to a runtime error in environments where the document structure is not guaranteed.
| * @returns NavMenuItem.children | ||
| */ | ||
| export const toMarketingHostUrls = ({ children = [] }: NavMenuItem, depth?: number) => { | ||
| export const toMarketingHostUrls = ({ children = [] }: NavMenuItem, depth: number = 0) => { |
There was a problem hiding this comment.
[💡 maintainability]
The default value for depth is set to 0, which is a good practice for ensuring a default is always provided. However, consider explicitly documenting the maximum depth limit (9) in the function's description or as a constant to improve maintainability and clarity.
| // safe escape if things get out of control | ||
| if (depth >= 9) { | ||
| return | ||
| return children |
There was a problem hiding this comment.
[correctness]
The return statement now returns children instead of undefined when the depth limit is reached. This change improves correctness by ensuring a consistent return type, which is beneficial for functions expected to return arrays.
| export const filterRoutes = ( | ||
| navMenuItem: NavMenuItem, | ||
| filter: (n: NavMenuItem) => boolean, | ||
| depth: number = 0 |
There was a problem hiding this comment.
[correctness]
The default value for depth is set to 0, which is fine, but ensure that this does not conflict with any existing logic that might expect depth to be undefined initially. This change should be verified against all usages of filterRoutes.
| * @returns NavMenuItem.children | ||
| */ | ||
| export const activateAuthenticatedRoutes = (isAuthenticated: boolean, { children = [] }: NavMenuItem, depth?: number) => { | ||
| export const activateAuthenticatedRoutes = (isAuthenticated: boolean, { children = [] }: NavMenuItem, depth: number = 0): NavMenuItem[] => { |
There was a problem hiding this comment.
[correctness]
The default value for depth is set to 0, which is fine, but ensure that this does not conflict with any existing logic that might expect depth to be undefined initially. This change should be verified against all usages of activateAuthenticatedRoutes.
| export const matchRoutes = (navMenu: NavMenuItem, path: string): NavMenuItem[] => { | ||
|
|
||
| return (function parseNavMenu(l, { children = [] }) { | ||
| return (function parseNavMenu(l: number, { children = [] }: NavMenuItem): NavMenuItem[] | undefined { |
There was a problem hiding this comment.
[correctness]
The return type of the inner function parseNavMenu has been changed to NavMenuItem[] | undefined. Ensure that all usages of matchRoutes handle the case where undefined is returned correctly, especially since the outer function defaults to an empty array.
| } | ||
| })(0, navMenu) | ||
| return undefined; | ||
| })(0, navMenu) ?? []; |
There was a problem hiding this comment.
[correctness]
The use of ?? [] ensures that matchRoutes always returns an array, which is a good safeguard. However, ensure that this change is compatible with all existing code that consumes matchRoutes.
| ) => void | ||
|
|
||
| const NavigationLoadersMap = { | ||
| const NavigationLoadersMap: Record<NavigationType, () => Promise<any>> = { |
There was a problem hiding this comment.
[maintainability]
The use of Promise<any> in NavigationLoadersMap can lead to type safety issues. Consider using a more specific type instead of any to improve type safety and maintainability.
|
|
||
| const appContext = ctx.get('appContext'); | ||
| if (!appContext) { | ||
| throw new Error(`Navigation #${targetId} is missing context!`); |
There was a problem hiding this comment.
[💡 readability]
The error message thrown when appContext is missing could be more descriptive by including the targetId to aid in debugging.
| const tcUnivNavConfig = (window[globalName] ?? {}) as any; | ||
| const queue = tcUnivNavConfig.q ?? []; | ||
| const globalName = (('TcUnivNavConfig' in window && (window as any).TcUnivNavConfig) || 'tcUniNav') as string; | ||
| const tcUnivNavConfig = ((globalName in window && (window as any)[globalName]) ?? {}) as { q?: unknown[] }; |
There was a problem hiding this comment.
[maintainability]
The type assertion (window as any)[globalName] could be improved by defining a more specific type for tcUnivNavConfig to avoid using any, which can lead to runtime errors.
| // replace the method that adds the calls to the queue | ||
| // with a direct exec call | ||
| window[globalName] = execQueueCall.bind(null) | ||
| Object.assign(window as any, {[globalName]: execQueueCall.bind(null)}); |
There was a problem hiding this comment.
[security]
Using Object.assign with window and any type can lead to runtime issues if the global object structure changes. Consider defining a type for window that includes globalName to ensure type safety.
| export interface AuthConfig { | ||
| user: AuthUser; | ||
| profileCompletionData: ProfileCompletionData; | ||
| user?: AuthUser; |
There was a problem hiding this comment.
[❗❗ correctness]
Changing user from a required to an optional property could lead to runtime errors if the rest of the codebase assumes user is always defined. Ensure that all usages of AuthConfig handle user being undefined appropriately.
| user: AuthUser; | ||
| profileCompletionData: ProfileCompletionData; | ||
| user?: AuthUser; | ||
| profileCompletionData?: ProfileCompletionData; |
There was a problem hiding this comment.
[❗❗ correctness]
Changing profileCompletionData from a required to an optional property could lead to runtime errors if the rest of the codebase assumes profileCompletionData is always defined. Ensure that all usages of AuthConfig handle profileCompletionData being undefined appropriately.
| * @return {NavMenuItem} The filtered menu items based on the user roles | ||
| */ | ||
| export declare const filterRoutesByUserRole: (routes: NavMenuItem) => any; | ||
| export declare const filterRoutesByUserRole: (routes: NavMenuItem) => NavMenuItem | undefined; |
There was a problem hiding this comment.
[❗❗ correctness]
The return type has been changed from any to NavMenuItem | undefined. Ensure that all usages of filterRoutesByUserRole handle the possibility of an undefined return value to prevent runtime errors.
| export type fetchUserProfileFn = () => AuthUser | null; | ||
| export declare const getJwtUserhandle: () => AuthUser["handle"] | undefined; | ||
| export declare const getJwtUserRoles: () => AuthUser["roles"] | undefined; | ||
| export declare const getUserAppRoles: () => AuthUser["roles"]; |
There was a problem hiding this comment.
[❗❗ correctness]
The return type of getUserAppRoles has been changed from AuthUser['roles'] | undefined to AuthUser['roles']. Ensure that this change is intentional and that the function will not encounter scenarios where undefined was previously returned, as this could lead to runtime errors.
| * @returns Promise<AuthUser> | ||
| */ | ||
| export declare const fetchUserProfile: () => Promise<AuthUser>; | ||
| export declare const fetchUserProfile: () => Promise<AuthUser | undefined>; |
There was a problem hiding this comment.
[❗❗ correctness]
The return type of fetchUserProfile has been changed from Promise<AuthUser> to Promise<AuthUser | undefined>. Verify that all consumers of this function handle the undefined case appropriately to prevent potential runtime errors.
| @@ -1,3 +1,3 @@ | |||
| export declare const getCookieValue: (name: string) => any; | |||
| export declare const checkCookie: (cookieName: string, value?: any) => boolean; | |||
| export declare const checkCookie: (cookieName: string, value?: any) => true | undefined; | |||
There was a problem hiding this comment.
[❗❗ correctness]
The return type of checkCookie has been changed from boolean to true | undefined. This change implies that the function can no longer return false, which might affect the logic depending on this function elsewhere in the codebase. Ensure that all usages of checkCookie are reviewed to confirm they handle the new return type correctly.
| * @returns NavMenuItem | ||
| */ | ||
| export declare const filterRoutes: (navMenuItem: NavMenuItem, filter: (n: NavMenuItem) => boolean, depth?: number) => any; | ||
| export declare const filterRoutes: (navMenuItem: NavMenuItem, filter: (n: NavMenuItem) => boolean, depth?: number) => NavMenuItem | undefined; |
There was a problem hiding this comment.
[❗❗ correctness]
The return type of filterRoutes has been changed from any to NavMenuItem | undefined. This is a positive change for type safety and clarity. However, ensure that all usages of filterRoutes are updated to handle the undefined case appropriately to prevent potential runtime errors.
https://topcoder.atlassian.net/browse/PM-2749 - UniNavi - Update NPM Modules to Latest Versions (major upgrade except svelte, where the syntax has changed greatly and would require rewrite).
Also removed the "nudge" functionality (not used for a long time).