From 8f50d28777160bbb987fa899eda44f972220eaca Mon Sep 17 00:00:00 2001 From: Carlos Garcia Date: Sat, 28 Mar 2026 22:30:09 -0300 Subject: [PATCH 1/2] refactor: extract AliasAttribute type and fix subscriber cleanup in useQueryBuilder --- src/classes/Builder.ts | 36 ++++++++++++++---------------------- src/hooks/useQueryBuilder.ts | 15 +++++++++------ src/types/index.ts | 36 ++++++++++++------------------------ 3 files changed, 35 insertions(+), 52 deletions(-) diff --git a/src/classes/Builder.ts b/src/classes/Builder.ts index c4bac53..ec3a098 100644 --- a/src/classes/Builder.ts +++ b/src/classes/Builder.ts @@ -26,6 +26,7 @@ import { toArray } from "@/actions/presenter"; import { clearSortsAction, removeSortAction, sortAction } from "@/actions/sort"; import { whenAction } from "@/actions/when"; import { + type AliasAttribute, type BaseConfig, type Field, type FilterValue, @@ -164,9 +165,7 @@ export class Builder< } filter( - attribute: Aliases extends object - ? (keyof Aliases & string) | string - : string, + attribute: AliasAttribute, value: TFilter, ...args: TFilter extends OperatorType ? [override: FilterValue] @@ -232,9 +231,7 @@ export class Builder< return hasField(fields, this.state); } - hasFilter( - ...filters: (Aliases extends object ? keyof Aliases | string : string)[] - ): boolean { + hasFilter(...filters: AliasAttribute[]): boolean { return hasFilter(filters, this.state); } @@ -246,11 +243,7 @@ export class Builder< return hasParam(params, this.state); } - hasSort( - ...sorts: (Aliases extends object - ? (keyof Aliases & string) | string - : string)[] - ): boolean { + hasSort(...sorts: AliasAttribute[]): boolean { return hasSort(sorts, this.state); } @@ -277,7 +270,9 @@ export class Builder< if (this.state.pagination?.page && this.state.pagination.page >= 1) { this.setState((s) => pageAction( - /* v8 ignore next */ s.pagination?.page !== undefined ? s.pagination.page + 1 : 1, + /* v8 ignore next */ s.pagination?.page !== undefined + ? s.pagination.page + 1 + : 1, s, ), ); @@ -295,7 +290,10 @@ export class Builder< previousPage(): QueryBuilder { if (this.state.pagination?.page && this.state.pagination.page > 1) { this.setState((s) => - pageAction(/* v8 ignore next */ s.pagination?.page ? s.pagination.page - 1 : 1, s), + pageAction( + /* v8 ignore next */ s.pagination?.page ? s.pagination.page - 1 : 1, + s, + ), ); } return this; @@ -310,9 +308,7 @@ export class Builder< } removeFilter( - ...filtersToRemove: (Aliases extends object - ? (keyof Aliases & string) | string - : string)[] + ...filtersToRemove: AliasAttribute[] ): QueryBuilder { if ( this.state.filters.some((filter) => @@ -351,9 +347,7 @@ export class Builder< } removeSort( - ...sortsToRemove: (Aliases extends object - ? (keyof Aliases & string) | string - : string)[] + ...sortsToRemove: AliasAttribute[] ): QueryBuilder { const hasSortToRemove = sortsToRemove.some((s) => this.state.sorts.some((sort) => sort.attribute === s), @@ -379,9 +373,7 @@ export class Builder< } sort( - attribute: Aliases extends object - ? (keyof Aliases & string) | string - : string, + attribute: AliasAttribute, direction?: "asc" | "desc", ): QueryBuilder { const currentSort = this.state.sorts.find((f) => f.attribute === attribute); diff --git a/src/hooks/useQueryBuilder.ts b/src/hooks/useQueryBuilder.ts index 8027bbe..fd728b0 100644 --- a/src/hooks/useQueryBuilder.ts +++ b/src/hooks/useQueryBuilder.ts @@ -1,7 +1,6 @@ -import { useRef, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { Builder } from "@/classes/Builder"; -import { useMount } from "@/hooks/useMount"; -import { type BaseConfig, type QueryBuilder } from "src/types"; +import { type BaseConfig, type QueryBuilder } from "@/types"; export const useQueryBuilder = < Aliases extends Record | undefined = undefined, @@ -12,11 +11,15 @@ export const useQueryBuilder = < const [, setReRendersCounter] = useState(0); - useMount(() => { - builder.current.addSubscriber(() => { + useEffect(() => { + const instance = builder.current; + const id = instance.addSubscriber(() => { setReRendersCounter((r) => r + 1); }); - }); + return () => { + instance.removeSubscriber(id); + }; + }, []); return builder.current; }; diff --git a/src/types/index.ts b/src/types/index.ts index 0d49835..1b9e224 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -1,5 +1,9 @@ +export type AliasAttribute = Al extends object + ? (keyof Al & string) | string + : string; + export interface Sort { - attribute: T extends object ? (keyof T & string) | string : string; + attribute: AliasAttribute; direction: "asc" | "desc"; } @@ -25,7 +29,7 @@ export const FilterOperator = { export type OperatorType = (typeof FilterOperator)[keyof typeof FilterOperator]; export interface Filter { - attribute: Al extends object ? (keyof Al & string) | string : string; + attribute: AliasAttribute; value: (string | number)[]; /** Filter comparison operator. Defaults to `FilterOperator.Equals` (`=`) when omitted. */ operator?: OperatorType; @@ -84,33 +88,25 @@ export interface QueryBuilder< * builder.filter("status", "inactive", true) */ filter: ( - attribute: AliasType extends object - ? (keyof AliasType & string) | string - : string, + attribute: AliasAttribute, value: TFilter, ...override: TFilter extends OperatorType ? [override: FilterValue] : [override?: boolean] ) => QueryBuilder; removeFilter: ( - ...filtersToRemove: (AliasType extends object - ? (keyof AliasType & string) | string - : string)[] + ...filtersToRemove: AliasAttribute[] ) => QueryBuilder; clearFilters: () => QueryBuilder; include: (...includes: Include[]) => QueryBuilder; removeInclude: (...includesToRemove: Include[]) => QueryBuilder; clearIncludes: () => QueryBuilder; sort: ( - attribute: AliasType extends object - ? (keyof AliasType & string) | string - : string, + attribute: AliasAttribute, direction?: "asc" | "desc", ) => QueryBuilder; removeSort: ( - ...attributeToRemove: (AliasType extends object - ? (keyof AliasType & string) | string - : string)[] + ...attributeToRemove: AliasAttribute[] ) => QueryBuilder; clearSorts: () => QueryBuilder; build: () => string; @@ -131,16 +127,8 @@ export interface QueryBuilder< callback: (state: GlobalState) => void, ) => QueryBuilder; toArray: () => string[]; - hasFilter: ( - ...attribute: (AliasType extends object - ? keyof AliasType | string - : string)[] - ) => boolean; - hasSort: ( - ...attribute: (AliasType extends object - ? (keyof AliasType & string) | string - : string)[] - ) => boolean; + hasFilter: (...attribute: AliasAttribute[]) => boolean; + hasSort: (...attribute: AliasAttribute[]) => boolean; hasInclude: (...includes: Include[]) => boolean; hasField: (...fields: Field[]) => boolean; hasParam: (...key: string[]) => boolean; From 55313f514b7a2e5afb9d1e45442637494b9c758f Mon Sep 17 00:00:00 2001 From: Carlos Garcia Date: Sat, 28 Mar 2026 22:33:38 -0300 Subject: [PATCH 2/2] test: add subscriber cleanup test for useQueryBuilder --- tests/Units/hooks/useQueryBuilder.test.tsx | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/Units/hooks/useQueryBuilder.test.tsx b/tests/Units/hooks/useQueryBuilder.test.tsx index 78c6b66..7999050 100644 --- a/tests/Units/hooks/useQueryBuilder.test.tsx +++ b/tests/Units/hooks/useQueryBuilder.test.tsx @@ -78,4 +78,15 @@ describe("useQueryBuilder", () => { expect(result.current.build()).toBe("?filter[full_name]=John"); }); + + it("should not throw when mutating builder after unmount", () => { + const { result, unmount } = renderHook(() => useQueryBuilder()); + const builder = result.current; + + unmount(); + + expect(() => { + builder.filter("name", "John"); + }).not.toThrow(); + }); });