WIP: Bazza UI Filters #57
Conversation
- Created .env.example file to outline required API keys for various providers. - Updated .gitignore to include additional files and directories related to development and environment variables. - Added .taskmasterconfig file for project configuration settings. - Introduced new Cursor rules for maintaining consistency in development workflows and self-improvement guidelines. - Added new UI components including Calendar, Checkbox, Dialog, Popover, Slider, and Tabs with respective functionalities. - Implemented data table filter components and utilities for enhanced filtering capabilities. - Updated package dependencies to ensure compatibility and performance improvements.
|
|
WalkthroughThis update introduces a comprehensive data table filtering system, new documentation, configuration files, and several UI component enhancements. Major additions include a robust, type-safe filtering framework for data tables, new React components (checkbox, calendar, dialog, slider, tabs), workflow and rule documentation, and environment/configuration templates. Dependency updates and utility hooks for debounce, filter sync, and query management are also included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FilterUI
participant useDataTableFilters
participant DataTable
participant QueryClient
User->>FilterUI: Interacts with filter controls
FilterUI->>useDataTableFilters: Updates filter state/actions
useDataTableFilters->>DataTable: Provides filtered columns/data
DataTable->>QueryClient: Fetches filtered data (server-driven)
QueryClient-->>DataTable: Returns filtered/paginated data
DataTable-->>User: Renders updated table
sequenceDiagram
participant Dev
participant TaskmasterCLI
participant MCPServer
participant ProjectRepo
Dev->>TaskmasterCLI: Run task commands (init, list, analyze, etc.)
TaskmasterCLI->>MCPServer: (Optional) Use MCP tools for automation
TaskmasterCLI->>ProjectRepo: Reads/writes tasks, config, .env
ProjectRepo-->>Dev: Updates files, configuration, and docs
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
📝 Storybook Preview: View Storybook This preview will be updated automatically when you push new changes to this PR.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 45
🔭 Outside diff range comments (1)
apps/docs/src/lib/storybook/react-router-stub.tsx (1)
96-101: 🛠️ Refactor suggestionUpdate withURLState to support the new providers
The
withURLStatedecorator callswithReactRouterStubDecoratorbut doesn't pass any configuration for the new providers.Update the withURLState decorator to support configuring the new providers:
/** * A decorator that provides URL state management for stories * * Use this when you need URL query parameters in your stories */ -export const withURLState = (initialPath = '/'): Decorator => { +export const withURLState = ( + initialPath = '/', + options: Omit<RemixStubOptions, 'routes' | 'initialPath'> = {} +): Decorator => { return withReactRouterStubDecorator({ routes: [{ path: '/' }], initialPath, + ...options }); };
🧹 Nitpick comments (43)
.gitignore (1)
188-190: Consolidate and correct log ignore patterns.
You’ve added standard log patterns (*.log,npm-debug.log*), which correctly capture log files. To avoid confusion, consider removing the earlier mis-typed entries (_.log,npm-debug.log_) so the ignore list remains concise and accurate..cursor/rules/dev_workflow.mdc (1)
100-100: Update or remove reference to missing link.The line mentions "task structure details (previously linked to
tasks.mdc)" which indicates a link that may have been removed or changed. This could confuse readers expecting to find this link.-Refer to task structure details (previously linked to `tasks.mdc`). +Refer to task structure details outlined above..cursor/rules/taskmaster.mdc (3)
12-14: Complete the incomplete statement about AI-powered tools.The sentence starts by listing AI-powered tools but doesn't complete the thought or explain the implications for users (such as potential wait times, API key requirements, etc.)
-**Important:** Several MCP tools involve AI processing... The AI-powered tools include `parse_prd`, `analyze_project_complexity`, `update_subtask`, `update_task`, `update`, `expand_all`, `expand_task`, and `add_task`. +**Important:** Several MCP tools involve AI processing which may require a wait time of up to one minute and appropriate API keys configured in your environment. The AI-powered tools include `parse_prd`, `analyze_project_complexity`, `update_subtask`, `update_task`, `update`, `expand_all`, `expand_task`, and `add_task`.
40-40: Clarify what "PRD" stands for.The term "PRD" is used without being defined. While it's a common acronym in product development, it would be helpful to explicitly mention that it stands for "Product Requirements Document" for users who might not be familiar with the term.
-**Important:** Once complete, you *MUST* parse a prd in order to generate tasks. There will be no tasks files until then. The next step after initializing should be to create a PRD using the example PRD in scripts/example_prd.txt. +**Important:** Once complete, you *MUST* parse a PRD (Product Requirements Document) in order to generate tasks. There will be no tasks files until then. The next step after initializing should be to create a PRD using the example PRD in scripts/example_prd.txt.
378-379: Provide an example of API key configuration in environment files.While the document mentions setting API keys in
.envor.cursor/mcp.jsonfiles, it would be helpful to provide a brief example of the expected format to ensure users configure these correctly.-**Set API keys** in your **`.env`** file in the project root (for CLI use) or within the `env` section of your **`.cursor/mcp.json`** file (for MCP/Cursor integration). All other settings (model choice, max tokens, temperature, log level, custom endpoints) are managed in `.taskmasterconfig` via `task-master models` command or `models` MCP tool. +**Set API keys** in your **`.env`** file in the project root (for CLI use) or within the `env` section of your **`.cursor/mcp.json`** file (for MCP/Cursor integration). + +Example `.env` file: +``` +ANTHROPIC_API_KEY=sk-ant-xxxxx +OPENAI_API_KEY=sk-xxxxx +``` + +Example `.cursor/mcp.json` env section: +```json +{ + "env": { + "ANTHROPIC_API_KEY": "sk-ant-xxxxx", + "OPENAI_API_KEY": "sk-xxxxx" + } +} +``` + +All other settings (model choice, max tokens, temperature, log level, custom endpoints) are managed in `.taskmasterconfig` via `task-master models` command or `models` MCP tool.packages/components/src/ui/data-table-filter/lib/i18n.ts (1)
3-9: Consider stronger typing for translation keys
Right nowTranslationsis aRecord<string, string>, which may allow typos in lookup keys. You could derive the key union from the JSON file or define a literal type for keys to get compile-time safety.packages/components/src/ui/utils/use-filter-sync.ts (1)
15-15: Consider implementing the TODO for debouncingThe TODO comment indicates that debouncing might be needed for this hook, which is an important performance consideration for URL-synced filters.
I recommend implementing the debounce functionality to prevent frequent URL updates when filters change rapidly:
export function useFilterSync() { const [filters, setFilters] = useQueryState<FiltersState>( 'filters', // The query parameter key parseAsJson(filtersArraySchema.parse) // Now filtersArraySchema should be defined - .withDefault([]), - // TODO: Add debouncing if needed, e.g., .withOptions({ history: 'push', shallow: false, debounce: 300 }) + .withDefault([]) + .withOptions({ history: 'push', shallow: false, debounce: 300 }), ); return [filters, setFilters] as const; }packages/components/src/ui/data-table-filter/components/filter-subject.tsx (1)
12-15: Consider making the styling more customizableThe component currently has hardcoded styling classes for the filter subject display.
For better reusability, consider allowing for custom className props:
interface FilterSubjectProps<TData, TType extends ColumnDataType> { column: Column<TData, TType> + className?: string + iconClassName?: string + textClassName?: string } export function FilterSubject<TData, TType extends ColumnDataType>({ column, + className, + iconClassName, + textClassName, }: FilterSubjectProps<TData, TType>) { const hasIcon = !!column.icon return ( - <span className="flex select-none items-center gap-1 whitespace-nowrap px-2 font-medium"> - {hasIcon && <column.icon className="size-4 stroke-[2.25px]" />} - <span>{column.displayName}</span> + <span className={cn("flex select-none items-center gap-1 whitespace-nowrap px-2 font-medium", className)}> + {hasIcon && <column.icon className={cn("size-4 stroke-[2.25px]", iconClassName)} />} + <span className={textClassName}>{column.displayName}</span> </span> ) }You'll need to import the
cnutility:import type { Column, ColumnDataType } from '../core/types' + import { cn } from '../../utils'packages/components/src/ui/data-table-filter/components/filter-actions.tsx (1)
19-20: Consider adding error handling for the removeAllFilters action.The button click handler uses optional chaining (
actions?.removeAllFilters), which means it will silently do nothing ifactionsis undefined or the method fails. Consider adding error handling or feedback to the user when the clear action is performed.- onClick={actions?.removeAllFilters} + onClick={() => { + try { + actions?.removeAllFilters?.(); + } catch (error) { + console.error('Failed to clear filters:', error); + // Consider adding user feedback here + } + }}packages/components/src/ui/checkbox.tsx (3)
8-12: Consider extracting lengthy className into a constant or utility function.The className string is quite long and complex, making it harder to maintain. Consider extracting it into a constant or utility function for better readability and maintainability.
+const checkboxStyles = 'peer border-input dark:bg-input/30 data-[state=checked]:bg-primary data-[state=checked]:text-primary-foreground dark:data-[state=checked]:bg-primary data-[state=checked]:border-primary focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive size-4 shrink-0 rounded-[4px] border shadow-xs transition-shadow outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50'; function Checkbox({ className, ...props }: React.ComponentProps<typeof CheckboxPrimitive.Root>) { return ( <CheckboxPrimitive.Root data-slot="checkbox" - className={cn( - 'peer border-input dark:bg-input/30 data-[state=checked]:bg-primary data-[state=checked]:text-primary-foreground dark:data-[state=checked]:bg-primary data-[state=checked]:border-primary focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive size-4 shrink-0 rounded-[4px] border shadow-xs transition-shadow outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50', - className, - )} + className={cn(checkboxStyles, className)}
5-23: Add size variants for better flexibility.The component only supports a single size (size-4). Consider adding size variants to make the checkbox more flexible for different UI contexts.
-function Checkbox({ className, ...props }: React.ComponentProps<typeof CheckboxPrimitive.Root>) { +type CheckboxSize = 'sm' | 'md' | 'lg'; + +interface CheckboxProps extends React.ComponentProps<typeof CheckboxPrimitive.Root> { + size?: CheckboxSize; +} + +function Checkbox({ className, size = 'md', ...props }: CheckboxProps) { + const sizeClasses = { + sm: 'size-3', + md: 'size-4', + lg: 'size-5' + }; + + const iconSizeClasses = { + sm: 'size-2.5', + md: 'size-3.5', + lg: 'size-4' + }; return ( <CheckboxPrimitive.Root data-slot="checkbox" className={cn( - 'peer border-input dark:bg-input/30 data-[state=checked]:bg-primary data-[state=checked]:text-primary-foreground dark:data-[state=checked]:bg-primary data-[state=checked]:border-primary focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive size-4 shrink-0 rounded-[4px] border shadow-xs transition-shadow outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50', + 'peer border-input dark:bg-input/30 data-[state=checked]:bg-primary data-[state=checked]:text-primary-foreground dark:data-[state=checked]:bg-primary data-[state=checked]:border-primary focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive shrink-0 rounded-[4px] border shadow-xs transition-shadow outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50', + sizeClasses[size], className, )} {...props} > <CheckboxPrimitive.Indicator data-slot="checkbox-indicator" className="flex items-center justify-center text-current transition-none" > - <CheckIcon className="size-3.5" /> + <CheckIcon className={iconSizeClasses[size]} /> </CheckboxPrimitive.Indicator> </CheckboxPrimitive.Root> ); }
1-25: Add JSDoc comments for component documentation.The component lacks documentation explaining its purpose, props, and usage. Consider adding JSDoc comments to improve developer experience.
import * as CheckboxPrimitive from '@radix-ui/react-checkbox'; import { CheckIcon } from 'lucide-react'; import { cn } from './utils'; +/** + * A checkbox component built on top of Radix UI Checkbox primitive. + * Supports all the props of the underlying Radix UI component with consistent styling. + * + * @example + * ```tsx + * <Checkbox id="terms" /> + * <label htmlFor="terms">Accept terms and conditions</label> + * ``` + */ function Checkbox({ className, ...props }: React.ComponentProps<typeof CheckboxPrimitive.Root>) { // Component implementation... }packages/components/src/ui/utils/use-issues-query.ts (1)
31-37: Add pagination and query options support.The hook doesn't support pagination or customizable query options, which limits its flexibility. Consider adding support for pagination parameters and exposing more query options.
+interface UseIssuesQueryOptions { + apiUrl?: string; + page?: number; + pageSize?: number; + queryOptions?: Omit<UseQueryOptions<IssuesApiResponse, Error, IssuesApiResponse, string[]>, 'queryKey' | 'queryFn'>; +} /** * Custom hook to fetch issues data using TanStack Query, based on filter state. * * @param filters The current filter state. + * @param options Additional options for the query. * @returns The TanStack Query result object for the issues query. */ -export function useIssuesQuery(filters: FiltersState) { +export function useIssuesQuery(filters: FiltersState, options: UseIssuesQueryOptions = {}) { + const { apiUrl, page = 1, pageSize = 20, queryOptions = {} } = options; + return useQuery({ - queryKey: ['issues', filters], // Use filters in the query key for caching - queryFn: () => fetchIssues(filters), + queryKey: ['issues', filters, page, pageSize], // Include pagination in cache key + queryFn: () => fetchIssues(filters, { + apiUrl, + page, + pageSize + }), placeholderData: (previousData) => previousData, // Keep previous data while fetching // Consider adding options like staleTime, gcTime, refetchOnWindowFocus, etc. + ...queryOptions }); }packages/components/src/ui/data-table-filter/locales/en.json (1)
1-42: Organize translations with comments and consistent naming conventions.The localization file could benefit from better organization through comments and consistent naming conventions. Consider adding section comments and ensure consistent key naming patterns.
{ + /* General UI terms */ "clear": "Clear", "search": "Search...", "noresults": "No results.", "operators": "Operators", "filter": "Filter", "and": "and", "single": "Single", "range": "Range", "value": "Value", "min": "Min", "max": "Max", + + /* Option filter operators */ "filters.option.is": "is", "filters.option.isNot": "is not", "filters.option.isAnyOf": "is any of", "filters.option.isNoneOf": "is none of", + + /* Multi-option filter operators */ "filters.multiOption.include": "includes", "filters.multiOption.exclude": "excludes", "filters.multiOption.includeAnyOf": "includes any of", "filters.multiOption.excludeAllOf": "excludes all of", "filters.multiOption.includeAllOf": "includes all of", "filters.multiOption.excludeIfAnyOf": "excludes if any of", "filters.multiOption.excludeIfAll": "excludes if all of", + + /* Date filter operators */ "filters.date.is": "is", "filters.date.isNot": "is not", "filters.date.isBefore": "is before", "filters.date.isOnOrAfter": "is on or after", "filters.date.isAfter": "is after", "filters.date.isOnOrBefore": "is on or before", "filters.date.isBetween": "is between", "filters.date.isNotBetween": "is not between", + + /* Text filter operators */ "filters.text.contains": "contains", "filters.text.doesNotContain": "does not contain", + + /* Number filter operators */ "filters.number.is": "is", "filters.number.isNot": "is not", "filters.number.greaterThan": "greater than", "filters.number.greaterThanOrEqual": "greater than or equal", "filters.number.lessThan": "less than", "filters.number.lessThanOrEqual": "less than or equal", "filters.number.isBetween": "is between", "filters.number.isNotBetween": "is not between" }packages/components/src/ui/data-table-filter/components/data-table-filter.tsx (1)
16-21: Consider refactoring the double type assertions for better type safetyThe use of double type assertions (
as unknown as FilterOperators) for default operators bypasses TypeScript's type safety. This pattern is often used when direct type conversion isn't possible, but it can hide potential type errors.Consider defining your FilterOperators type to allow direct assignment or create a helper function that validates the strings at runtime:
// Define default operators based on column type const defaultOperatorMap: Record<string, FilterOperators> = { - option: 'is any of' as unknown as FilterOperators, // Use double assertion - text: 'contains' as unknown as FilterOperators, // Use double assertion - date: 'is' as unknown as FilterOperators, // Use double assertion - number: 'eq' as unknown as FilterOperators, // Use double assertion + option: 'is any of' as FilterOperators, + text: 'contains' as FilterOperators, + date: 'is' as FilterOperators, + number: 'eq' as FilterOperators, };packages/components/src/ui/data-table-filter/lib/memo.ts (1)
10-21: Remove or uncomment debug logging statementsThe code contains commented-out console.log statements that were likely used for debugging.
Either remove these commented logs or uncomment them and add a flag to enable/disable them:
+ const DEBUG = false; return () => { - // console.log(`[memo] Calling memoized function: ${options.key}`) + if (DEBUG) console.log(`[memo] Calling memoized function: ${options.key}`) const deps = getDeps() // If no previous deps or deps have changed, recompute if (!prevDeps || !shallowEqual(prevDeps, deps)) { - // console.log(`[memo] Cache MISS - ${options.key}`) + if (DEBUG) console.log(`[memo] Cache MISS - ${options.key}`) cachedResult = compute(deps) prevDeps = deps } else { - // console.log(`[memo] Cache HIT - ${options.key}`) + if (DEBUG) console.log(`[memo] Cache HIT - ${options.key}`) }packages/components/src/ui/slider.tsx (1)
13-16: Consider simplifying nested ternary operatorsThe nested ternary operators make the logic harder to read and understand.
Simplify the code with a more readable approach:
const _values = React.useMemo( - () => (Array.isArray(value) ? value : Array.isArray(defaultValue) ? defaultValue : [min, max]), + () => { + if (Array.isArray(value)) return value; + if (Array.isArray(defaultValue)) return defaultValue; + return [min, max]; + }, [value, defaultValue, min, max], );apps/docs/src/lib/storybook/react-router-stub.tsx (1)
79-87: Consider making providers configurableThe decorator now wraps the RouterProvider with multiple context providers, but without options to configure them.
Consider allowing configuration of these providers through options:
interface RemixStubOptions { routes: StubRouteObject[]; initialPath?: string; + queryClientOptions?: QueryClientConfig; + dayPickerInitialProps?: DayPickerProps; } export const withReactRouterStubDecorator = (options: RemixStubOptions): Decorator => { - const { routes, initialPath = '/' } = options; + const { + routes, + initialPath = '/', + queryClientOptions, + dayPickerInitialProps = {} + } = options; // ... return (Story, context) => { + const queryClient = new QueryClient(queryClientOptions); // ... return ( <QueryClientProvider client={queryClient}> - <DayPickerProvider initialProps={{}}> + <DayPickerProvider initialProps={dayPickerInitialProps}> <NuqsAdapter> <Command> <RouterProvider router={router} /> </Command> </NuqsAdapter> </DayPickerProvider> </QueryClientProvider> ); }; };packages/components/src/ui/data-table-filter/components/debounced-input.tsx (1)
1-3: Consider using a more specific import path.The import for TextInput uses a relative parent path that is a bit vague:
- import { TextInput } from '../..'; + import { TextInput } from '../../text-input';This makes the code more maintainable and explicit about its dependencies.
packages/components/src/ui/utils/filters.ts (2)
3-17: Consider adding documentation and validation constraints.The enum schemas are well-defined, but lack documentation and validation between types and operators.
Add JSDoc comments and consider defining constants for better developer experience:
+/** + * Represents the available filter operators. + */ +export const FILTER_OPERATORS = { + IS_ANY_OF: 'is any of', + IS_NONE_OF: 'is none of', + CONTAINS: 'contains', + DOES_NOT_CONTAIN: 'does not contain' +} as const; -export const filterOperatorSchema = z.enum([ - 'is any of', - 'is none of', - 'contains', - 'does not contain', - // Add other operators as needed -]); +export const filterOperatorSchema = z.enum(Object.values(FILTER_OPERATORS));Similar enhancement can be applied to the filter types.
19-24: Improve type safety for filter values.Currently, the
valuesfield allows a mix of types which could lead to type confusion.Consider making the values type more specific based on the filter type:
export const filterSchema = z.object({ columnId: z.string(), type: filterTypeSchema, operator: filterOperatorSchema, - values: z.array(z.union([z.string(), z.number(), z.boolean(), z.null()])), + values: z.array(z.union([z.string(), z.number(), z.boolean(), z.null()])) + .refine( + (values) => { + // Add validation rules here if needed + return true; + }, + { + message: 'Values must be compatible with the filter type' + } + ), });packages/components/src/ui/tabs.tsx (2)
22-33: Improve maintainability of the TabsTrigger styles.The className for TabsTrigger is very long and hard to maintain.
Break down the long class name string into logical groups:
function TabsTrigger({ className, ...props }: React.ComponentProps<typeof TabsPrimitive.Trigger>) { + const baseStyles = "inline-flex h-[calc(100%-1px)] flex-1 items-center justify-center gap-1.5 rounded-md"; + const textStyles = "text-sm font-medium whitespace-nowrap text-foreground dark:text-muted-foreground"; + const borderStyles = "border border-transparent"; + const stateStyles = "data-[state=active]:bg-background dark:data-[state=active]:text-foreground dark:data-[state=active]:border-input dark:data-[state=active]:bg-input/30 data-[state=active]:shadow-sm"; + const focusStyles = "focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px] focus-visible:outline-1 focus-visible:outline-ring"; + const interactionStyles = "transition-[color,box-shadow] disabled:pointer-events-none disabled:opacity-50"; + const iconStyles = "[&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4"; + const spacingStyles = "px-2 py-1"; return ( <TabsPrimitive.Trigger data-slot="tabs-trigger" className={cn( - "data-[state=active]:bg-background dark:data-[state=active]:text-foreground focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:outline-ring dark:data-[state=active]:border-input dark:data-[state=active]:bg-input/30 text-foreground dark:text-muted-foreground inline-flex h-[calc(100%-1px)] flex-1 items-center justify-center gap-1.5 rounded-md border border-transparent px-2 py-1 text-sm font-medium whitespace-nowrap transition-[color,box-shadow] focus-visible:ring-[3px] focus-visible:outline-1 disabled:pointer-events-none disabled:opacity-50 data-[state=active]:shadow-sm [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4", + baseStyles, + textStyles, + borderStyles, + stateStyles, + focusStyles, + interactionStyles, + iconStyles, + spacingStyles, className, )} {...props} /> ); }
1-4: Missing JSDoc comments for component documentation.Add JSDoc comments to explain each component's purpose and usage:
import * as TabsPrimitive from '@radix-ui/react-tabs'; import type * as React from 'react'; import { cn } from './utils'; +/** + * Tabs component based on Radix UI's tabs primitive. + * Used to organize content into tab panels where only one panel is visible at a time. + */ function Tabs({ className, ...props }: React.ComponentProps<typeof TabsPrimitive.Root>) {packages/components/src/ui/button.tsx (2)
34-47: Good refactoring but JSDoc documentation would be helpful.The Button component implementation is now more concise, but lacks documentation to explain its props and usage.
Add JSDoc comments to improve developer experience:
+/** + * Button component with various style variants and size options. + * + * @param variant - The visual style of the button + * @param size - The size of the button + * @param asChild - When true, the component will render its child directly with the button's props applied + */ function Button({ className, variant, size, asChild = false, ...props }: ComponentProps<'button'> & VariantProps<typeof buttonVariants> & { asChild?: boolean; }) { const Comp = asChild ? Slot : 'button'; return <Comp data-slot="button" className={cn(buttonVariants({ variant, size, className }))} {...props} />; }
6-8: Consider extracting long className string to improve maintainability.The base className for buttonVariants is quite lengthy. Consider breaking it into logical groups:
+const baseStyles = "inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md text-sm font-medium transition-all"; +const disabledStyles = "disabled:pointer-events-none disabled:opacity-50"; +const iconStyles = "[&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 shrink-0 [&_svg]:shrink-0"; +const focusStyles = "outline-none focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px]"; +const validationStyles = "aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive"; const buttonVariants = cva( - "inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md text-sm font-medium transition-all disabled:pointer-events-none disabled:opacity-50 [&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 shrink-0 [&_svg]:shrink-0 outline-none focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px] aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive", + [baseStyles, disabledStyles, iconStyles, focusStyles, validationStyles].join(" "), {packages/components/src/ui/data-table-filter/components/filter-operator.tsx (3)
87-91: Address TODO commentThere's a TODO comment to reduce the multiple type-specific filter operator controllers into a single component.
Consider implementing a more generic controller using a strategy pattern or factory pattern to handle different data types with a single component implementation, reducing code duplication.
99-152: Simplify the controller switch statementThe current implementation uses a switch statement with repetitive type casting for each case.
You could simplify this by:
- Creating a mapping of column types to controller components
- Using a more generic approach with a component factory
Example:
const operatorControllers = { option: FilterOperatorOptionController, multiOption: FilterOperatorMultiOptionController, date: FilterOperatorDateController, text: FilterOperatorTextController, number: FilterOperatorNumberController } as const; export function FilterOperatorController<TData, TType extends ColumnDataType>({ filter, column, actions, closeController, locale = 'en', }: FilterOperatorControllerProps<TData, TType>) { const ControllerComponent = operatorControllers[column.type]; if (!ControllerComponent) return null; return ( <ControllerComponent filter={filter as any} // Type casting still needed but in one place column={column as any} actions={actions} closeController={closeController} locale={locale} /> ); }
291-291: Inconsistent onSelect handlingIn
FilterOperatorNumberController, theonSelectprop uses a callback function, while other controllers pass the value directly.- <CommandItem onSelect={() => changeOperator(r.value)} value={r.value} key={r.value}> + <CommandItem onSelect={changeOperator} value={r.value} key={r.value}>This would make it consistent with the other controllers and simplify the code.
packages/components/src/ui/data-table-filter/lib/array.ts (1)
2-2: Add missing semicolonLine ends without a semicolon, which is inconsistent with most of the other statements in the file.
- return a.filter((x) => b.includes(x)) + return a.filter((x) => b.includes(x));packages/components/src/ui/data-table-filter/lib/helpers.ts (3)
14-21: Simplify the conditional logicThe function handles single values well, but the handling of multiple values could be more explicit.
export function createNumberFilterValue( values: number[] | undefined, ): number[] { if (!values || values.length === 0) return [] if (values.length === 1) return [values[0]] if (values.length === 2) return createNumberRange(values) - return [values[0], values[1]] + // If more than 2 values, use only the first two and create a range + return createNumberRange([values[0], values[1]]) }
39-54: Simplify the number range creation logicThe current implementation has redundant code that can be simplified.
export function createNumberRange(values: number[] | undefined) { - let a = 0 - let b = 0 - - if (!values || values.length === 0) return [a, b] - if (values.length === 1) { - a = values[0] - } else { - a = values[0] - b = values[1] - } + if (!values || values.length === 0) return [0, 0] + + const a = values[0] + const b = values.length > 1 ? values[1] : 0 const [min, max] = a < b ? [a, b] : [b, a] return [min, max] }
73-90: Check Map value types more efficientlyThe current implementation checks keys and values separately, which could be more efficient.
export function isColumnOptionMap( value: unknown, ): value is Map<string, number> { if (!(value instanceof Map)) { return false } - for (const key of value.keys()) { - if (typeof key !== 'string') { - return false - } - } - for (const val of value.values()) { - if (typeof val !== 'number') { - return false - } - } + for (const [key, val] of value.entries()) { + if (typeof key !== 'string' || typeof val !== 'number') { + return false + } + } return true }packages/components/src/ui/dialog.tsx (1)
3-6: Use explicit type importsThe static analysis tool suggests using explicit type imports.
-import * as React from "react" +import type * as React from "react" import * as DialogPrimitive from "@radix-ui/react-dialog" import { XIcon } from "lucide-react"🧰 Tools
🪛 Biome (1.9.4)
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
packages/components/src/ui/data-table-filter/core/operators.ts (1)
380-407: Simplify complex value length calculationThe nested ternary expressions for calculating array lengths are difficult to read.
export function determineNewOperator<TType extends ColumnDataType>( type: TType, oldVals: FilterValues<TType>, nextVals: FilterValues<TType>, currentOperator: FilterOperators[TType], ): FilterOperators[TType] { - const a = - Array.isArray(oldVals) && Array.isArray(oldVals[0]) - ? oldVals[0].length - : oldVals.length - const b = - Array.isArray(nextVals) && Array.isArray(nextVals[0]) - ? nextVals[0].length - : nextVals.length + // Get effective length, considering nested arrays + function getEffectiveLength(vals: any[]): number { + return Array.isArray(vals) && Array.isArray(vals[0]) ? vals[0].length : vals.length + } + + const a = getEffectiveLength(oldVals) + const b = getEffectiveLength(nextVals) // If filter size has not transitioned from single to multiple (or vice versa) // or is unchanged, return the current operator. if (a === b || (a >= 2 && b >= 2) || (a <= 1 && b <= 1)) return currentOperator const opDetails = filterTypeOperatorDetails[type][currentOperator] // Handle transition from single to multiple filter values. if (a < b && b >= 2) return opDetails.singularOf ?? currentOperator // Handle transition from multiple to single filter values. if (a > b && b <= 1) return opDetails.pluralOf ?? currentOperator return currentOperator }apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx (1)
282-285: Memo dependencies re-compute on every render
useMemo(() => loaderData?.data ?? [], [loaderData?.data])– the dependency[loaderData?.data]is a new reference every loader refresh, but also whenever Storybook re-mounts. IfloaderDatais unchanged, React still treats the dependency as different and busts the memo.Safer:
-const data = useMemo(() => loaderData?.data ?? [], [loaderData?.data]); +const data = useMemo(() => loaderData?.data ?? [], [loaderData]);or memoise in the loader itself so callers receive a stable reference.
packages/components/src/ui/utils/use-data-table-filters.tsx (2)
110-180: Large duplication between option and multiOption branches
addFilterValue/removeFilterValuecontain nearly identical blocks for'option'and'multiOption'. This inflates bundle size and increases maintenance cost.Refactor by extracting the common algorithm:
- if (column.type === 'option') { - … big block … - } - if (column.type === 'multiOption') { - … almost same big block … - } +if (column.type === 'option' || column.type === 'multiOption') { + handleOptionLikeFilter(column, values, /* add | remove */); + return; +}The helper can parameterise
addUniq/removeUniqand the operator map.
62-64: Controlled/uncontrolled guard throws at runtimeThe invariant check executes on every render. In a strict-mode dev build React double-invokes components, so you’ll see the exception twice, polluting logs.
Move the guard into a
useEffect(() => { … }, [])or derive aconst isControlled = externalFilters !== undefinedboolean and throw only when the mode flips between renders (following React’s controlled component guidelines).packages/components/src/ui/data-table-filter/core/filters.ts (2)
161-165: Unsafe non-null assertions (!) can be replaced by optional chainingInside the guarded block
if (column.orderFn) { … }you can safely callcolumn.orderFnvia the variable you just checked without!:- models = models.sort((m1, m2) => - column.orderFn!(m1 as ElementType<NonNullable<TVal>>, m2 as ElementType<NonNullable<TVal>>), - ); + models = models.sort((m1, m2) => + column.orderFn?.(m1 as ElementType<NonNullable<TVal>>, m2 as ElementType<NonNullable<TVal>>)!, + );This removes Biome warnings (
noNonNullAssertion) and prevents accidental runtime throws if the function becomes undefined between check and call.🧰 Tools
🪛 Biome (1.9.4)
[error] 163-163: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
318-334: Private “prefetched” caches should be truly privateThe
_prefetched*Cacheproperties are exposed on the publiccolumnobject.
Consumers could mutate them accidentally, breaking memoisation logic.Consider using a
WeakMap<Column, Cache>inside the factory or prefix with#(private class fields) if you transpile to a target that supports them.packages/components/src/ui/data-table-filter/components/filter-value.tsx (3)
61-65: Tailwind class with parentheses is invalid – replace with bracket syntax
origin-(--radix-popover-content-transform-origin)is not parsed by Tailwind. Use the arbitrary-value syntax instead:-className="w-fit p-0 origin-(--radix-popover-content-transform-origin)" +className="w-fit p-0 origin-[var(--radix-popover-content-transform-origin)]"This guarantees the computed transform-origin is applied.
638-651: Removeanycasts by tightening the generic
actions.setFilterValueis already generic – castingcolumn as anyforfeits type-safety and triggers Biome warnings.- setFilterValueDebounced(column as any, value); + setFilterValueDebounced(column, value);Apply the same change in the
changeMinNumberandchangeMaxNumberhelpers.🧰 Tools
🪛 Biome (1.9.4)
[error] 638-638: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 644-644: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 650-650: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
377-383:initialSelectedis never read insideOptionItem
It’s declared in the prop type and passed from the parent but not used, creating dead weight and misleading future readers. Either drop the field fromOptionItemPropsor utilise it (e.g. data-attribute for styling).packages/components/src/ui/data-table-filter/core/types.ts (1)
102-113: Defaulting generic parameters toanyhides real typing issues
Several generics (ColumnConfig,Column, etc.) default toany, defeating the purpose of the strong typing you established everywhere else. Consider defaulting tounknown(safer) or removing the default so callers must be explicit.- TType extends ColumnDataType = any, + TType extends ColumnDataType,and similar for other type aliases. This will eliminate the Biome “Unexpected any” warnings and surface incorrect usages early.
🧰 Tools
🪛 Biome (1.9.4)
[error] 104-104: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
-
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (44)
-
.cursor/rules/cursor_rules.mdc(1 hunks) -
.cursor/rules/dev_workflow.mdc(1 hunks) -
.cursor/rules/self_improve.mdc(1 hunks) -
.cursor/rules/taskmaster.mdc(1 hunks) -
.env.example(1 hunks) -
.gitignore(1 hunks) -
.taskmasterconfig(1 hunks) -
apps/docs/src/lib/storybook/react-router-stub.tsx(4 hunks) -
apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx(1 hunks) -
packages/components/package.json(2 hunks) -
packages/components/src/ui/button.tsx(2 hunks) -
packages/components/src/ui/calendar.tsx(1 hunks) -
packages/components/src/ui/checkbox.tsx(1 hunks) -
packages/components/src/ui/data-table-filter/components/active-filters.tsx(1 hunks) -
packages/components/src/ui/data-table-filter/components/data-table-filter.tsx(1 hunks) -
packages/components/src/ui/data-table-filter/components/debounced-input.tsx(1 hunks) -
packages/components/src/ui/data-table-filter/components/filter-actions.tsx(1 hunks) -
packages/components/src/ui/data-table-filter/components/filter-operator.tsx(1 hunks) -
packages/components/src/ui/data-table-filter/components/filter-selector.tsx(1 hunks) -
packages/components/src/ui/data-table-filter/components/filter-subject.tsx(1 hunks) -
packages/components/src/ui/data-table-filter/components/filter-value.tsx(1 hunks) -
packages/components/src/ui/data-table-filter/core/filters.ts(1 hunks) -
packages/components/src/ui/data-table-filter/core/operators.ts(1 hunks) -
packages/components/src/ui/data-table-filter/core/types.ts(1 hunks) -
packages/components/src/ui/data-table-filter/index.tsx(1 hunks) -
packages/components/src/ui/data-table-filter/lib/array.ts(1 hunks) -
packages/components/src/ui/data-table-filter/lib/debounce.ts(1 hunks) -
packages/components/src/ui/data-table-filter/lib/filter-fns.ts(1 hunks) -
packages/components/src/ui/data-table-filter/lib/helpers.ts(1 hunks) -
packages/components/src/ui/data-table-filter/lib/i18n.ts(1 hunks) -
packages/components/src/ui/data-table-filter/lib/memo.ts(1 hunks) -
packages/components/src/ui/data-table-filter/locales/en.json(1 hunks) -
packages/components/src/ui/data-table/data-table-faceted-filter.tsx(3 hunks) -
packages/components/src/ui/dialog.tsx(1 hunks) -
packages/components/src/ui/popover.tsx(2 hunks) -
packages/components/src/ui/select.tsx(5 hunks) -
packages/components/src/ui/slider.tsx(1 hunks) -
packages/components/src/ui/tabs.tsx(1 hunks) -
packages/components/src/ui/utils/filters.ts(1 hunks) -
packages/components/src/ui/utils/use-data-table-filters.tsx(1 hunks) -
packages/components/src/ui/utils/use-debounce-callback.tsx(1 hunks) -
packages/components/src/ui/utils/use-filter-sync.ts(1 hunks) -
packages/components/src/ui/utils/use-issues-query.ts(1 hunks) -
packages/components/src/ui/utils/use-unmount.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (17)
packages/components/src/ui/select.tsx (1)
packages/components/src/ui/utils/index.ts (1)
cn(4-6)
packages/components/src/ui/utils/use-filter-sync.ts (3)
packages/components/src/ui/data-table-filter/core/types.ts (1)
FiltersState(294-294)packages/components/src/ui/utils/filters.ts (2)
FiltersState(31-31)filtersArraySchema(26-26)packages/components/src/remix-hook-form/data-table-router-parsers.ts (1)
parseAsJson(47-62)
packages/components/src/ui/data-table/data-table-faceted-filter.tsx (2)
packages/components/src/ui/command.tsx (6)
CommandInput(101-101)CommandList(102-102)CommandEmpty(103-103)CommandGroup(104-104)CommandItem(105-105)CommandSeparator(107-107)packages/components/src/ui/utils/index.ts (1)
cn(4-6)
packages/components/src/ui/checkbox.tsx (1)
packages/components/src/ui/utils/index.ts (1)
cn(4-6)
packages/components/src/ui/utils/use-issues-query.ts (2)
packages/components/src/ui/data-table-filter/core/types.ts (1)
FiltersState(294-294)packages/components/src/ui/utils/filters.ts (1)
FiltersState(31-31)
packages/components/src/ui/slider.tsx (1)
packages/components/src/ui/utils/index.ts (1)
cn(4-6)
packages/components/src/ui/data-table-filter/components/filter-actions.tsx (4)
packages/components/src/ui/data-table-filter/core/types.ts (1)
DataTableFilterActions(204-228)packages/components/src/ui/data-table-filter/lib/i18n.ts (2)
Locale(3-3)t(11-13)packages/components/src/ui/button.tsx (1)
Button(49-49)packages/components/src/ui/utils/index.ts (1)
cn(4-6)
packages/components/src/ui/data-table-filter/components/filter-subject.tsx (1)
packages/components/src/ui/data-table-filter/core/types.ts (2)
ColumnDataType(39-47)Column(192-198)
packages/components/src/ui/data-table-filter/components/debounced-input.tsx (2)
packages/components/src/ui/data-table-filter/lib/debounce.ts (1)
debounce(13-138)packages/components/src/ui/text-input.tsx (1)
TextInput(20-20)
packages/components/src/ui/utils/filters.ts (1)
packages/components/src/ui/data-table-filter/core/types.ts (1)
FiltersState(294-294)
packages/components/src/ui/button.tsx (1)
packages/components/src/ui/utils/index.ts (1)
cn(4-6)
packages/components/src/ui/tabs.tsx (1)
packages/components/src/ui/utils/index.ts (1)
cn(4-6)
packages/components/src/ui/calendar.tsx (2)
packages/components/src/ui/utils/index.ts (1)
cn(4-6)packages/components/src/ui/button.tsx (1)
buttonVariants(49-49)
packages/components/src/ui/data-table-filter/lib/helpers.ts (1)
packages/components/src/ui/data-table-filter/core/types.ts (2)
Column(192-198)ColumnOption(22-29)
packages/components/src/ui/data-table-filter/core/operators.ts (1)
packages/components/src/ui/data-table-filter/core/types.ts (6)
ColumnDataType(39-47)FilterOperatorTarget(303-303)FilterOperators(270-276)FilterDetails(299-301)FilterTypeOperatorDetails(350-352)FilterValues(72-74)
packages/components/src/ui/dialog.tsx (1)
packages/components/src/ui/utils/index.ts (1)
cn(4-6)
packages/components/src/ui/data-table-filter/lib/filter-fns.ts (3)
packages/components/src/ui/data-table-filter/core/types.ts (1)
FilterModel(287-292)packages/components/src/ui/data-table-filter/lib/array.ts (1)
intersection(1-3)packages/components/src/ui/data-table-filter/core/operators.ts (1)
dateFilterOperators(136-227)
🪛 Biome (1.9.4)
packages/components/src/ui/data-table-filter/lib/memo.ts
[error] 1-1: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 23-23: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
packages/components/src/ui/data-table-filter/components/filter-selector.tsx
[error] 52-52: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
packages/components/src/ui/slider.tsx
[error] 45-45: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
packages/components/src/ui/utils/use-debounce-callback.tsx
[error] 17-17: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 22-22: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
packages/components/src/ui/calendar.tsx
[error] 11-11: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
packages/components/src/ui/data-table-filter/components/filter-value.tsx
[error] 638-638: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 644-644: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 650-650: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 416-416: This hook does not specify all of its dependencies: column.getFacetedUniqueValues
This dependency is not specified in the hook dependency list.
Either include it or remove the dependency array
(lint/correctness/useExhaustiveDependencies)
[error] 416-416: This hook does not specify all of its dependencies: column.getOptions
This dependency is not specified in the hook dependency list.
Either include it or remove the dependency array
(lint/correctness/useExhaustiveDependencies)
[error] 416-416: This hook does not specify all of its dependencies: filter?.values.includes
This dependency is not specified in the hook dependency list.
This dependency is not specified in the hook dependency list.
Either include them or remove the dependency array
(lint/correctness/useExhaustiveDependencies)
[error] 480-480: This hook does not specify all of its dependencies: column.getFacetedUniqueValues
This dependency is not specified in the hook dependency list.
Either include it or remove the dependency array
(lint/correctness/useExhaustiveDependencies)
[error] 480-480: This hook does not specify all of its dependencies: column.getOptions
This dependency is not specified in the hook dependency list.
Either include it or remove the dependency array
(lint/correctness/useExhaustiveDependencies)
[error] 480-480: This hook does not specify all of its dependencies: filter?.values.includes
This dependency is not specified in the hook dependency list.
Either include it or remove the dependency array
(lint/correctness/useExhaustiveDependencies)
[error] 653-653: This hook does not specify all of its dependencies: setFilterOperatorDebounced.cancel
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
[error] 653-653: This hook does not specify all of its dependencies: setFilterValueDebounced.cancel
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
packages/components/src/ui/dialog.tsx
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
packages/components/src/ui/utils/use-data-table-filters.tsx
[error] 30-30: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 30-30: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 30-30: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 48-48: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 48-48: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 48-48: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 273-273: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
packages/components/src/ui/data-table-filter/lib/array.ts
[error] 11-11: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 28-29: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 54-54: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 54-54: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 104-105: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
packages/components/src/ui/data-table-filter/core/filters.ts
[error] 19-19: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 36-36: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 42-42: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 53-53: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 63-63: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 72-72: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 83-83: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 94-94: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 105-105: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 163-163: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
[error] 171-171: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
[error] 213-213: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
[error] 290-290: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 290-290: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 290-290: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 296-296: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 296-296: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 300-300: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 308-308: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 308-308: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
packages/components/src/ui/data-table-filter/core/types.ts
[error] 104-104: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 135-135: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 135-135: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 135-135: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 135-135: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 150-150: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 150-150: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 150-150: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 150-150: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 194-194: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 287-287: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
packages/components/src/ui/data-table-filter/lib/debounce.ts
[error] 13-13: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 13-13: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 21-21: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 93-93: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
🪛 GitHub Actions: Run Tests
packages/components/src/ui/data-table-filter/components/filter-selector.tsx
[error] 61-61: TypeScript error TS2322: Type '(value: string, search: string, keywords: string[]) => 0 | 1' is not assignable to type 'CommandFilter'. Parameter 'keywords' type mismatch: 'string[] | undefined' is not assignable to 'string[]'.
[error] 66-66: TypeScript error TS2322: Property 'ref' does not exist on type of 'CommandInput' component props.
[error] 156-156: TypeScript error TS2322: Property 'ref' does not exist on type of 'Children' component props.
packages/components/src/ui/utils/use-debounce-callback.tsx
[error] 31-31: TypeScript error TS2339: Property 'cancel' does not exist on type '(...args: any[]) => void'.
[error] 36-36: TypeScript error TS2554: Expected 2 arguments, but got 3 in debounce function call.
[error] 38-38: TypeScript error TS2322: Type '{ (...args: Parameters): void; cancel(): void; isPending(): boolean; flush(): any; }' is not assignable to type 'DebouncedState'. Return type mismatch.
[error] 43-43: TypeScript error TS2339: Property 'cancel' does not exist on type '(...args: Parameters) => void'.
[error] 51-51: TypeScript error TS2339: Property 'flush' does not exist on type '(...args: Parameters) => void'.
[error] 59-59: TypeScript error TS2554: Expected 2 arguments, but got 3 in debounce function call.
packages/components/src/ui/data-table-filter/components/filter-value.tsx
[error] 572-572: TypeScript error TS2322: Type '(value: DateRange | undefined) => void' is not assignable to type 'ReactEventHandler'. Parameter type mismatch: expected SyntheticEvent but got DateRange.
packages/components/src/ui/dialog.tsx
[error] 7-7: TypeScript error TS2307: Cannot find module 'src/lib/utils' or its corresponding type declarations.
[error] 1-1: Build failed: Rollup failed to resolve import 'src/lib/utils'. This may break the application at runtime. Consider adding it to build.rollupOptions.external.
packages/components/src/ui/data-table-filter/lib/debounce.ts
[error] 19-19: TypeScript error TS2503: Cannot find namespace 'NodeJS'.
[error] 54-54: TypeScript error TS2503: Cannot find namespace 'NodeJS'.
🔇 Additional comments (57)
.gitignore (1)
185-186: Verify the necessity of the.react-router/ignore entry.
The project does not typically generate a.react-router/directory. Confirm whether this directory is produced during build or development and adjust or remove the ignore rule accordingly..taskmasterconfig (1)
1-31: Configuration looks solid.
The JSON structure correctly defines the AI models and global settings, and aligns with the environment variables in.env.example. No changes needed..env.example (1)
1-8: Environment variable placeholders are clear and comprehensive.
All required and optional API keys for various providers are listed with appropriate comments, matching the models configured in.taskmasterconfig..cursor/rules/cursor_rules.mdc (7)
1-5: Front matter is correctly structured.
The YAML header (description,globs,alwaysApply) follows the prescribed format and will apply to all Cursor rule files.
7-18: Required Rule Structure example is clear.
The markdown snippet demonstrates the correct front matter and content formatting, including bold main points and sub-points within a code block.
20-24: File References guidelines are accurate.
Link syntax ([filename](mdc:path/to/file)) and examples correctly show how to reference both rule and code files.
25-33: Code Examples section is well-defined.
The TypeScript snippet with DO/DON’T comments effectively illustrates the expected style and anti-patterns.
35-41: Rule Content Guidelines are comprehensive.
This section clearly lists actionable steps for writing rules, including overviews, examples, and DRY principles.
42-47: Rule Maintenance directives are thorough.
The guidance on updating, adding examples, and cross-referencing related rules is well articulated.
48-53: Best Practices summary is concise.
Key recommendations on formatting, example usage, and consistency round out the document nicely..cursor/rules/self_improve.mdc (9)
7-13: Rule Improvement Triggers section is comprehensive.
The list clearly outlines the conditions that should prompt rule improvements.
14-20: Analysis Process steps are well-defined.
The bullet points cover comparison with existing rules, documentation checks, and test monitoring.
21-27: Add New Rules When criteria are appropriate.
The thresholds and examples for adding new rules are logical and actionable.
28-33: Modify Existing Rules When guidelines are useful.
The conditions for updating existing rules reflect real-world maintenance needs.
34-46: Example Pattern Recognition is illustrative.
The Prisma query example effectively demonstrates how to identify repeatable patterns for new rules.
48-53: Rule Quality Checks are precise.
Actionable, specific, and code-based requirements ensure high-quality rules.
54-60: Continuous Improvement guidelines are thorough.
Monitoring, tracking, and linking processes are clearly described for ongoing rule refinement.
61-66: Rule Deprecation procedures are complete.
Marking, removing, and documenting migration paths for outdated rules covers the deprecation lifecycle.
67-73: Documentation Updates summary is solid.
Syncing examples, updating external references, and preserving links ensure documentation stays accurate..cursor/rules/dev_workflow.mdc (3)
1-5: Well-structured metadata section.The metadata is clear and properly defines this document's purpose and application scope. The
alwaysApply: truesetting ensures this guide is available throughout the project.
6-219: Comprehensive and well-structured development workflow guide.This document provides a thorough guide to using Task Master for managing task-driven development workflows. It clearly explains the interaction modes, workflow process, task management, and implementation techniques. The step-by-step instructions and cross-references to other documentation make it a valuable resource for users.
A few minor improvements noted in other comments would enhance the document's clarity and completeness, but overall, this is an excellent addition to the project documentation.
196-196:❌ Incorrect review comment
Update or remove reference to missing links.
The line references "internal guidelines (previously linked to
cursor_rules.mdcandself_improve.mdc)" which suggests links that were removed or changed. This could confuse readers expecting to find these links.
🏁 Script executed:
#!/bin/bash # Check if the referenced files exist echo "Checking for referenced files..." for file in ".cursor/rules/cursor_rules.mdc" ".cursor/rules/self_improve.mdc"; do if [ -f "$file" ]; then echo "File exists: $file" else echo "File NOT found: $file" fi doneLength of output: 516
Referenced files exist – review comment outdated
The files
.cursor/rules/cursor_rules.mdcand.cursor/rules/self_improve.mdcare present in the repo, so there’s no missing-link issue here. You can ignore this suggestion.Likely an incorrect or invalid review comment.
.cursor/rules/taskmaster.mdc (1)
6-383: Comprehensive and well-structured command reference.This document provides a thorough reference for all Taskmaster MCP tools and CLI commands. It clearly explains each command's purpose, parameters, options, and usage scenarios. The organization into logical sections and consistent formatting make it easy to navigate and understand.
A few minor improvements noted in other comments would enhance the document's clarity and completeness, but overall, this is an excellent addition to the project documentation that will greatly assist users in understanding and using Taskmaster effectively.
packages/components/src/ui/data-table-filter/index.tsx (1)
1-1: Expose DataTableFilter entry point
The re-export ofDataTableFilterfrom its component module aligns with the library’s export patterns, making imports more ergonomic for consumers.packages/components/src/ui/popover.tsx (2)
11-11: Add PopoverAnchor primitive to API
IntroducingPopoverAnchordirectly from the Radix primitive expands positioning flexibility without additional workarounds.
35-35: Update public exports to include PopoverAnchor
ExportingPopoverAnchoralongside the other Popover components completes the API surface for anchoring use cases.packages/components/src/ui/data-table-filter/lib/i18n.ts (1)
11-13: Simple fallback implementation is correct
Thetfunction correctly returns the translation or the key as a fallback. This meets expectations for now.packages/components/src/ui/select.tsx (1)
2-2: These changes are purely import re-ordering, blank-line insertion, and formatting (quote style andcn()usage adjustments). No functional logic has changed.Also applies to: 5-6, 32-32, 42-42, 64-64, 74-74
packages/components/package.json (1)
45-50: Dependency updates look good
The upgrades and new additions (@radix-ui/*,@tanstack/react-query,nuqs,zod, etc.) align with the newly introduced components and hooks. Ensure that all peer dependency ranges remain compatible with React 19.0.0.Also applies to: 53-55, 57-60, 68-68, 77-77
packages/components/src/ui/utils/use-unmount.tsx (1)
3-14: Well-implemented unmount hook with stale closure protectionThis is a solid implementation of a hook to run a function during component unmount. The use of
useRefto track the latest function reference ensures that even if the component re-renders with a new function instance, the unmount handler will always call the most recent version of the function, avoiding stale closure issues.packages/components/src/ui/data-table-filter/components/filter-subject.tsx (1)
3-17: Clean and type-safe filter subject componentThe component is well-structured with proper TypeScript generics to maintain type safety across the filter system. The UI implementation with conditional icon rendering and consistent styling helps maintain a cohesive filtering experience.
packages/components/src/ui/data-table/data-table-faceted-filter.tsx (3)
7-7: Improved import structure for Command componentsThe refactoring to import only the needed subcomponents rather than the entire Command component improves code organization.
100-137: Clean refactoring of Command structureThe structure has been refined to use the Command subcomponents directly without an outer Command wrapper, maintaining the same functionality with better component composition.
48-54: Minor whitespace adjustmentsThe whitespace changes around the
handleValueChangefunction enhance code readability.packages/components/src/ui/data-table-filter/components/filter-actions.tsx (2)
14-26: Good use of memoization for performance optimization.The component is properly memoized to prevent unnecessary re-renders, which is good practice for UI components that might re-render frequently.
22-23: Good responsive design pattern.The component intelligently hides the text label on smaller screens while keeping the icon visible, which is a good responsive design pattern for mobile interfaces.
packages/components/src/ui/data-table-filter/components/data-table-filter.tsx (2)
44-45: Resolve uncertainty about FilterActions componentThe comment suggests uncertainty about whether the FilterActions component is needed ("Filter Actions (Always visible, but maybe redundant now?)").
Please verify if this component is still necessary or if it can be removed. If it's needed, consider removing the comment to avoid confusion for future developers.
31-53: LGTM! Well-structured responsive layoutThe component has a clean responsive layout that adapts for both desktop and mobile views, using appropriate Tailwind CSS classes for conditional rendering.
packages/components/src/ui/data-table-filter/lib/memo.ts (1)
27-35: Efficient implementation of shallowEqualThe shallowEqual implementation is clean and efficient, handling identity checks before doing item-by-item comparison.
packages/components/src/ui/slider.tsx (1)
18-41: Well-structured Radix UI integration with appropriate stylingThe Slider component effectively wraps the Radix UI slider primitives with appropriate Tailwind CSS styling, handling different orientations and states.
apps/docs/src/lib/storybook/react-router-stub.tsx (1)
66-66: Good default search parameters for data tableThe default search string now includes pagination parameters (
page=0&pageSize=10), which is helpful for data table components.packages/components/src/ui/calendar.tsx (2)
54-56: LGTM: Nice icon customizationThe customization of the navigation icons with proper sizing and className handling is well-implemented.
43-43: 💡 Verification agent🧩 Analysis chain
Fix CSS class in PopoverContent
The
origin-(--radix-popover-content-transform-origin)class appears to use non-standard Tailwind syntax.
🏁 Script executed:
#!/bin/bash # Description: Check if this is a custom defined Tailwind class grep -r "origin-" --include="*.{js,ts,jsx,tsx,css}" .Length of output: 53
To verify whether the
origin-(--radix-popover-content-transform-origin)class is actually used (and to check for any custom safelist in your Tailwind setup), let’s search for both the non-standard and correct Tailwind arbitrary-value syntaxes across your codebase:
🏁 Script executed:
#!/bin/bash # Search for non-standard and arbitrary-value Tailwind origin classes rg -n "origin-\(" --glob "*.{js,ts,jsx,tsx,css}" rg -n "origin-\[" --glob "*.{js,ts,jsx,tsx,css}" # Look for any mention of the Radix popover transform-origin variable rg -n "radix-popover-content-transform-origin" --glob "*.{js,ts,jsx,tsx,css}" # If you have a Tailwind config, check for a safelist that might allow the paren syntax config=$(find . -maxdepth 2 -type f -name "tailwind.config.*" | head -n1) if [[ -n "$config" ]]; then echo "Inspecting $config for safelist entries…" rg -n "safelist" "$config" else echo "No Tailwind config found at top level." fiLength of output: 1366
Replace non-standard Tailwind syntax for PopoverContent transform-origin
The class
origin-(--radix-popover-content-transform-origin)isn’t valid Tailwind arbitrary-value syntax. Update it to use square brackets so Tailwind can parse the custom property.Affected locations:
- packages/components/src/ui/data-table-filter/components/filter-selector.tsx (≈ line 105)
- packages/components/src/ui/data-table-filter/components/filter-operator.tsx (≈ line 43)
- packages/components/src/ui/data-table-filter/components/filter-value.tsx (≈ line 64)
Suggested change in each file:
- className="w-fit p-0 origin-(--radix-popover-content-transform-origin)" + className="w-fit p-0 origin-[--radix-popover-content-transform-origin]"Likely an incorrect or invalid review comment.
packages/components/src/ui/data-table-filter/lib/helpers.ts (2)
4-12: Well implemented utility function for column retrievalThis utility function correctly finds a column by ID and throws a clear error message if not found.
56-63: Good type guard implementationThis type guard properly checks for the presence of required properties in a ColumnOption.
packages/components/src/ui/data-table-filter/lib/filter-fns.ts (5)
13-32: Clear and concise implementation for option filteringThe function handles empty cases correctly and properly implements case-insensitive comparison.
34-65: Effective implementation of multi-option filteringGood use of the intersection utility to handle various inclusion/exclusion operators.
85-114: Consistent date range handlingThe function correctly implements various date comparison operations using date-fns utilities.
One minor inconsistency: at line 110-113, you directly reference
filterValue.values[0]andfilterValue.values[1]instead of using thed1andd2variables defined earlier.
117-136: Effective text filtering implementationThe function correctly handles case-insensitivity and trims whitespace.
138-175: Well-structured number filtering logicThe function handles various comparison operators clearly and includes a default case.
packages/components/src/ui/dialog.tsx (2)
9-73: Well-structured dialog components with proper accessibilityThe dialog components are well-implemented with:
- Consistent data-slot attributes
- Animation states for transitions
- Proper accessibility with screen reader support
- Responsive styling
75-122: Good responsive layout componentsThe header and footer components provide appropriate responsive layouts that adapt to different screen sizes.
packages/components/src/ui/data-table-filter/core/operators.ts (3)
11-35: Well-organized default operatorsThe DEFAULT_OPERATORS object provides a clear mapping between column data types and their default operators for different target types.
37-227: Comprehensive operator definitions for option and date typesThe operator definitions are well-structured with complete metadata including localization keys, negation relationships, and target types.
229-359: Well-defined text and number operatorsThe text and number operator definitions follow the same pattern as the other operators, providing consistent metadata.
apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx (1)
316-337:useEffectcan thrash on every URL change – missing stable deps
location.searchis part of the dependency array. After eachnavigate, React-Router gives you a newlocationobject, which changeslocation.searchand re-fires the effect even when nothing else changed.
While you guard by comparingparams.toString()the extra render / effect cycle is still incurred.Consider basing the effect on controlled state only:
-useEffect(() => { +useEffect(() => { const params = new URLSearchParams(location.search); … if (params.toString() !== location.search.substring(1)) { navigate(`${location.pathname}?${params.toString()}`, { replace: true }); } -}, [pagination, sorting, navigate, location.search, location.pathname]); +// depend on the states you actually mutate +}, [pagination, sorting, navigate, location.pathname]);Or memoise
locationKey =${location.pathname}|${location.search}`` if you really need both.packages/components/src/ui/data-table-filter/components/filter-value.tsx (1)
416-424:useMemowith empty dependency list may lead to stale data
initialOptionsis computed only once. Ifcolumn,filter.values, or the faceted counts change (e.g. server-side refetch), the UI will not update. If that is intentional add eslint-disable comment, otherwise include the proper deps:- }, []); + }, [column, filter?.values]);(or whichever subset really needs to trigger a recompute).
🧰 Tools
🪛 Biome (1.9.4)
[error] 416-416: This hook does not specify all of its dependencies: column.getFacetedUniqueValues
This dependency is not specified in the hook dependency list.
Either include it or remove the dependency array
(lint/correctness/useExhaustiveDependencies)
[error] 416-416: This hook does not specify all of its dependencies: column.getOptions
This dependency is not specified in the hook dependency list.
Either include it or remove the dependency array
(lint/correctness/useExhaustiveDependencies)
[error] 416-416: This hook does not specify all of its dependencies: filter?.values.includes
This dependency is not specified in the hook dependency list.
This dependency is not specified in the hook dependency list.
Either include them or remove the dependency array
(lint/correctness/useExhaustiveDependencies)
| --- | ||
| description: Guidelines for continuously improving Cursor rules based on emerging code patterns and best practices. | ||
| globs: **/* | ||
| alwaysApply: true | ||
| --- |
There was a problem hiding this comment.
Incorrect globs pattern in front matter.
The globs field is set to **/*, which may apply this guideline to every file rather than just rule files. It should be scoped to .cursor/rules/*.mdc to target the intended documents.
🤖 Prompt for AI Agents (early access)
In .cursor/rules/self_improve.mdc at lines 1 to 5, the front matter's globs
pattern is set to **/*, which applies the guideline to all files. Change the
globs value to .cursor/rules/*.mdc to restrict the scope to only rule files in
that directory.
| - Refer to [`mcp.mdc`](mdc:.cursor/rules/mcp.mdc) for details on the MCP architecture and available tools. | ||
| - A comprehensive list and description of MCP tools and their corresponding CLI commands can be found in [`taskmaster.mdc`](mdc:.cursor/rules/taskmaster.mdc). |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify that the referenced documentation files exist.
The document links to mcp.mdc and taskmaster.mdc files which are critical for users to understand the system. Let's verify these files exist in the codebase.
🏁 Script executed:
#!/bin/bash
# Check if the referenced documentation files exist
echo "Checking for referenced documentation files..."
for file in ".cursor/rules/mcp.mdc" ".cursor/rules/taskmaster.mdc"; do
if [ -f "$file" ]; then
echo "File exists: $file"
else
echo "File NOT found: $file"
fi
doneLength of output: 495
Broken link to mcp.mdc – file not found
The link to .cursor/rules/mcp.mdc in dev_workflow.mdc is broken; the file doesn’t exist in the repository. Please correct this to ensure users can access the MCP documentation.
Locations to address:
- File:
.cursor/rules/dev_workflow.mdc - Lines: 18–19
- Refer to [`mcp.mdc`](mdc:.cursor/rules/mcp.mdc) for details on the MCP architecture… - A comprehensive list … in [`taskmaster.mdc`](mdc:.cursor/rules/taskmaster.mdc).
Actions:
- Add or restore
.cursor/rules/mcp.mdcwith the appropriate content, or - Update the link to point to the correct path (if the MCP doc resides elsewhere)
The taskmaster.mdc link is valid.
🤖 Prompt for AI Agents (early access)
In .cursor/rules/dev_workflow.mdc at lines 18-19, the link to mcp.mdc is broken
because the file does not exist in the repository. To fix this, either add or
restore the missing .cursor/rules/mcp.mdc file with the correct MCP
documentation content, or update the link in dev_workflow.mdc to point to the
correct existing location of the MCP documentation if it has been moved. Ensure
the taskmaster.mdc link remains unchanged as it is valid.
| - Break down complex tasks using `expand_task` / `task-master expand --id=<id> --force --research` (see [`taskmaster.mdc`](mdc:.cursor/rules/taskmaster.mdc)) with appropriate flags like `--force` (to replace existing subtasks) and `--research`. | ||
| - Clear existing subtasks if needed using `clear_subtasks` / `task-master clear-subtasks --id=<id>` (see [`taskmaster.mdc`](mdc:.cursor/rules/taskmaster.mdc)) before regenerating | ||
| - Implement code following task details, dependencies, and project standards | ||
| - Verify tasks according to test strategies before marking as complete (See [`tests.mdc`](mdc:.cursor/rules/tests.mdc)) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify that the referenced tests.mdc file exists.
The document references a tests.mdc file which should contain test strategies information, but it's not clear if this file exists in the codebase.
🏁 Script executed:
#!/bin/bash
# Check if the referenced tests.mdc file exists
if [ -f ".cursor/rules/tests.mdc" ]; then
echo "File exists: .cursor/rules/tests.mdc"
else
echo "File NOT found: .cursor/rules/tests.mdc"
fiLength of output: 125
Broken reference to tests.mdc
The document .cursor/rules/dev_workflow.mdc references tests.mdc on line 42, but .cursor/rules/tests.mdc does not exist in the repository.
- Either add the missing
tests.mdcfile at.cursor/rules/tests.mdcwith the expected test strategies, - Or update the link to point to the correct location of the test strategy document.
🤖 Prompt for AI Agents (early access)
In .cursor/rules/dev_workflow.mdc at line 42, the document references a
tests.mdc file that does not exist at .cursor/rules/tests.mdc. To fix this,
either create and add the missing tests.mdc file with the appropriate test
strategies at that path, or update the link in dev_workflow.mdc to point to the
correct existing location of the test strategy document.
| * Stage the relevant code changes and any updated/new rule files (`git add .`). | ||
| * Craft a comprehensive Git commit message summarizing the work done for the subtask, including both code implementation and any rule adjustments. | ||
| * Execute the commit command directly in the terminal (e.g., `git commit -m 'feat(module): Implement feature X for subtask <subtaskId>\n\n- Details about changes...\n- Updated rule Y for pattern Z'`). | ||
| * Consider if a Changeset is needed according to internal versioning guidelines (previously linked to `changeset.mdc`). If so, run `npm run changeset`, stage the generated file, and amend the commit or create a new one. |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Update or remove reference to missing link.
The line references "internal versioning guidelines (previously linked to changeset.mdc)" which suggests a link that was removed or changed. This could confuse readers expecting to find this link.
🏁 Script executed:
#!/bin/bash
# Check if the referenced changeset.mdc file exists
if [ -f ".cursor/rules/changeset.mdc" ]; then
echo "File exists: .cursor/rules/changeset.mdc"
else
echo "File NOT found: .cursor/rules/changeset.mdc"
fiLength of output: 137
Update or remove reference to missing changeset.mdc link
The file changeset.mdc no longer exists, so the “previously linked to changeset.mdc” text will confuse readers. Please either remove that parenthetical note or replace it with the correct link to your current internal versioning guidelines.
Locations to update:
- File:
.cursor/rules/dev_workflow.mdc - Line: 205
Suggested change:
- * Consider if a Changeset is needed according to internal versioning guidelines (previously linked to `changeset.mdc`). If so, run `npm run changeset`, stage the generated file, and amend the commit or create a new one.
+ * Consider if a Changeset is needed according to internal versioning guidelines. If so, run `npm run changeset`, stage the generated file, and amend the commit or create a new one.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * Consider if a Changeset is needed according to internal versioning guidelines (previously linked to `changeset.mdc`). If so, run `npm run changeset`, stage the generated file, and amend the commit or create a new one. | |
| * Consider if a Changeset is needed according to internal versioning guidelines. If so, run `npm run changeset`, stage the generated file, and amend the commit or create a new one. |
🤖 Prompt for AI Agents (early access)
In .cursor/rules/dev_workflow.mdc at line 205, the text references a missing
link to "changeset.mdc" which no longer exists. Remove the parenthetical note
"(previously linked to `changeset.mdc`)" or replace it with the correct current
link to the internal versioning guidelines to avoid confusing readers.
| ## Environment Variables Configuration (Updated) | ||
|
|
||
| Taskmaster primarily uses the **`.taskmasterconfig`** file (in project root) for configuration (models, parameters, logging level, etc.), managed via `task-master models --setup`. | ||
|
|
||
| Environment variables are used **only** for sensitive API keys related to AI providers and specific overrides like the Ollama base URL: | ||
|
|
||
| * **API Keys (Required for corresponding provider):** | ||
| * `ANTHROPIC_API_KEY` | ||
| * `PERPLEXITY_API_KEY` | ||
| * `OPENAI_API_KEY` | ||
| * `GOOGLE_API_KEY` | ||
| * `MISTRAL_API_KEY` | ||
| * `AZURE_OPENAI_API_KEY` (Requires `AZURE_OPENAI_ENDPOINT` too) | ||
| * `OPENROUTER_API_KEY` | ||
| * `XAI_API_KEY` | ||
| * `OLLANA_API_KEY` (Requires `OLLAMA_BASE_URL` too) | ||
| * **Endpoints (Optional/Provider Specific inside .taskmasterconfig):** | ||
| * `AZURE_OPENAI_ENDPOINT` | ||
| * `OLLAMA_BASE_URL` (Default: `http://localhost:11434/api`) | ||
|
|
||
| **Set API keys** in your **`.env`** file in the project root (for CLI use) or within the `env` section of your **`.cursor/mcp.json`** file (for MCP/Cursor integration). All other settings (model choice, max tokens, temperature, log level, custom endpoints) are managed in `.taskmasterconfig` via `task-master models` command or `models` MCP tool. | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Cross-reference with .env.example file mentioned in PR objectives.
The PR objectives mention creating a .env.example file that outlines required API keys, but this document references assets/env.example on line 117. We should verify if these are the same file or if one should reference the other.
🏁 Script executed:
#!/bin/bash
# Check for env.example files in the repository
echo "Checking for env example files..."
if [ -f ".env.example" ]; then
echo "File exists: .env.example"
echo "Contents:"
cat ".env.example" | grep -v "KEY" | head -n 5 # Show first few lines, avoiding actual keys
fi
if [ -f "assets/env.example" ]; then
echo "File exists: assets/env.example"
echo "Contents:"
cat "assets/env.example" | grep -v "KEY" | head -n 5 # Show first few lines, avoiding actual keys
fiLength of output: 319
Update documentation to reference the correct .env.example file
The repository only contains a top-level .env.example; there is no assets/env.example. Please update the reference in your docs:
• File: .cursor/rules/taskmaster.mdc
- At the reference on or around line 117, change:
- Refer to
assets/env.example
+ Refer to.env.example
```
This ensures the docs and the example file location stay in sync.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents (early access)
In .cursor/rules/taskmaster.mdc around line 117, update the documentation to
reference the correct .env.example file located at the project root instead of
assets/env.example, as the repository only contains the top-level .env.example
file. This change will keep the documentation consistent with the actual file
structure.
apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx
Outdated
Show resolved
Hide resolved
| const newValues = | ||
| column.type === 'number' | ||
| ? createNumberFilterValue(values as number[]) | ||
| : column.type === 'date' | ||
| ? createDateFilterValue(values as [Date, Date] | [Date] | [] | undefined) | ||
| : uniq(values); | ||
| if (newValues.length === 0) return prev; | ||
| if (!isColumnFiltered) { | ||
| return [ |
There was a problem hiding this comment.
Unable to clear number / date filters
setFilterValue bails out when newValues.length === 0, returning the previous state and leaving the stale filter in place.
For number / date sliders a user often wants to “clear” the range; currently that’s impossible without using removeFilter() manually.
- if (newValues.length === 0) return prev;
+ if (newValues.length === 0) {
+ // remove the filter completely
+ return prev.filter((f) => f.columnId !== column.id);
+ }This brings behaviour in line with option/multi-option helpers.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const newValues = | |
| column.type === 'number' | |
| ? createNumberFilterValue(values as number[]) | |
| : column.type === 'date' | |
| ? createDateFilterValue(values as [Date, Date] | [Date] | [] | undefined) | |
| : uniq(values); | |
| if (newValues.length === 0) return prev; | |
| if (!isColumnFiltered) { | |
| return [ | |
| const newValues = | |
| column.type === 'number' | |
| ? createNumberFilterValue(values as number[]) | |
| : column.type === 'date' | |
| ? createDateFilterValue(values as [Date, Date] | [Date] | [] | undefined) | |
| : uniq(values); | |
| if (newValues.length === 0) { | |
| // remove the filter completely | |
| return prev.filter((f) => f.columnId !== column.id); | |
| } | |
| if (!isColumnFiltered) { | |
| return [ |
🤖 Prompt for AI Agents (early access)
In packages/components/src/ui/utils/use-data-table-filters.tsx around lines 248
to 256, the code prevents clearing number and date filters because it returns
the previous state when newValues is empty, leaving stale filters. Modify the
logic to allow clearing these filters by not bailing out when newValues is empty
for number and date types, ensuring the filter state updates accordingly to
reflect the cleared filter.
| import { isEqual } from 'date-fns'; | ||
| import { format } from 'date-fns'; | ||
| import { Calendar, Ellipsis } from 'lucide-react'; | ||
| import { type ElementType, cloneElement, isValidElement, memo, useCallback, useEffect, useMemo, useState } from 'react'; |
There was a problem hiding this comment.
Incorrect Calendar import causes TypeScript error & broken UI
A Calendar SVG icon from lucide-react is imported, but the JSX below expects the date-picker component (from your internal UI or react-day-picker). This mismatch generates the pipeline failure (TS2322) and will blow up at runtime.
-import { Calendar, Ellipsis } from 'lucide-react';
+// Keep the icon if you still need it elsewhere
+import { Calendar as CalendarIcon, Ellipsis } from 'lucide-react';
+// Import the real date-picker component
+import { Calendar } from '../../calendar'; // adjust path if necessaryIf you do not need the icon, simply remove it:
-import { Calendar, Ellipsis } from 'lucide-react';
+import { Calendar } from '../../calendar';
+import { Ellipsis } from 'lucide-react';No other code changes are needed because the <Calendar … /> usage already matches the correct props of the date-picker component.
Also applies to: 567-575
🤖 Prompt for AI Agents (early access)
In packages/components/src/ui/data-table-filter/components/filter-value.tsx at
lines 1 to 4 and also lines 567 to 575, the Calendar import from 'lucide-react'
is incorrect because the JSX expects a date-picker component, not an icon. To
fix this, remove the Calendar import entirely if it is not needed, ensuring the
existing <Calendar /> usage corresponds to the correct date-picker component.
This will resolve the TypeScript error TS2322 and prevent runtime issues.
| useEffect(() => { | ||
| if (filter?.values && filter.values.length === values.length && filter.values.every((v, i) => v === values[i])) { | ||
| setValues(filter.values); | ||
| } | ||
| }, [filter?.values, values]); |
There was a problem hiding this comment.
Array-equality check is inverted – local state never updates
setValues is called when the arrays are already equal, and skipped when they differ.
-if (filter?.values && filter.values.length === values.length && filter.values.every((v, i) => v === values[i])) {
- setValues(filter.values);
-}
+if (
+ filter?.values &&
+ (filter.values.length !== values.length ||
+ !filter.values.every((v, i) => v === values[i]))
+) {
+ setValues(filter.values);
+}This keeps the slider/text inputs in sync when another component modifies the number filter.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (filter?.values && filter.values.length === values.length && filter.values.every((v, i) => v === values[i])) { | |
| setValues(filter.values); | |
| } | |
| }, [filter?.values, values]); | |
| useEffect(() => { | |
| - if (filter?.values && filter.values.length === values.length && filter.values.every((v, i) => v === values[i])) { | |
| - setValues(filter.values); | |
| - } | |
| + if ( | |
| + filter?.values && | |
| + (filter.values.length !== values.length || | |
| + !filter.values.every((v, i) => v === values[i])) | |
| + ) { | |
| + setValues(filter.values); | |
| + } | |
| }, [filter?.values, values]); |
🤖 Prompt for AI Agents (early access)
In packages/components/src/ui/data-table-filter/components/filter-value.tsx
around lines 623 to 627, the useEffect hook incorrectly calls setValues only
when filter.values and values arrays are already equal, preventing local state
updates when they differ. To fix this, invert the condition to call
setValues(filter.values) only when the arrays are not equal, ensuring the local
state updates and keeps the slider/text inputs in sync with external changes.
There was a problem hiding this comment.
Actionable comments posted: 14
♻️ Duplicate comments (2)
packages/components/src/ui/utils/use-issues-query.ts (2)
5-8: 🛠️ Refactor suggestionDefine specific types instead of using
any.The
IssuesApiResponseinterface currently uses a generic type parameterTwith a default ofany, which reduces type safety. This was flagged by the static analysis tool as a potential issue. Consider defining a more specific type instead.+// Define a proper type for issue data +interface Issue { + id: string; + title: string; + status: string; + assignee: string; + // Add other issue properties as needed +} + +// Define a type for faceted counts +interface FacetedCounts { + status?: Record<string, number>; + assignee?: Record<string, number>; + // Add other facet categories as needed +} // Define the expected shape of the API response -interface IssuesApiResponse<T = any> { +interface IssuesApiResponse<T = Issue> { data: T[]; - facetedCounts: Record<string, Record<string, number>>; + facetedCounts: FacetedCounts; }🧰 Tools
🪛 Biome (1.9.4)
[error] 5-5: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
10-26: 🛠️ Refactor suggestionImprove error handling and configurability of the fetchData function.
The current implementation has basic error handling but could be improved to provide more context about errors. Additionally, it would be beneficial to make the function more configurable for different environments.
// Generic function to fetch data with filters async function fetchData<T = any>( endpoint: string, - filters: FiltersState + filters: FiltersState, + options?: { headers?: HeadersInit } ): Promise<IssuesApiResponse<T>> { // Encode filters for URL const filterParam = filters.length > 0 ? `filters=${encodeURIComponent(JSON.stringify(filters))}` : ''; - const response = await fetch(`${endpoint}?${filterParam}`); + const response = await fetch(`${endpoint}?${filterParam}`, { + headers: options?.headers, + }); if (!response.ok) { - const errorData = await response.json().catch(() => ({ message: 'Failed to parse error response' })); - throw new Error(errorData.error || `HTTP error! status: ${response.status}`); + try { + const errorData = await response.json(); + throw new Error( + `API Error: ${errorData.error || errorData.message || `HTTP status ${response.status}`}` + ); + } catch (e) { + // If parsing JSON fails, throw a generic error with status + if (e instanceof SyntaxError) { + throw new Error(`HTTP error! status: ${response.status}, could not parse error response`); + } + throw e; + } } - const data: IssuesApiResponse<T> = await response.json(); - return data; + return await response.json() as IssuesApiResponse<T>; }🧰 Tools
🪛 Biome (1.9.4)
[error] 11-11: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
🧹 Nitpick comments (2)
packages/components/src/ui/data-table-filter/README.md (1)
246-252: Consider adding links to example files.The references to example files are helpful, but adding direct links to these files in the repository would make it easier for developers to find them.
- See the `examples` directory for complete working examples: + See the [examples directory](../examples/) for complete working examples: - - `data-table-filter-example.tsx`: Comprehensive example with API integration - - `simplified-example.tsx`: Simplified example using the `useFilteredData` hook + - [`data-table-filter-example.tsx`](../examples/data-table-filter-example.tsx): Comprehensive example with API integration + - [`simplified-example.tsx`](../examples/simplified-example.tsx): Simplified example using the `useFilteredData` hook🧰 Tools
🪛 LanguageTool
[uncategorized] ~250-~250: Loose punctuation mark.
Context: ...ples: -data-table-filter-example.tsx: Comprehensive example with API integrat...(UNLIKELY_OPENING_PUNCTUATION)
packages/components/src/ui/data-table-filter/examples/data-table-filter-example.tsx (1)
177-223: Consider extracting repeated UI patterns to components.The table and debug panel UI patterns are duplicated between this example and the simplified example. Consider extracting these into reusable components.
Create components like
UserTableandFilterDebugPanelthat can be reused across examples to reduce duplication and improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/components/src/ui/data-table-filter/README.md(1 hunks)packages/components/src/ui/data-table-filter/examples/data-table-filter-example.tsx(1 hunks)packages/components/src/ui/data-table-filter/examples/simplified-example.tsx(1 hunks)packages/components/src/ui/utils/debounce.ts(1 hunks)packages/components/src/ui/utils/use-filter-sync.ts(1 hunks)packages/components/src/ui/utils/use-filtered-data.ts(1 hunks)packages/components/src/ui/utils/use-issues-query.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/src/ui/utils/use-filter-sync.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/components/src/ui/utils/use-issues-query.ts (2)
packages/components/src/ui/data-table-filter/core/types.ts (1)
FiltersState(294-294)packages/components/src/ui/utils/filters.ts (1)
FiltersState(31-31)
🪛 GitHub Actions: Run Tests
packages/components/src/ui/data-table-filter/examples/data-table-filter-example.tsx
[error] 62-62: TypeScript TS7023: 'icon' implicitly has return type 'any' because it lacks a return type annotation.
[error] 74-74: TypeScript TS7023: 'icon' implicitly has return type 'any' because it lacks a return type annotation.
[error] 100-100: TypeScript TS2345: Argument type incompatible with expected 'readonly ColumnConfig<User, any, any, any>[]'. Property 'icon' type incompatible; missing '$$typeof' property.
[error] 106-106: TypeScript TS7006: Parameter 'column' implicitly has 'any' type.
[error] 106-106: TypeScript TS7006: Parameter 'values' implicitly has 'any' type.
[error] 124-124: TypeScript TS7006: Parameter 'column' implicitly has 'any' type.
[error] 124-124: TypeScript TS7006: Parameter 'values' implicitly has 'any' type.
[error] 141-141: TypeScript TS7006: Parameter 'column' implicitly has 'any' type.
[error] 141-141: TypeScript TS7006: Parameter 'values' implicitly has 'any' type.
[error] 159-159: TypeScript TS7006: Parameter 'columnId' implicitly has 'any' type.
[error] 159-159: TypeScript TS7006: Parameter 'operator' implicitly has 'any' type.
[error] 168-168: TypeScript TS7006: Parameter 'columnId' implicitly has 'any' type.
packages/components/src/ui/utils/use-filtered-data.ts
[error] 53-63: TypeScript TS2339: Property 'facetedCounts' does not exist on type '{}'. Multiple occurrences at lines 53, 57, 61, and 63.
[error] 75-75: TypeScript TS2339 and TS2345: Property 'data' does not exist on type '{}'; argument type incompatible with expected 'readonly ColumnConfig<TData, any, any, any>[]'.
[error] 82-140: TypeScript TS2345: Argument type incompatible with expected filter array type; includes incompatible 'type' and 'operator' property values.
[error] 104-104: TypeScript TS2345: Argument of type 'string | number | boolean | null' is not assignable to parameter of type 'ElementType<ColumnDataNativeMap[TType]>'. 'null' is not assignable.
[error] 117-140: TypeScript TS2345: Argument type incompatible with expected filter array type; includes incompatible 'type' and 'operator' property values and 'Date' type not assignable to expected types.
[error] 157-158: TypeScript TS2339: Properties 'data' and 'facetedCounts' do not exist on type '{}'.
packages/components/src/ui/data-table-filter/examples/simplified-example.tsx
[error] 32-32: TypeScript TS7023: 'icon' implicitly has return type 'any' because it lacks a return type annotation.
[error] 44-44: TypeScript TS7023: 'icon' implicitly has return type 'any' because it lacks a return type annotation.
[error] 62-62: TypeScript TS2322: Argument type incompatible with expected 'readonly ColumnConfig<User, any, any, any>[]'. Property 'icon' type incompatible; missing '$$typeof' property.
[error] 98-98: TypeScript TS7006: Parameter 'user' implicitly has 'any' type.
packages/components/src/ui/utils/use-issues-query.ts
[error] 52-52: TypeScript TS2769: No overload matches call; object literal specifies unknown property 'onSuccess' which does not exist in expected type.
🪛 Biome (1.9.4)
packages/components/src/ui/utils/use-filtered-data.ts
[error] 9-9: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 9-9: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 9-9: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
packages/components/src/ui/utils/debounce.ts
[error] 10-10: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 10-10: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 24-24: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 119-119: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 25-25: This let declares a variable that is only assigned once.
'maxWait' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 30-30: This let declares a variable that is only assigned once.
'leading' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 31-31: This let declares a variable that is only assigned once.
'trailing' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
packages/components/src/ui/utils/use-issues-query.ts
[error] 5-5: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 11-11: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 36-36: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
🪛 LanguageTool
packages/components/src/ui/data-table-filter/README.md
[uncategorized] ~172-~172: Loose punctuation mark.
Context: ...ale="en" /> ``` #### Props - columns: Array of column definitions with filter...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~203-~203: Loose punctuation mark.
Context: ..., }); ``` #### Parameters - endpoint: API endpoint to fetch data from - `colu...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~211-~211: Loose punctuation mark.
Context: ...for the query #### Returns - filters: Current filter state - setFilters: Fu...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~250-~250: Loose punctuation mark.
Context: ...ples: - data-table-filter-example.tsx: Comprehensive example with API integrat...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (11)
packages/components/src/ui/utils/debounce.ts (3)
10-22: Replaceanywith safer generics & extract a dedicatedDebouncedFunctiontype
anysneaks past the compiler and the static-analysis warnings confirm this.
By introducing an explicit alias that models “a callable object withcancel/flushmethods” and switching the generic tounknown[] | unknown, we preserve type-safety without hurting ergonomics.+/** + * A function returned by `debounce` – invokable like the original callback + * but extended with `cancel` and `flush` helpers. + */ +export type DebouncedFunction<T extends (...args: unknown[]) => unknown> = { + (...args: Parameters<T>): ReturnType<T> | undefined + cancel: () => void + flush: () => ReturnType<T> | undefined +} + -export function debounce<T extends (...args: any[]) => any>( +export function debounce<T extends (...args: unknown[]) => unknown>( func: T, wait = 300, options: { leading?: boolean; trailing?: boolean; maxWait?: number; } = {} -): { - (...args: Parameters<T>): ReturnType<T> | undefined; - cancel: () => void; - flush: () => ReturnType<T> | undefined; -} { +): DebouncedFunction<T> {This eliminates the
anyusages flagged by Biome, makes consumer code easier to reason about, and centralises the “shape” of a debounced function in one place.
[suggest_essential_refactor]🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 10-10: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
24-32: Useconstfor variables that are never reassigned
lastArgset al. must stay mutable, butmaxWait,leading, andtrailingare never changed after initialisation. Marking them asconstsilences linter complaints and clearly documents intent.- let maxWait: number | undefined = options.maxWait; + const maxWait: number | undefined = options.maxWait; … - let leading = !!options.leading; - let trailing = 'trailing' in options ? !!options.trailing : true; + const leading = !!options.leading; + const trailing = 'trailing' in options ? !!options.trailing : true;[suggest_nitpick]
🧰 Tools
🪛 Biome (1.9.4)
[error] 24-24: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 25-25: This let declares a variable that is only assigned once.
'maxWait' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 30-30: This let declares a variable that is only assigned once.
'leading' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 31-31: This let declares a variable that is only assigned once.
'trailing' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
43-46: Avoid parameter shadowing for readability
startTimer’s second parameter shares the namewaitwith the outer scope, which can trip up readers during debugging. Renaming it todelaykeeps meanings distinct and clarifies intent.-function startTimer(pendingFunc: () => void, wait: number) { - return setTimeout(pendingFunc, wait); +function startTimer(pendingFunc: () => void, delay: number) { + return setTimeout(pendingFunc, delay); }[suggest_nitpick]
packages/components/src/ui/data-table-filter/README.md (5)
1-13: Documentation looks comprehensive and well-structured.The introduction and features section provide a clear overview of the component's capabilities. The component seems to offer a robust filtering system with multiple filter types, URL synchronization, and real-time updates.
15-74: Basic usage example is well-documented.The example with the
useFilteredDatahook demonstrates a clean implementation pattern that abstracts away much of the complexity. The code sample is clear and includes helpful comments.
76-154: Advanced usage example provides good flexibility.The manual setup example shows how to handle more complex scenarios with direct control over filter state, API integration, and faceted counts. This gives developers flexibility when the simplified approach isn't sufficient.
156-221: API reference is thorough and helpful.The documentation for both the component props and hook parameters/return values is detailed and will be valuable to developers implementing this component.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~172-~172: Loose punctuation mark.
Context: ...ale="en" /> ``` #### Props -columns: Array of column definitions with filter...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~203-~203: Loose punctuation mark.
Context: ..., }); ``` #### Parameters -endpoint: API endpoint to fetch data from - `colu...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~211-~211: Loose punctuation mark.
Context: ...for the query #### Returns -filters: Current filter state -setFilters: Fu...(UNLIKELY_OPENING_PUNCTUATION)
222-245: Server-side implementation guidance is valuable.The documentation includes the expected API response format for faceted counts, which is crucial for proper integration. This will help backend developers implement compatible endpoints.
packages/components/src/ui/data-table-filter/examples/simplified-example.tsx (2)
8-22: Clear data model and mock data.The User interface and mock data are well structured and provide a good foundation for the example.
71-117: Well-structured UI with appropriate loading state.The component renders the DataTableFilter with proper props and displays a loading indicator or the filtered data table based on the loading state. The debug panel is a nice touch for development and debugging purposes.
🧰 Tools
🪛 GitHub Actions: Run Tests
[error] 98-98: TypeScript TS7006: Parameter 'user' implicitly has 'any' type.
packages/components/src/ui/data-table-filter/examples/data-table-filter-example.tsx (1)
11-42: Well-structured data model and mock API.The User interface is clearly defined, and the mock API function provides a realistic simulation with faceted counts.
| // Use our custom hook to handle everything | ||
| const { | ||
| filters, | ||
| columns, | ||
| actions, | ||
| data, | ||
| isLoading, | ||
| } = useFilteredData<User>({ | ||
| endpoint: '/api/users', | ||
| columnsConfig, | ||
| initialData: initialUsers, | ||
| queryOptions: { | ||
| // In a real app, you'd set this to true | ||
| // For demo purposes, we'll disable actual API calls | ||
| enabled: false, | ||
| } | ||
| }); |
There was a problem hiding this comment.
Implement proper typing for the useFilteredData hook arguments.
The TypeScript error in the pipeline indicates incompatible types for the columnsConfig parameter.
The issue is likely related to the icon property return type. After fixing the icon return types as suggested above, this error should be resolved.
🧰 Tools
🪛 GitHub Actions: Run Tests
[error] 62-62: TypeScript TS2322: Argument type incompatible with expected 'readonly ColumnConfig<User, any, any, any>[]'. Property 'icon' type incompatible; missing '$$typeof' property.
🤖 Prompt for AI Agents (early access)
In packages/components/src/ui/data-table-filter/examples/simplified-example.tsx
around lines 53 to 69, the useFilteredData hook is called with columnsConfig
that has incompatible types, likely due to incorrect icon property return types.
To fix this, ensure that the icon properties in columnsConfig return the correct
React element types expected by the hook's type definitions. Adjust the icon
return types to match the expected type, which should resolve the TypeScript
error.
| </tr> | ||
| </thead> | ||
| <tbody> | ||
| {data.map(user => ( |
There was a problem hiding this comment.
Add explicit type for the map callback parameter.
The TypeScript compiler is reporting that the user parameter implicitly has an any type.
- {data.map(user => (
+ {data.map((user: User) => (🧰 Tools
🪛 GitHub Actions: Run Tests
[error] 98-98: TypeScript TS7006: Parameter 'user' implicitly has 'any' type.
🤖 Prompt for AI Agents (early access)
In packages/components/src/ui/data-table-filter/examples/simplified-example.tsx
at line 98, the map callback parameter 'user' lacks an explicit type, causing a
TypeScript implicit any error. Fix this by explicitly typing the 'user'
parameter with the appropriate user data type in the map function to satisfy the
compiler.
| const columnsConfig = useMemo(() => [ | ||
| { | ||
| id: 'role', | ||
| type: 'option' as const, | ||
| displayName: 'Role', | ||
| accessor: (user: User) => user.role, | ||
| icon: () => null, | ||
| options: [ | ||
| { value: 'admin', label: 'Admin' }, | ||
| { value: 'user', label: 'User' }, | ||
| { value: 'editor', label: 'Editor' }, | ||
| ], | ||
| }, | ||
| { | ||
| id: 'status', | ||
| type: 'option' as const, | ||
| displayName: 'Status', | ||
| accessor: (user: User) => user.status, | ||
| icon: () => null, | ||
| options: [ | ||
| { value: 'active', label: 'Active' }, | ||
| { value: 'inactive', label: 'Inactive' }, | ||
| { value: 'pending', label: 'Pending' }, | ||
| ], | ||
| }, | ||
| ], []); |
There was a problem hiding this comment.
Fix TypeScript errors in column configuration.
The TypeScript compiler is reporting that the icon functions lack return type annotations.
- icon: () => null,
+ icon: (): React.ReactNode => null,- icon: () => null,
+ icon: (): React.ReactNode => null,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const columnsConfig = useMemo(() => [ | |
| { | |
| id: 'role', | |
| type: 'option' as const, | |
| displayName: 'Role', | |
| accessor: (user: User) => user.role, | |
| icon: () => null, | |
| options: [ | |
| { value: 'admin', label: 'Admin' }, | |
| { value: 'user', label: 'User' }, | |
| { value: 'editor', label: 'Editor' }, | |
| ], | |
| }, | |
| { | |
| id: 'status', | |
| type: 'option' as const, | |
| displayName: 'Status', | |
| accessor: (user: User) => user.status, | |
| icon: () => null, | |
| options: [ | |
| { value: 'active', label: 'Active' }, | |
| { value: 'inactive', label: 'Inactive' }, | |
| { value: 'pending', label: 'Pending' }, | |
| ], | |
| }, | |
| ], []); | |
| const columnsConfig = useMemo(() => [ | |
| { | |
| id: 'role', | |
| type: 'option' as const, | |
| displayName: 'Role', | |
| accessor: (user: User) => user.role, | |
| icon: (): React.ReactNode => null, | |
| options: [ | |
| { value: 'admin', label: 'Admin' }, | |
| { value: 'user', label: 'User' }, | |
| { value: 'editor', label: 'Editor' }, | |
| ], | |
| }, | |
| { | |
| id: 'status', | |
| type: 'option' as const, | |
| displayName: 'Status', | |
| accessor: (user: User) => user.status, | |
| icon: (): React.ReactNode => null, | |
| options: [ | |
| { value: 'active', label: 'Active' }, | |
| { value: 'inactive', label: 'Inactive' }, | |
| { value: 'pending', label: 'Pending' }, | |
| ], | |
| }, | |
| ], []); |
🧰 Tools
🪛 GitHub Actions: Run Tests
[error] 32-32: TypeScript TS7023: 'icon' implicitly has return type 'any' because it lacks a return type annotation.
[error] 44-44: TypeScript TS7023: 'icon' implicitly has return type 'any' because it lacks a return type annotation.
🤖 Prompt for AI Agents (early access)
In packages/components/src/ui/data-table-filter/examples/simplified-example.tsx
around lines 26 to 51, the icon functions in the columnsConfig array lack
explicit return type annotations, causing TypeScript errors. Add explicit return
type annotations to these icon functions, specifying the correct return type
(likely React.ReactNode or null) to satisfy the TypeScript compiler.
| interface UseFilteredDataOptions<TData> { | ||
| endpoint: string; | ||
| columnsConfig: ReadonlyArray<ColumnConfig<TData, any, any, any>>; | ||
| strategy?: FilterStrategy; | ||
| initialData?: TData[]; | ||
| queryOptions?: { | ||
| enabled?: boolean; | ||
| refetchInterval?: number | false; | ||
| onSuccess?: (data: { data: TData[]; facetedCounts: Record<string, Record<string, number>> }) => void; | ||
| onError?: (error: Error) => void; | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace any types with more specific types.
The static analysis tool flags the use of any in the columnsConfig type. This could lead to type safety issues.
- columnsConfig: ReadonlyArray<ColumnConfig<TData, any, any, any>>;
+ columnsConfig: ReadonlyArray<ColumnConfig<TData, unknown, unknown, unknown>>;Alternatively, consider creating a more specific type that captures the possible column configurations for your use cases.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| interface UseFilteredDataOptions<TData> { | |
| endpoint: string; | |
| columnsConfig: ReadonlyArray<ColumnConfig<TData, any, any, any>>; | |
| strategy?: FilterStrategy; | |
| initialData?: TData[]; | |
| queryOptions?: { | |
| enabled?: boolean; | |
| refetchInterval?: number | false; | |
| onSuccess?: (data: { data: TData[]; facetedCounts: Record<string, Record<string, number>> }) => void; | |
| onError?: (error: Error) => void; | |
| }; | |
| } | |
| interface UseFilteredDataOptions<TData> { | |
| endpoint: string; | |
| - columnsConfig: ReadonlyArray<ColumnConfig<TData, any, any, any>>; | |
| + columnsConfig: ReadonlyArray<ColumnConfig<TData, unknown, unknown, unknown>>; | |
| strategy?: FilterStrategy; | |
| initialData?: TData[]; | |
| queryOptions?: { | |
| enabled?: boolean; | |
| refetchInterval?: number | false; | |
| onSuccess?: (data: { data: TData[]; facetedCounts: Record<string, Record<string, number>> }) => void; | |
| onError?: (error: Error) => void; | |
| }; | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 9-9: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 9-9: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 9-9: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
🤖 Prompt for AI Agents (early access)
In packages/components/src/ui/utils/use-filtered-data.ts between lines 7 and 18,
the UseFilteredDataOptions interface uses 'any' types in the columnsConfig
property, which reduces type safety. Replace these 'any' types with more
specific, appropriate generic types or create a dedicated type that accurately
represents the possible column configurations to ensure stronger type checking
and prevent potential runtime errors.
| const { data, isLoading, isError, error, refetch } = useDataQuery<TData>( | ||
| endpoint, | ||
| filters, | ||
| queryOptions | ||
| ); |
There was a problem hiding this comment.
Add proper type handling for the data query response.
The TypeScript errors indicate that the data returned from useDataQuery might not have the expected structure with facetedCounts and data properties.
Update the type definition or add type guards to ensure safe property access:
- const { data, isLoading, isError, error, refetch } = useDataQuery<TData>(
+ interface DataQueryResponse<T> {
+ data: T[];
+ facetedCounts?: Record<string, Record<string, number>>;
+ }
+
+ const { data, isLoading, isError, error, refetch } = useDataQuery<TData, DataQueryResponse<TData>>(Then update the code to safely access these properties where needed.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents (early access)
In packages/components/src/ui/utils/use-filtered-data.ts around lines 41 to 45,
the data returned from useDataQuery<TData> may not have the expected structure
with facetedCounts and data properties, causing TypeScript errors. To fix this,
update the type definition of TData to accurately reflect the expected shape
including facetedCounts and data, or add type guards before accessing these
properties to ensure they exist and are of the correct type. This will prevent
unsafe property access and resolve the TypeScript errors.
| const columns = useMemo(() => { | ||
| if (!data) return [] as Column<User>[]; | ||
|
|
||
| // Apply faceted counts from the API to the columns | ||
| const enhancedConfig = columnsConfig.map(config => { | ||
| if (config.type === 'option' && data.facetedCounts[config.id]) { | ||
| return { | ||
| ...config, | ||
| facetedOptions: new Map( | ||
| Object.entries(data.facetedCounts[config.id]).map(([key, count]) => [key, count]) | ||
| ) | ||
| }; | ||
| } | ||
| return config; | ||
| }); | ||
|
|
||
| return createColumns(data.data || [], enhancedConfig, 'client'); | ||
| }, [data, columnsConfig]); |
There was a problem hiding this comment.
Add proper typing for createColumns arguments.
The TypeScript error indicates incompatible types for the createColumns function call.
This is likely related to the icon property return types. After fixing those, ensure the enhanced config is properly typed:
- return createColumns(data.data || [], enhancedConfig, 'client');
+ return createColumns<User>(data.data || [], enhancedConfig as any, 'client');Note: The as any is a temporary solution. Ideally, you would properly type the enhancedConfig to match what createColumns expects.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const columns = useMemo(() => { | |
| if (!data) return [] as Column<User>[]; | |
| // Apply faceted counts from the API to the columns | |
| const enhancedConfig = columnsConfig.map(config => { | |
| if (config.type === 'option' && data.facetedCounts[config.id]) { | |
| return { | |
| ...config, | |
| facetedOptions: new Map( | |
| Object.entries(data.facetedCounts[config.id]).map(([key, count]) => [key, count]) | |
| ) | |
| }; | |
| } | |
| return config; | |
| }); | |
| return createColumns(data.data || [], enhancedConfig, 'client'); | |
| }, [data, columnsConfig]); | |
| const columns = useMemo(() => { | |
| if (!data) return [] as Column<User>[]; | |
| // Apply faceted counts from the API to the columns | |
| const enhancedConfig = columnsConfig.map(config => { | |
| if (config.type === 'option' && data.facetedCounts[config.id]) { | |
| return { | |
| ...config, | |
| facetedOptions: new Map( | |
| Object.entries(data.facetedCounts[config.id]).map(([key, count]) => [key, count]) | |
| ) | |
| }; | |
| } | |
| return config; | |
| }); | |
| - return createColumns(data.data || [], enhancedConfig, 'client'); | |
| + return createColumns<User>(data.data || [], enhancedConfig as any, 'client'); | |
| }, [data, columnsConfig]); |
🧰 Tools
🪛 GitHub Actions: Run Tests
[error] 100-100: TypeScript TS2345: Argument type incompatible with expected 'readonly ColumnConfig<User, any, any, any>[]'. Property 'icon' type incompatible; missing '$$typeof' property.
🤖 Prompt for AI Agents (early access)
In
packages/components/src/ui/data-table-filter/examples/data-table-filter-example.tsx
around lines 84 to 101, the call to createColumns has a TypeScript error due to
incompatible argument types, likely from the icon property in columnsConfig. To
fix this, first correct the icon property types in columnsConfig, then
explicitly type enhancedConfig to match the expected input type of createColumns
instead of using 'as any'. This ensures type compatibility and resolves the
error without resorting to temporary type assertions.
| const columnsConfig = useMemo(() => [ | ||
| { | ||
| id: 'role', | ||
| type: 'option' as const, | ||
| displayName: 'Role', | ||
| accessor: (user: User) => user.role, | ||
| icon: () => null, | ||
| options: [ | ||
| { value: 'admin', label: 'Admin' }, | ||
| { value: 'user', label: 'User' }, | ||
| { value: 'editor', label: 'Editor' }, | ||
| ], | ||
| }, | ||
| { | ||
| id: 'status', | ||
| type: 'option' as const, | ||
| displayName: 'Status', | ||
| accessor: (user: User) => user.status, | ||
| icon: () => null, | ||
| options: [ | ||
| { value: 'active', label: 'Active' }, | ||
| { value: 'inactive', label: 'Inactive' }, | ||
| { value: 'pending', label: 'Pending' }, | ||
| ], | ||
| }, | ||
| ], []); |
There was a problem hiding this comment.
Fix TypeScript errors in column configuration.
Similar to the simplified example, the icon functions need return type annotations.
- icon: () => null,
+ icon: (): React.ReactNode => null,- icon: () => null,
+ icon: (): React.ReactNode => null,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const columnsConfig = useMemo(() => [ | |
| { | |
| id: 'role', | |
| type: 'option' as const, | |
| displayName: 'Role', | |
| accessor: (user: User) => user.role, | |
| icon: () => null, | |
| options: [ | |
| { value: 'admin', label: 'Admin' }, | |
| { value: 'user', label: 'User' }, | |
| { value: 'editor', label: 'Editor' }, | |
| ], | |
| }, | |
| { | |
| id: 'status', | |
| type: 'option' as const, | |
| displayName: 'Status', | |
| accessor: (user: User) => user.status, | |
| icon: () => null, | |
| options: [ | |
| { value: 'active', label: 'Active' }, | |
| { value: 'inactive', label: 'Inactive' }, | |
| { value: 'pending', label: 'Pending' }, | |
| ], | |
| }, | |
| ], []); | |
| const columnsConfig = useMemo(() => [ | |
| { | |
| id: 'role', | |
| type: 'option' as const, | |
| displayName: 'Role', | |
| accessor: (user: User) => user.role, | |
| icon: (): React.ReactNode => null, | |
| options: [ | |
| { value: 'admin', label: 'Admin' }, | |
| { value: 'user', label: 'User' }, | |
| { value: 'editor', label: 'Editor' }, | |
| ], | |
| }, | |
| { | |
| id: 'status', | |
| type: 'option' as const, | |
| displayName: 'Status', | |
| accessor: (user: User) => user.status, | |
| icon: (): React.ReactNode => null, | |
| options: [ | |
| { value: 'active', label: 'Active' }, | |
| { value: 'inactive', label: 'Inactive' }, | |
| { value: 'pending', label: 'Pending' }, | |
| ], | |
| }, | |
| ], []); |
🧰 Tools
🪛 GitHub Actions: Run Tests
[error] 62-62: TypeScript TS7023: 'icon' implicitly has return type 'any' because it lacks a return type annotation.
[error] 74-74: TypeScript TS7023: 'icon' implicitly has return type 'any' because it lacks a return type annotation.
🤖 Prompt for AI Agents (early access)
In
packages/components/src/ui/data-table-filter/examples/data-table-filter-example.tsx
between lines 56 and 81, the icon functions in the columnsConfig array lack
explicit return type annotations, causing TypeScript errors. Add explicit return
type annotations to these icon functions, specifying the correct React element
or null return type to resolve the type errors.
| const actions = useMemo(() => { | ||
| return { | ||
| addFilterValue: (column, values) => { | ||
| setFilters(prev => { | ||
| const filter = prev.find(f => f.columnId === column.id); | ||
| if (!filter) { | ||
| return [...prev, { | ||
| columnId: column.id, | ||
| type: column.type, | ||
| operator: column.type === 'option' ? 'is any of' : 'contains', | ||
| values | ||
| }]; | ||
| } | ||
| return prev.map(f => | ||
| f.columnId === column.id | ||
| ? { ...f, values: [...new Set([...f.values, ...values])] } | ||
| : f | ||
| ); | ||
| }); | ||
| }, | ||
| removeFilterValue: (column, values) => { | ||
| setFilters(prev => { | ||
| const filter = prev.find(f => f.columnId === column.id); | ||
| if (!filter) return prev; | ||
|
|
||
| const newValues = filter.values.filter(v => !values.includes(v)); | ||
| if (newValues.length === 0) { | ||
| return prev.filter(f => f.columnId !== column.id); | ||
| } | ||
|
|
||
| return prev.map(f => | ||
| f.columnId === column.id | ||
| ? { ...f, values: newValues } | ||
| : f | ||
| ); | ||
| }); | ||
| }, | ||
| setFilterValue: (column, values) => { | ||
| setFilters(prev => { | ||
| const exists = prev.some(f => f.columnId === column.id); | ||
| if (!exists) { | ||
| return [...prev, { | ||
| columnId: column.id, | ||
| type: column.type, | ||
| operator: column.type === 'option' ? 'is any of' : 'contains', | ||
| values | ||
| }]; | ||
| } | ||
| return prev.map(f => | ||
| f.columnId === column.id | ||
| ? { ...f, values } | ||
| : f | ||
| ); | ||
| }); | ||
| }, | ||
| setFilterOperator: (columnId, operator) => { | ||
| setFilters(prev => | ||
| prev.map(f => | ||
| f.columnId === columnId | ||
| ? { ...f, operator } | ||
| : f | ||
| ) | ||
| ); | ||
| }, | ||
| removeFilter: (columnId) => { | ||
| setFilters(prev => prev.filter(f => f.columnId !== columnId)); | ||
| }, | ||
| removeAllFilters: () => { | ||
| setFilters([]); | ||
| } | ||
| }; | ||
| }, [setFilters]); |
There was a problem hiding this comment.
Add explicit type annotations for filter action parameters.
Multiple TypeScript errors indicate implicit 'any' types for the parameters in filter actions.
- addFilterValue: (column, values) => {
+ addFilterValue: (column: Column<User>, values: Array<any>) => {Apply similar type annotations to all action functions to resolve the implicit 'any' type errors.
🧰 Tools
🪛 GitHub Actions: Run Tests
[error] 106-106: TypeScript TS7006: Parameter 'column' implicitly has 'any' type.
[error] 106-106: TypeScript TS7006: Parameter 'values' implicitly has 'any' type.
[error] 124-124: TypeScript TS7006: Parameter 'column' implicitly has 'any' type.
[error] 124-124: TypeScript TS7006: Parameter 'values' implicitly has 'any' type.
[error] 141-141: TypeScript TS7006: Parameter 'column' implicitly has 'any' type.
[error] 141-141: TypeScript TS7006: Parameter 'values' implicitly has 'any' type.
[error] 159-159: TypeScript TS7006: Parameter 'columnId' implicitly has 'any' type.
[error] 159-159: TypeScript TS7006: Parameter 'operator' implicitly has 'any' type.
[error] 168-168: TypeScript TS7006: Parameter 'columnId' implicitly has 'any' type.
🤖 Prompt for AI Agents (early access)
In
packages/components/src/ui/data-table-filter/examples/data-table-filter-example.tsx
between lines 104 and 175, the filter action functions have parameters without
explicit type annotations causing implicit 'any' TypeScript errors. Fix this by
adding explicit type annotations for all parameters in the action functions,
such as specifying the types for 'column', 'values', 'columnId', and 'operator'
according to the expected data structures or interfaces used in the component.
This will resolve the implicit 'any' errors and improve type safety.
| export function useIssuesQuery( | ||
| filters: FiltersState, | ||
| options?: { | ||
| enabled?: boolean; | ||
| refetchInterval?: number | false; | ||
| onSuccess?: (data: IssuesApiResponse) => void; | ||
| onError?: (error: Error) => void; | ||
| } | ||
| ) { | ||
| return useDataQuery('/api/issues', filters, options); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make the API endpoint configurable and update type parameters to match useDataQuery changes.
The endpoint is currently hardcoded, which reduces flexibility for different environments. Also, this hook should be updated to match the type parameter changes made to useDataQuery.
-export function useIssuesQuery(
+export function useIssuesQuery<T = Issue>(
filters: FiltersState,
options?: {
enabled?: boolean;
refetchInterval?: number | false;
- onSuccess?: (data: IssuesApiResponse) => void;
- onError?: (error: Error) => void;
+ endpoint?: string;
+ queryOptions?: {
+ onSuccess?: (data: IssuesApiResponse<T>) => void;
+ onError?: (error: Error) => void;
+ };
}
) {
- return useDataQuery('/api/issues', filters, options);
+ const apiEndpoint = options?.endpoint || '/api/issues';
+ return useDataQuery<T>(apiEndpoint, filters, options);
}This makes the API endpoint configurable while maintaining the default value, and ensures the hook's signature is consistent with the updated useDataQuery function.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents (early access)
In packages/components/src/ui/utils/use-issues-query.ts around lines 67 to 77,
the API endpoint is hardcoded and the type parameters do not align with the
updated useDataQuery function. Modify the function to accept the API endpoint as
a parameter with a default value of '/api/issues' to allow configurability.
Update the type parameters in the function signature and the useDataQuery call
to match the new type definitions, ensuring consistency and flexibility.
| export function useDataQuery<T = any>( | ||
| endpoint: string, | ||
| filters: FiltersState, | ||
| options?: { | ||
| enabled?: boolean; | ||
| refetchInterval?: number | false; | ||
| onSuccess?: (data: IssuesApiResponse<T>) => void; | ||
| onError?: (error: Error) => void; | ||
| } | ||
| ) { | ||
| return useQuery({ | ||
| queryKey: [endpoint, filters], // Use endpoint and filters in the query key for caching | ||
| queryFn: () => fetchData<T>(endpoint, filters), | ||
| placeholderData: (previousData) => previousData, // Keep previous data while fetching | ||
| enabled: options?.enabled !== false, // Enabled by default unless explicitly disabled | ||
| refetchInterval: options?.refetchInterval, // Optional refetch interval | ||
| onSuccess: options?.onSuccess, | ||
| onError: options?.onError, | ||
| // Reduced stale time to ensure more frequent updates | ||
| staleTime: 30 * 1000, // 30 seconds | ||
| }); | ||
| } |
There was a problem hiding this comment.
Fix TanStack Query options and remove default any type.
There's a pipeline failure indicating an issue with the onSuccess property in line 52. The current implementation specifies properties that may not match what TanStack Query expects. Additionally, using any as a default type parameter reduces type safety.
-export function useDataQuery<T = any>(
+export function useDataQuery<T>(
endpoint: string,
filters: FiltersState,
options?: {
enabled?: boolean;
refetchInterval?: number | false;
- onSuccess?: (data: IssuesApiResponse<T>) => void;
- onError?: (error: Error) => void;
+ queryOptions?: {
+ onSuccess?: (data: IssuesApiResponse<T>) => void;
+ onError?: (error: Error) => void;
+ };
}
) {
return useQuery({
queryKey: [endpoint, filters], // Use endpoint and filters in the query key for caching
- queryFn: () => fetchData<T>(endpoint, filters),
+ queryFn: () => fetchData<T>(endpoint, filters),
placeholderData: (previousData) => previousData, // Keep previous data while fetching
enabled: options?.enabled !== false, // Enabled by default unless explicitly disabled
refetchInterval: options?.refetchInterval, // Optional refetch interval
- onSuccess: options?.onSuccess,
- onError: options?.onError,
+ ...(options?.queryOptions || {}), // Spread additional query options
// Reduced stale time to ensure more frequent updates
staleTime: 30 * 1000, // 30 seconds
});
}The pipeline failure suggests that the onSuccess property doesn't exist in the expected type. TanStack Query's useQuery function might expect these callbacks to be passed differently. The fix above nests these options under a queryOptions object, which you can then spread into the useQuery call.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function useDataQuery<T = any>( | |
| endpoint: string, | |
| filters: FiltersState, | |
| options?: { | |
| enabled?: boolean; | |
| refetchInterval?: number | false; | |
| onSuccess?: (data: IssuesApiResponse<T>) => void; | |
| onError?: (error: Error) => void; | |
| } | |
| ) { | |
| return useQuery({ | |
| queryKey: [endpoint, filters], // Use endpoint and filters in the query key for caching | |
| queryFn: () => fetchData<T>(endpoint, filters), | |
| placeholderData: (previousData) => previousData, // Keep previous data while fetching | |
| enabled: options?.enabled !== false, // Enabled by default unless explicitly disabled | |
| refetchInterval: options?.refetchInterval, // Optional refetch interval | |
| onSuccess: options?.onSuccess, | |
| onError: options?.onError, | |
| // Reduced stale time to ensure more frequent updates | |
| staleTime: 30 * 1000, // 30 seconds | |
| }); | |
| } | |
| export function useDataQuery<T>( | |
| endpoint: string, | |
| filters: FiltersState, | |
| options?: { | |
| enabled?: boolean; | |
| refetchInterval?: number | false; | |
| queryOptions?: { | |
| onSuccess?: (data: IssuesApiResponse<T>) => void; | |
| onError?: (error: Error) => void; | |
| }; | |
| } | |
| ) { | |
| return useQuery({ | |
| queryKey: [endpoint, filters], // Use endpoint and filters in the query key for caching | |
| queryFn: () => fetchData<T>(endpoint, filters), | |
| placeholderData: (previousData) => previousData, // Keep previous data while fetching | |
| enabled: options?.enabled !== false, // Enabled by default unless explicitly disabled | |
| refetchInterval: options?.refetchInterval, // Optional refetch interval | |
| ...(options?.queryOptions || {}), // Spread additional query options | |
| // Reduced stale time to ensure more frequent updates | |
| staleTime: 30 * 1000, // 30 seconds | |
| }); | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 36-36: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
🪛 GitHub Actions: Run Tests
[error] 52-52: TypeScript TS2769: No overload matches call; object literal specifies unknown property 'onSuccess' which does not exist in expected type.
🤖 Prompt for AI Agents (early access)
In packages/components/src/ui/utils/use-issues-query.ts lines 36 to 57, the
useQuery call incorrectly passes onSuccess and onError directly, causing type
errors and reducing type safety by defaulting to any. To fix this, remove the
default any type parameter and ensure the options object matches TanStack
Query's expected QueryOptions type by spreading options directly into useQuery
instead of manually assigning onSuccess and onError. This aligns with TanStack
Query's API and improves type safety.
- Updated VSCode settings to disable HTTP/2 for improved development experience. - Refactored data table filter stories to improve type imports and added new mock data for testing. - Revised pagination and sorting state management in DataTableWithBazzaFilters for better synchronization with URL parameters. - Enhanced DataTable component to accept a className prop for improved styling flexibility. - Updated data table filter logic to handle faceted options using Maps for better performance and clarity. - Added debug logging in useDataTableFilters for enhanced visibility during development.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/components/src/ui/utils/use-data-table-filters.tsx (1)
270-281: Cannot clear number / date filters – same issue as previously flagged
🧹 Nitpick comments (2)
packages/components/src/ui/data-table/data-table.tsx (1)
2-2: Prefer the shorter relative import path
data-table.tsxsits right besideutils(one directory up).
Using"../utils"is a bit easier to read and maintain than"../../ui/utils", and avoids an extra unnecessary traversal.-import { cn } from '../../ui/utils'; +import { cn } from '../utils';apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx (1)
343-344: Consider formatting dates once at source to avoid per-cell re-parsing
new Date(row.getValue('createdDate')).toLocaleDateString()runs aDateparse on every render of every row.
If the server already returns an ISO string, you can:
- Parse once in the loader (
createdDate: new Date(...).toISOString())- Format directly:
cell: ({ row }) => <div>{row.getValue<string>('createdDate')}</div>Or import a lightweight formatter (
date-fns/format) for locale-friendly output without repeated parsing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.vscode/settings.json(1 hunks)apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx(1 hunks)packages/components/src/ui/data-table-filter/components/data-table-filter.tsx(1 hunks)packages/components/src/ui/data-table/data-table.tsx(3 hunks)packages/components/src/ui/utils/use-data-table-filters.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .vscode/settings.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/src/ui/data-table-filter/components/data-table-filter.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/components/src/ui/data-table/data-table.tsx (1)
packages/components/src/ui/utils/index.ts (1)
cn(4-6)
🔇 Additional comments (3)
packages/components/src/ui/data-table/data-table.tsx (2)
12-14:classNameprop addition looks goodExposing
classNamegives downstream code the flexibility it needs without affecting existing behaviour. 👍
24-24: Great use ofcnto merge external stylesMerging the caller-supplied
classNamewith your internal spacing class via thecnutility is exactly the right approach and prevents accidental Tailwind clashes.apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx (1)
507-508: Correct use ofcolumns.lengthforcolSpanNice catch – passing
columns.lengthaligns withDataTable’s expectation of a numeric column count, fixing the mismatch noted in an earlier review.
| if (faceted instanceof Map && (config.type === 'option' || config.type === 'multiOption')) { | ||
| const potentialMapForColumn = faceted.get(config.id as OptionColumnIds<TColumns>); | ||
| if (potentialMapForColumn && isColumnOptionMap(potentialMapForColumn)) { | ||
| final = { ...final, facetedOptions: potentialMapForColumn }; | ||
| } else { | ||
| // If faceted is a Map but the entry for this column isn't a Map or doesn't exist, return original config. | ||
| return config; | ||
| } | ||
| } else if (config.type === 'option' || config.type === 'multiOption') { | ||
| // If faceted is not a Map (or not provided) but it's an option column, return original config. | ||
| return config; | ||
| } | ||
|
|
||
| // Set faceted min/max values, if exists | ||
| if (faceted instanceof Map && config.type === 'number') { | ||
| const potentialTupleForColumn = faceted.get(config.id as NumberColumnIds<TColumns>); | ||
| if (potentialTupleForColumn && isMinMaxTuple(potentialTupleForColumn)) { | ||
| final = { | ||
| ...final, | ||
| min: potentialTupleForColumn[0], | ||
| max: potentialTupleForColumn[1], | ||
| }; | ||
| } else { | ||
| // If faceted is a Map but the entry for this column isn't a tuple or doesn't exist, return original config. | ||
| return config; | ||
| } | ||
| } else if (config.type === 'number') { | ||
| // If faceted is not a Map (or not provided) but it's a number column, return original config. | ||
| return config; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
faceted is typed as a Record but treated as a Map – leads to runtime errors
DataTableFiltersOptions.faceted is declared as a (partial) Record<columnId, …>, yet the code checks faceted instanceof Map and later calls faceted.get(...).
If a caller supplies the object form (which is permitted by the type), instanceof Map will be false, so option & number columns silently miss out on faceted data.
Conversely, declaring the param as Map makes the current implementation correct but the type wrong.
Pick one representation and use it consistently – Map works well for iteration:
- faceted?: Partial<
- | Record<OptionColumnIds<TColumns>, Map<string, number> | undefined>
- | Record<NumberColumnIds<TColumns>, [number, number] | undefined>
- >;
+ faceted?: Map<
+ OptionColumnIds<TColumns> | NumberColumnIds<TColumns>,
+ Map<string, number> | [number, number] | undefined
+ >;Then remove all instanceof checks and access via faceted.get(...).
Failing to align these will surface as “faceted.get is not a function” in production.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents (early access)
In packages/components/src/ui/utils/use-data-table-filters.tsx between lines 85
and 113, the variable faceted is typed as a Record but the code treats it as a
Map, using instanceof Map checks and calling faceted.get(), which causes runtime
errors if faceted is actually a Record object. To fix this, choose either Map or
Record as the consistent type for faceted. If you choose Map, update the type
declaration accordingly and remove all instanceof checks, accessing faceted data
only via faceted.get(). If you choose Record, remove all instanceof checks and
replace faceted.get() calls with standard property access (faceted[config.id]).
Ensure the code consistently uses the chosen data structure to avoid runtime
errors.
| // --- MODIFIED DEBUG LOG HERE --- | ||
| console.log('[useDataTableFilters] Inspecting enhancedConfigs before passing to createColumns:'); | ||
| enhancedConfigs.forEach((config, index) => { | ||
| console.log(` [Enhanced Config Index: ${index}, ID: ${config.id}]`, config); | ||
| if (config.facetedOptions instanceof Map) { | ||
| console.log(` ↳ Faceted Options (Map entries):`, Array.from(config.facetedOptions.entries())); | ||
| } else { | ||
| console.log(` ↳ Faceted Options:`, config.facetedOptions); // Log it directly if not a map, to see what it is | ||
| } | ||
| }); | ||
| // --- END DEBUG LOG --- |
There was a problem hiding this comment.
Remove temporary debug logs before merging
console.log statements left in production hooks will clutter the browser console and leak internal data structures.
Please strip these lines (119–129) or guard them behind an environment check.
-// --- MODIFIED DEBUG LOG HERE ---
-console.log('[useDataTableFilters] Inspecting enhancedConfigs before passing to createColumns:');
-enhancedConfigs.forEach((config, index) => {
- console.log(` [Enhanced Config Index: ${index}, ID: ${config.id}]`, config);
- if (config.facetedOptions instanceof Map) {
- console.log(` ↳ Faceted Options (Map entries):`, Array.from(config.facetedOptions.entries()));
- } else {
- console.log(` ↳ Faceted Options:`, config.facetedOptions);
- }
-});
-// --- END DEBUG LOG ---
+// (debug logging removed)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // --- MODIFIED DEBUG LOG HERE --- | |
| console.log('[useDataTableFilters] Inspecting enhancedConfigs before passing to createColumns:'); | |
| enhancedConfigs.forEach((config, index) => { | |
| console.log(` [Enhanced Config Index: ${index}, ID: ${config.id}]`, config); | |
| if (config.facetedOptions instanceof Map) { | |
| console.log(` ↳ Faceted Options (Map entries):`, Array.from(config.facetedOptions.entries())); | |
| } else { | |
| console.log(` ↳ Faceted Options:`, config.facetedOptions); // Log it directly if not a map, to see what it is | |
| } | |
| }); | |
| // --- END DEBUG LOG --- | |
| // (debug logging removed) |
🤖 Prompt for AI Agents (early access)
In packages/components/src/ui/utils/use-data-table-filters.tsx between lines 119
and 129, remove the temporary console.log debug statements or wrap them in a
condition that checks for a development environment (e.g., process.env.NODE_ENV
=== 'development') to prevent logging internal data structures in production.
- Added "Bazza" to the cSpell dictionary for improved spell checking. - Simplified type imports in data-table-bazza-filters stories for clarity. - Enhanced filter handling in DataTableWithBazzaFilters to ensure robust updates and synchronization with URL parameters. - Improved type safety in filter-selector component by refining filter type usage. - Streamlined dialog and command components for better readability and maintainability.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx (1)
475-479:DataTablestill receivescolumns.lengthinstead of the column defsPassing a number deprives the table of header / cell metadata and repeats the issue flagged in the previous review.
- <DataTable className="mt-4" table={table} columns={columns.length} pagination /> + <DataTable className="mt-4" table={table} columns={columns} pagination />If
DataTablereally expects a count, rename the prop tocolumnCountto avoid confusion.
🧹 Nitpick comments (3)
packages/components/src/ui/data-table-filter/core/filters.ts (3)
53-58:icon()leaksanyand bypasses JSX type checking
icon(value: ElementType<any>)accepts anything that happens to be a valid JSX tag – even a string.
Consider narrowing the contract to React components only:-import { memo } from '../lib/memo'; +import type { ComponentType } from 'react'; … - // biome-ignore lint/suspicious/noExplicitAny: any allows for flexibility - icon(value: ElementType<any>): ColumnConfigBuilder<…> { + icon<TProps = unknown>( + value: ComponentType<TProps> | keyof JSX.IntrinsicElements, + ): ColumnConfigBuilder<…> {This affords consumers proper prop-type IntelliSense and removes the
biome-ignorewaiver.
305-309: Unnecessary work on server strategy
getValuesis evaluated, memoised, and later passed togetFacetedUniqueValueseven whenstrategy === "server", where it is guaranteed to return[].Short-circuiting early avoids a needless array allocation and a memo entry per column:
- () => (strategy === 'client' ? getColumnValues(columnConfig, data) : []), + () => { + if (strategy === 'client') { + return getColumnValues(columnConfig, data); + } + return []; // avoid accessor call & memo entry + },This is minor but helps large tables with many columns.
352-376:setTimeout(…, 0)prefetch may starve in busy event loopsUsing a zero-delay macro-task for pre-fetching adds at least one event-loop tick of latency and competes with UI work. A micro-task (
queueMicrotask) or a synchronous warm-up would fulfil the “make sure the cache is filled before first real use” requirement with less churn:- await new Promise((resolve) => - setTimeout(() => { - const options = getOptions(); - column._prefetchedOptionsCache = options; - resolve(undefined); - }, 0), - ); + if (!column._prefetchedOptionsCache) { + column._prefetchedOptionsCache = getOptions(); + }Same adjustment applies to the other three prefetch helpers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.vscode/settings.json(2 hunks)apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx(1 hunks)packages/components/src/ui/command.tsx(3 hunks)packages/components/src/ui/data-table-filter/components/filter-selector.tsx(1 hunks)packages/components/src/ui/data-table-filter/core/filters.ts(1 hunks)packages/components/src/ui/dialog.tsx(1 hunks)packages/components/src/ui/utils/debounce.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- .vscode/settings.json
- packages/components/src/ui/dialog.tsx
- packages/components/src/ui/data-table-filter/components/filter-selector.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/components/src/ui/command.tsx (1)
packages/components/src/ui/utils/index.ts (1)
cn(4-6)
🔇 Additional comments (14)
packages/components/src/ui/command.tsx (3)
4-4: Good addition of full React import.The change from a type-only import to a full import is necessary to support the
React.forwardReffunctionality used in the refactored components.
33-48: Well-implemented ref forwarding for CommandInput.The refactoring of
CommandInputto useReact.forwardRefenhances the component's compatibility with form libraries and parent components that need to access the underlying DOM element. The implementation correctly forwards the ref and maintains all existing functionality.This change improves the composability of the component within the UI system.
85-97: Good implementation of ref forwarding for CommandItem.Similar to the
CommandInputrefactoring, this change properly implements ref forwarding for theCommandItemcomponent, making it more flexible when used in complex component hierarchies.The TypeScript typing is well-structured, using
React.ElementRefandReact.ComponentPropsWithoutRefto ensure type safety.packages/components/src/ui/utils/debounce.ts (10)
1-9: Well-documented and comprehensive JSDoc.The JSDoc comments clearly describe the purpose and parameters of the debounce function. This is especially helpful for a utility function that will likely be used throughout the codebase.
11-11: Biome ignore comment is appropriately used.The
biome-ignorecomment for thenoExplicitAnylint rule is justified for a utility function like this where type flexibility is necessary.
12-24: Well-structured function signature with robust return type.The function signature is well-designed with:
- Generic type parameters to preserve the original function's signature
- Default wait time of 300ms
- Comprehensive options object
- Return type that includes the debounced function plus cancel/flush methods
This provides excellent TypeScript support while maintaining flexibility.
25-35: Good state tracking with clear options handling.The closure variables are well-defined to track state between invocations. The options processing is clear, with sensible defaults (trailing enabled by default).
36-44: Clean function invocation logic.The
invokeFunchelper properly handles:
- Capturing and clearing state
- Updating the invoke time
- Preserving the original function's context and arguments
- Returning the original function's result
This ensures the debounced function behaves as expected.
54-84: Robust edge case handling in timing calculations.The
leadingEdge,remainingWait, andshouldInvokefunctions handle various edge cases:
- First function call
- System time changes/going backwards
- Maximum wait time limits
- Proper timing calculations based on last call and last invoke times
This ensures the debounce behavior remains consistent even in unexpected scenarios.
86-106: Well-implemented timer expiration and trailing edge logic.The timer expiration logic properly manages the debounce behavior by either invoking the trailing edge or rescheduling the timer based on remaining wait time. The trailing edge implementation respects the options and only invokes if there are pending arguments.
108-118: Clear cleanup and control methods.The
cancelandflushmethods provide excellent control mechanisms for:
- Canceling pending invocations and cleaning up state
- Forcing immediate execution of pending invocations
This gives consumers flexibility in controlling the debounced function's behavior.
121-143: Core debouncing logic handles all cases correctly.The main
debouncedfunction properly:
- Captures arguments and context
- Determines if immediate invocation is needed
- Handles various invocation scenarios (first call, tight loops with maxWait, etc.)
- Manages timer state
The implementation is robust and covers all edge cases.
145-149: Good method attachment and return.Attaching the
cancelandflushmethods to the debounced function and returning it provides a clean API for consumers.apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx (1)
363-373: Initial pagination state can overshoot available pages
pageIndexis initialised from URL parameters beforepageCountis known.
If the user bookmarkspage=10but the filter later shrinks the dataset to only 3 pages, the table tries to display an empty page until the effect runs and resets state.Consider clamping on initialisation:
const [pagination, setPagination] = useState<PaginationState>(() => ({ - pageIndex: currentPageIndexFromUrl, + pageIndex: Math.min(currentPageIndexFromUrl, pageCount - 1), pageSize: currentPageSizeFromUrl, }));(or fall back to
0whencurrentPageIndexFromUrl >= pageCount).
| id<TNewId extends string>(value: TNewId): ColumnConfigBuilder<TData, TType, TVal, TNewId> { | ||
| const newInstance = this.clone(); | ||
| (newInstance.config as Partial<ColumnConfig<TData, TType, TVal, TNewId>>).id = value; | ||
| return newInstance as unknown as ColumnConfigBuilder<TData, TType, TVal, TNewId>; | ||
| } | ||
|
|
||
| accessor<TNewVal>(accessor: TAccessorFn<TData, TNewVal>): ColumnConfigBuilder<TData, TType, TNewVal, TId> { | ||
| const newInstance = this.clone(); | ||
| (newInstance.config as Partial<ColumnConfig<TData, TType, TNewVal, TId>>).accessor = accessor; | ||
| return newInstance as unknown as ColumnConfigBuilder<TData, TType, TNewVal, TId>; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Runtime-only type guards defeat the main goal of the fluent builder
min, max, options, transformOptionFn, and orderFn are callable on any ColumnConfigBuilder instance.
That means the following compiles yet explodes at runtime:
createColumnConfigHelper<any>()
.text()
.min(0) // compiles – should not
.options([]); // compiles – should not‣ The generic return types change to 'number' | 'option' etc., but the methods are still present on an incompatible builder, leaving the safety burden to a throw branch inside each method.
‣ We can keep fluent ergonomics and remove the casts / runtime errors by splitting the builder into scoped sub-types (one per data-type) or by discriminating on TType with conditional types so that the undesirable methods are never for incompatible builders.
Example sketch:
type NumericBuilder<TData, TVal, TId extends string> =
Omit<ColumnConfigBuilder<TData, 'number', TVal, TId>, 'options'|'transformOptionFn'> &
{ min(value: number): NumericBuilder<TData,TVal,TId>; max(...): … };
function text<TData>() { /* returns TextBuilder */ }
function number<TData>(){ /* returns NumericBuilder */ }This removes every unknown as cast, makes misuse impossible at compile time, and silences the Biome “unexpected any/unknown” lints.
🤖 Prompt for AI Agents (early access)
In packages/components/src/ui/data-table-filter/core/filters.ts around lines 35
to 45, the fluent builder methods like min, max, options, transformOptionFn, and
orderFn are callable on any ColumnConfigBuilder instance regardless of the data
type, causing runtime errors. To fix this, refactor the builder by splitting it
into scoped sub-types or use conditional types discriminated on TType so that
only compatible methods are available for each builder variant. This eliminates
unsafe casts and runtime errors by enforcing method availability at compile
time, improving type safety and developer experience.
1. Fix server-side filtering to properly use filters from URL params 2. Ensure filters persist on page refresh by using URL as source of truth 3. Fix pagination to allow navigation between pages 4. Update faceted counts to reflect filtered data instead of original data
…sts-for-data-table-bazza-filters
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx (1)
450-450:<DataTable>receivescolumns.lengthinstead of column definitionsThe DataTable component is being passed a numeric count (
columns.length) rather than the actual column definitions array. This will prevent the table from rendering properly with headers and cell renderers.Apply this fix:
- <DataTable className="mt-4" table={table} columns={columns.length} pagination /> + <DataTable className="mt-4" table={table} columns={columns} pagination />🧰 Tools
🪛 GitHub Actions: Run Tests
[error] StorybookTestRunnerError: Unable to find an element with the text 'Issues Table (Bazza UI Server Filters via Loader)'. This caused the test 'Data Table/Bazza UI Filters › ServerDriven › play-test' to fail.
🧹 Nitpick comments (4)
apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx (4)
348-374: Remove unnecessary dependencies from useEffectThe static analysis tool correctly flagged that
defaultPageIndexanddefaultPageSizedon't need to be in the dependency array, as they're constants that don't change between renders.useEffect(() => { const newPageIndex = loaderData?.meta.page ?? defaultPageIndex; const newPageSize = loaderData?.meta.pageSize ?? defaultPageSize; if (pagination.pageIndex !== newPageIndex || pagination.pageSize !== newPageSize) { setPagination({ pageIndex: newPageIndex, pageSize: newPageSize }); } const params = new URLSearchParams(location.search); const sortFieldFromUrl = params.get('sortField'); const sortDescFromUrl = params.get('sortDesc') === 'true'; const currentSorting = sorting.length > 0 ? sorting[0] : null; const urlHasSorting = !!sortFieldFromUrl; if (urlHasSorting) { // Ensure sortFieldFromUrl is not null before using it with ! if ( sortFieldFromUrl && (!currentSorting || currentSorting.id !== sortFieldFromUrl || currentSorting.desc !== sortDescFromUrl) ) { setSorting([{ id: sortFieldFromUrl, desc: sortDescFromUrl }]); } } else if (currentSorting) { setSorting([]); } - }, [loaderData, location.search, pagination, sorting, defaultPageIndex, defaultPageSize]); + }, [loaderData, location.search, pagination, sorting]);🧰 Tools
🪛 Biome (1.9.4)
[error] 348-348: This hook specifies more dependencies than necessary: defaultPageIndex, defaultPageSize
This dependency can be removed from the list.
This dependency can be removed from the list.
(lint/correctness/useExhaustiveDependencies)
🪛 GitHub Actions: Run Tests
[error] StorybookTestRunnerError: Unable to find an element with the text 'Issues Table (Bazza UI Server Filters via Loader)'. This caused the test 'Data Table/Bazza UI Filters › ServerDriven › play-test' to fail.
487-525: Improve filter logic with early returns for better readabilityThe nested filter logic can be refactored to improve readability with early returns and clearer type checking.
Consider refactoring the filter logic:
// 1. Apply filters (support option and text types) if (parsedFilters.length > 0) { parsedFilters.forEach((filter) => { processedData = processedData.filter((item) => { switch (filter.type) { case 'option': { // Option filter: support multi-value (is any of) - if (Array.isArray(filter.values) && filter.values.length > 0) { - const value = item[filter.columnId as keyof MockIssue]; - if ( - typeof value === 'string' || - typeof value === 'number' || - typeof value === 'boolean' || - value === null - ) { - return filter.values.includes(value); - } - // If value is not a supported type (e.g., Date), skip filtering - return true; - } - return true; + // Return early if no values to filter by + if (!Array.isArray(filter.values) || filter.values.length === 0) return true; + + const value = item[filter.columnId as keyof MockIssue]; + + // Skip filtering for unsupported types (e.g., Date) + if ( + typeof value !== 'string' && + typeof value !== 'number' && + typeof value !== 'boolean' && + value !== null + ) return true; + + return filter.values.includes(value); } case 'text': { // Text filter: support contains - if (Array.isArray(filter.values) && filter.values.length > 0 && typeof filter.values[0] === 'string') { - const value = item[filter.columnId as keyof MockIssue]; - return typeof value === 'string' && value.toLowerCase().includes(String(filter.values[0]).toLowerCase()); - } - return true; + // Return early if no text to filter by + if (!Array.isArray(filter.values) || filter.values.length === 0 || typeof filter.values[0] !== 'string') return true; + + const value = item[filter.columnId as keyof MockIssue]; + return typeof value === 'string' && value.toLowerCase().includes(String(filter.values[0]).toLowerCase()); } // Add more filter types as needed (number, date, etc.) default: return true; } }); }); }🧰 Tools
🪛 GitHub Actions: Run Tests
[error] StorybookTestRunnerError: Unable to find an element with the text 'Issues Table (Bazza UI Server Filters via Loader)'. This caused the test 'Data Table/Bazza UI Filters › ServerDriven › play-test' to fail.
496-505: Add type safety for filtering valuesThe code assumes that filter.values can include null, which may not be safe without explicit type checking.
if (Array.isArray(filter.values) && filter.values.length > 0) { const value = item[filter.columnId as keyof MockIssue]; if ( typeof value === 'string' || typeof value === 'number' || typeof value === 'boolean' || value === null ) { - return filter.values.includes(value); + // Ensure type-safe comparison by converting to string when needed + return filter.values.some(filterValue => + filterValue === null ? value === null : String(filterValue) === String(value) + ); } // If value is not a supported type (e.g., Date), skip filtering return true;🧰 Tools
🪛 GitHub Actions: Run Tests
[error] StorybookTestRunnerError: Unable to find an element with the text 'Issues Table (Bazza UI Server Filters via Loader)'. This caused the test 'Data Table/Bazza UI Filters › ServerDriven › play-test' to fail.
313-453: Implement error boundary or loading state for DataTableWithBazzaFiltersThe component doesn't handle loading or error states, which could improve user experience.
Consider adding loading and error states:
function DataTableWithBazzaFilters() { const loaderData = useLoaderData<DataResponse>(); const location = useLocation(); const navigate = useNavigate(); + const [isLoading, setIsLoading] = useState(false); + // Add navigation listener for loading state + useEffect(() => { + const handleNavigation = () => { + setIsLoading(true); + }; + + window.addEventListener('popstate', handleNavigation); + return () => { + window.removeEventListener('popstate', handleNavigation); + }; + }, []); + + // Reset loading state when data arrives + useEffect(() => { + if (loaderData) { + setIsLoading(false); + } + }, [loaderData]); // Rest of component code... return ( <div className="container mx-auto py-10"> <h1 className="text-2xl font-bold mb-4">Issues Table (Bazza UI Server Filters via Loader)</h1> {/* ... */} {/* Render Bazza UI Filters - Pass Bazza's processed columns */} <DataTableFilter columns={bazzaProcessedColumns} filters={filters} actions={actions} strategy={strategy} /> + {/* Loading state */} + {isLoading && ( + <div className="flex justify-center items-center h-20 mt-4"> + <div className="animate-spin h-6 w-6 border-2 border-gray-500 rounded-full border-t-transparent"></div> + </div> + )} + {/* Pass table instance (which now uses original columns for rendering) */} - <DataTable className="mt-4" table={table} columns={columns.length} pagination /> + <DataTable + className="mt-4" + table={table} + columns={columns} + pagination + /> </div> ); }🧰 Tools
🪛 Biome (1.9.4)
[error] 348-348: This hook specifies more dependencies than necessary: defaultPageIndex, defaultPageSize
This dependency can be removed from the list.
This dependency can be removed from the list.
(lint/correctness/useExhaustiveDependencies)
🪛 GitHub Actions: Run Tests
[error] StorybookTestRunnerError: Unable to find an element with the text 'Issues Table (Bazza UI Server Filters via Loader)'. This caused the test 'Data Table/Bazza UI Filters › ServerDriven › play-test' to fail.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx
[error] 348-348: This hook specifies more dependencies than necessary: defaultPageIndex, defaultPageSize
This dependency can be removed from the list.
This dependency can be removed from the list.
(lint/correctness/useExhaustiveDependencies)
🪛 GitHub Actions: Run Tests
apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx
[error] StorybookTestRunnerError: Unable to find an element with the text 'Issues Table (Bazza UI Server Filters via Loader)'. This caused the test 'Data Table/Bazza UI Filters › ServerDriven › play-test' to fail.
🔇 Additional comments (1)
apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx (1)
528-537: Add safety check for empty database in sorting logicThe sorting logic assumes mockDatabase has at least one element without checking.
// 2. Apply sorting - if (sortField && sortField in mockDatabase[0]) { + if (sortField && mockDatabase.length > 0 && sortField in mockDatabase[0]) { processedData.sort((a, b) => { const aValue = a[sortField as keyof MockIssue]; const bValue = b[sortField as keyof MockIssue]; let comparison = 0; if (aValue < bValue) comparison = -1; if (aValue > bValue) comparison = 1; return sortDesc ? comparison * -1 : comparison; }); }🧰 Tools
🪛 GitHub Actions: Run Tests
[error] StorybookTestRunnerError: Unable to find an element with the text 'Issues Table (Bazza UI Server Filters via Loader)'. This caused the test 'Data Table/Bazza UI Filters › ServerDriven › play-test' to fail.
| const testInitialRender = async ({ canvasElement }: StoryContext) => { | ||
| const canvas = within(canvasElement); | ||
|
|
||
| // Check if the table is rendered with the correct title | ||
| const title = canvas.getByText('Issues Table (Bazza UI Server Filters via Loader)'); | ||
| expect(title).toBeInTheDocument(); | ||
|
|
||
| // Check if the table has the correct number of rows initially (should be pageSize) | ||
| const rows = canvas.getAllByRole('row'); | ||
| // First row is header, so we expect pageSize + 1 rows | ||
| expect(rows.length).toBeGreaterThan(1); // At least header + 1 data row | ||
|
|
||
| // Check if pagination is rendered | ||
| const paginationControls = canvas.getByRole('navigation'); | ||
| expect(paginationControls).toBeInTheDocument(); | ||
| }; | ||
|
|
||
| const testFiltering = async ({ canvasElement }: StoryContext) => { | ||
| const canvas = within(canvasElement); | ||
|
|
||
| // Open the filter dropdown | ||
| const filterButton = canvas.getByRole('button', { name: /filter/i }); | ||
| await userEvent.click(filterButton); | ||
|
|
||
| // Select a filter type (e.g., Status) | ||
| const statusFilter = await canvas.findByText('Status'); | ||
| await userEvent.click(statusFilter); | ||
|
|
||
| // Select a filter value (e.g., "Todo") | ||
| const todoOption = await canvas.findByText('Todo'); | ||
| await userEvent.click(todoOption); | ||
|
|
||
| // Apply the filter | ||
| const applyButton = canvas.getByRole('button', { name: /apply/i }); | ||
| await userEvent.click(applyButton); | ||
|
|
||
| // Wait for the table to update | ||
| await new Promise(resolve => setTimeout(resolve, 500)); | ||
|
|
||
| // Check if the URL has been updated with the filter | ||
| expect(window.location.search).toContain('filters'); | ||
|
|
||
| // Check if the filter chip is displayed | ||
| const filterChip = await canvas.findByText('Status: Todo'); | ||
| expect(filterChip).toBeInTheDocument(); | ||
| }; |
There was a problem hiding this comment.
Fix the failing test for initial render
The pipeline is failing because the test cannot find the element with text "Issues Table (Bazza UI Server Filters via Loader)". This could be due to timing issues in the test or an issue with how the component renders.
Add a wait for stabilization before checking for the title:
const testInitialRender = async ({ canvasElement }: StoryContext) => {
const canvas = within(canvasElement);
+ // Wait for the component to stabilize after initial render
+ await new Promise(resolve => setTimeout(resolve, 500));
+
// Check if the table is rendered with the correct title
const title = canvas.getByText('Issues Table (Bazza UI Server Filters via Loader)');
expect(title).toBeInTheDocument();
// Check if the table has the correct number of rows initially (should be pageSize)
const rows = canvas.getAllByRole('row');
// First row is header, so we expect pageSize + 1 rows
expect(rows.length).toBeGreaterThan(1); // At least header + 1 data row
// Check if pagination is rendered
const paginationControls = canvas.getByRole('navigation');
expect(paginationControls).toBeInTheDocument();
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const testInitialRender = async ({ canvasElement }: StoryContext) => { | |
| const canvas = within(canvasElement); | |
| // Check if the table is rendered with the correct title | |
| const title = canvas.getByText('Issues Table (Bazza UI Server Filters via Loader)'); | |
| expect(title).toBeInTheDocument(); | |
| // Check if the table has the correct number of rows initially (should be pageSize) | |
| const rows = canvas.getAllByRole('row'); | |
| // First row is header, so we expect pageSize + 1 rows | |
| expect(rows.length).toBeGreaterThan(1); // At least header + 1 data row | |
| // Check if pagination is rendered | |
| const paginationControls = canvas.getByRole('navigation'); | |
| expect(paginationControls).toBeInTheDocument(); | |
| }; | |
| const testFiltering = async ({ canvasElement }: StoryContext) => { | |
| const canvas = within(canvasElement); | |
| // Open the filter dropdown | |
| const filterButton = canvas.getByRole('button', { name: /filter/i }); | |
| await userEvent.click(filterButton); | |
| // Select a filter type (e.g., Status) | |
| const statusFilter = await canvas.findByText('Status'); | |
| await userEvent.click(statusFilter); | |
| // Select a filter value (e.g., "Todo") | |
| const todoOption = await canvas.findByText('Todo'); | |
| await userEvent.click(todoOption); | |
| // Apply the filter | |
| const applyButton = canvas.getByRole('button', { name: /apply/i }); | |
| await userEvent.click(applyButton); | |
| // Wait for the table to update | |
| await new Promise(resolve => setTimeout(resolve, 500)); | |
| // Check if the URL has been updated with the filter | |
| expect(window.location.search).toContain('filters'); | |
| // Check if the filter chip is displayed | |
| const filterChip = await canvas.findByText('Status: Todo'); | |
| expect(filterChip).toBeInTheDocument(); | |
| }; | |
| const testInitialRender = async ({ canvasElement }: StoryContext) => { | |
| const canvas = within(canvasElement); | |
| // Wait for the component to stabilize after initial render | |
| await new Promise(resolve => setTimeout(resolve, 500)); | |
| // Check if the table is rendered with the correct title | |
| const title = canvas.getByText('Issues Table (Bazza UI Server Filters via Loader)'); | |
| expect(title).toBeInTheDocument(); | |
| // Check if the table has the correct number of rows initially (should be pageSize) | |
| const rows = canvas.getAllByRole('row'); | |
| // First row is header, so we expect pageSize + 1 rows | |
| expect(rows.length).toBeGreaterThan(1); // At least header + 1 data row | |
| // Check if pagination is rendered | |
| const paginationControls = canvas.getByRole('navigation'); | |
| expect(paginationControls).toBeInTheDocument(); | |
| }; |
🧰 Tools
🪛 GitHub Actions: Run Tests
[error] StorybookTestRunnerError: Unable to find an element with the text 'Issues Table (Bazza UI Server Filters via Loader)'. This caused the test 'Data Table/Bazza UI Filters › ServerDriven › play-test' to fail.
🤖 Prompt for AI Agents (early access)
In apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx around
lines 589 to 634, the test for initial render fails because it tries to find the
title element immediately, which may not be rendered yet. To fix this, replace
the synchronous getByText call with an asynchronous findByText call to wait for
the element to appear before asserting its presence. This ensures the test waits
for the component to stabilize before checking the title.
…sts-for-data-table-bazza-filters-fix
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx (2)
447-447:⚠️ Potential issue
<DataTable>receivescolumns.lengthinstead of column definitionsThe
DataTablecomponent is passed a numeric count rather than the actualColumnDef[]. This is likely causing rendering issues as the component expects column metadata (headers, cell renderers, etc.) rather than just a count.- <DataTable className="mt-4" table={table} columns={columns.length} pagination /> + <DataTable className="mt-4" table={table} columns={columns} pagination />
586-601:⚠️ Potential issueFix the failing test for initial render
The pipeline is failing because the test cannot find the element with text "Issues Table (Bazza UI Server Filters via Loader)". This could be due to timing issues in the test or an issue with how the component renders.
const testInitialRender = async ({ canvasElement }: StoryContext) => { const canvas = within(canvasElement); + // Wait for the component to stabilize after initial render + await new Promise(resolve => setTimeout(resolve, 500)); + // Check if the table is rendered with the correct title const title = canvas.getByText('Issues Table (Bazza UI Server Filters via Loader)'); expect(title).toBeInTheDocument(); // Check if the table has the correct number of rows initially (should be pageSize) const rows = canvas.getAllByRole('row'); // First row is header, so we expect pageSize + 1 rows expect(rows.length).toBeGreaterThan(1); // At least header + 1 data row // Check if pagination is rendered const paginationControls = canvas.getByRole('navigation'); expect(paginationControls).toBeInTheDocument(); };
🧹 Nitpick comments (5)
apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx (5)
345-371: Simplify useEffect dependenciesThe
useEffectincludes unnecessary dependenciesdefaultPageIndexanddefaultPageSizewhich can be removed according to static analysis.useEffect(() => { const newPageIndex = loaderData?.meta.page ?? defaultPageIndex; const newPageSize = loaderData?.meta.pageSize ?? defaultPageSize; if (pagination.pageIndex !== newPageIndex || pagination.pageSize !== newPageSize) { setPagination({ pageIndex: newPageIndex, pageSize: newPageSize }); } const params = new URLSearchParams(location.search); const sortFieldFromUrl = params.get('sortField'); const sortDescFromUrl = params.get('sortDesc') === 'true'; const currentSorting = sorting.length > 0 ? sorting[0] : null; const urlHasSorting = !!sortFieldFromUrl; if (urlHasSorting) { // Ensure sortFieldFromUrl is not null before using it with ! if ( sortFieldFromUrl && (!currentSorting || currentSorting.id !== sortFieldFromUrl || currentSorting.desc !== sortDescFromUrl) ) { setSorting([{ id: sortFieldFromUrl, desc: sortDescFromUrl }]); } } else if (currentSorting) { setSorting([]); } - }, [loaderData, location.search, pagination, sorting, defaultPageIndex, defaultPageSize]); + }, [loaderData, location.search, pagination, sorting]);🧰 Tools
🪛 Biome (1.9.4)
[error] 345-345: This hook specifies more dependencies than necessary: defaultPageIndex, defaultPageSize
This dependency can be removed from the list.
This dependency can be removed from the list.
(lint/correctness/useExhaustiveDependencies)
490-520: Add support for date filteringThe filtering logic supports text and option types, but lacks proper support for date filtering, which could be useful for the
createdDatefield.Consider adding a case for date filtering in the switch statement:
switch (filter.type) { case 'option': { // Option filter logic... } case 'text': { // Text filter logic... } + case 'date': { + // Date filter: support range queries + if (Array.isArray(filter.values) && filter.values.length > 0) { + const value = item[filter.columnId as keyof MockIssue]; + if (value instanceof Date) { + // Handle date ranges - values[0] is start date, values[1] is end date if provided + const startDate = new Date(filter.values[0]); + const endDate = filter.values[1] ? new Date(filter.values[1]) : new Date(); + return value >= startDate && value <= endDate; + } + return true; + } + return true; + } // Add more filter types as needed (number, date, etc.) default: return true; }
479-481: Improve error handling for filter parsingCurrently, the code logs errors and continues with empty filters when parsing fails, which might hide underlying issues. Consider adding more descriptive error messages or providing UI feedback.
} catch (error) { - console.error('[Loader] - Filter parsing/validation error (expecting Bazza v0.2 model):', error); + console.error('[Loader] - Filter parsing/validation error (expecting Bazza v0.2 model):', error); + // Consider adding UI feedback about filter parsing failure + // e.g., return a message in the response that can be displayed to the user parsedFilters = []; }
312-313: Recommend adding component documentationThe component
DataTableWithBazzaFilterswould benefit from JSDoc comments explaining its purpose, props, and behavior.+/** + * DataTableWithBazzaFilters + * + * A component that demonstrates server-driven filtering, sorting, and pagination + * using Bazza UI filters and React Router loader data. + * + * It synchronizes filter state with URL parameters and uses the loader function + * to fetch filtered data from the server. + */ function DataTableWithBazzaFilters() {
543-547: Optimize faceted count calculationThe faceted counts are calculated for all filtered data, which could become a performance issue with large datasets. Consider calculating faceted counts only for the columns that are displayed in the UI filters.
// Calculate faceted counts based on the filtered data (not the original database) // This ensures counts reflect the current filtered dataset const facetedColumns: Array<keyof MockIssue> = ['status', 'assignee', 'priority']; + // Only calculate faceted counts if there is filtered data to improve performance + const facetedCounts = processedData.length > 0 + ? calculateFacetedCounts(processedData, facetedColumns, allDefinedOptions) + : {}; - const facetedCounts = calculateFacetedCounts(processedData, facetedColumns, allDefinedOptions);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx
[error] 345-345: This hook specifies more dependencies than necessary: defaultPageIndex, defaultPageSize
This dependency can be removed from the list.
This dependency can be removed from the list.
(lint/correctness/useExhaustiveDependencies)
🪛 GitHub Actions: Run Tests
apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx
[error] 214-214: Test failure in story 'Data Table/Bazza UI Filters › ServerDriven › play-test': Unable to find an element with the text 'Issues Table (Bazza UI Server Filters via Loader)'. Possible cause: text is broken up by multiple elements. See full output at Storybook URL.
| if (sortField && sortField in mockDatabase[0]) { | ||
| processedData.sort((a, b) => { | ||
| const aValue = a[sortField as keyof MockIssue]; | ||
| const bValue = b[sortField as keyof MockIssue]; | ||
| let comparison = 0; | ||
| if (aValue < bValue) comparison = -1; | ||
| if (aValue > bValue) comparison = 1; | ||
| return sortDesc ? comparison * -1 : comparison; | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for sortField to prevent errors
The current implementation doesn't validate if the sortField exists on the first mock data item before accessing it, which could lead to errors with an invalid sort field or empty data.
// 2. Apply sorting
- if (sortField && sortField in mockDatabase[0]) {
+ if (sortField && mockDatabase.length > 0 && sortField in mockDatabase[0]) {
processedData.sort((a, b) => {
const aValue = a[sortField as keyof MockIssue];
const bValue = b[sortField as keyof MockIssue];
let comparison = 0;
if (aValue < bValue) comparison = -1;
if (aValue > bValue) comparison = 1;
return sortDesc ? comparison * -1 : comparison;
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (sortField && sortField in mockDatabase[0]) { | |
| processedData.sort((a, b) => { | |
| const aValue = a[sortField as keyof MockIssue]; | |
| const bValue = b[sortField as keyof MockIssue]; | |
| let comparison = 0; | |
| if (aValue < bValue) comparison = -1; | |
| if (aValue > bValue) comparison = 1; | |
| return sortDesc ? comparison * -1 : comparison; | |
| }); | |
| } | |
| // 2. Apply sorting | |
| if (sortField && mockDatabase.length > 0 && sortField in mockDatabase[0]) { | |
| processedData.sort((a, b) => { | |
| const aValue = a[sortField as keyof MockIssue]; | |
| const bValue = b[sortField as keyof MockIssue]; | |
| let comparison = 0; | |
| if (aValue < bValue) comparison = -1; | |
| if (aValue > bValue) comparison = 1; | |
| return sortDesc ? comparison * -1 : comparison; | |
| }); | |
| } |
🤖 Prompt for AI Agents (early access)
In apps/docs/src/remix-hook-form/data-table-bazza-filters.stories.tsx around
lines 525 to 534, add validation to ensure mockDatabase is not empty and that
sortField exists as a key on the first item before attempting to sort. This
prevents runtime errors when sortField is invalid or mockDatabase is empty.
Implement a check for mockDatabase length and confirm sortField presence on the
first element before proceeding with the sort logic.
Summary by CodeRabbit