Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions src/SQONBuilder.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
import {
ArrayFilter,
ArrayFilterValue,
CombinationKey,
CombinationKeys,
CombinationOperator,
FilterKey,
FilterKeys,
FilterOperator,
FilterValue,
FilterTypeMap,
GreaterThanFilter,
InFilter,
LesserThanFilter,
Operator,
SQON,
ScalarFilterValue,
isArrayFilter,
isCombination,
isFilter,
} from './types/sqon';
SQON,
type ArrayFilter,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💜

type ArrayFilterValue,
type CombinationKey,
type CombinationOperator,
type FilterKey,
type FilterOperator,
type FilterTypeMap,
type FilterValue,
type GreaterThanFilter,
type InFilter,
type LesserThanFilter,
type Operator,
type ScalarFilterValue,
} from './types';
import asArray from './utils/asArray';
import checkMatchingFilter, { checkMatchingArrays } from './utils/checkMatchingFilter';
import cloneDeepValues from './utils/cloneDeepValues';
Expand Down
2 changes: 1 addition & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export * from './types/sqon';
export * from './types';
export { default as checkMatchingFilter } from './utils/checkMatchingFilter';
export { default as reduceSQON } from './utils/reduceSQON';
export { emptySQON } from './SQONBuilder';
Expand Down
2 changes: 2 additions & 0 deletions src/types/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './sqon';
export * from './sqonFilters';
122 changes: 20 additions & 102 deletions src/types/sqon.ts
Original file line number Diff line number Diff line change
@@ -1,107 +1,28 @@
import { z as zod } from 'zod';
import { matchesSchema } from '../utils/matchesSchema';
import {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like these types are missing a verbatim import?

Suggested change
import {
import type {

Copy link
Member

@justincorrigible justincorrigible Feb 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

following up on that, after going further into the file, and I'm honestly not sure how to say this without being mistaken as confrontational... so I'm just going to write my thought process, and hope you know I mean well:

  • I look at line 4 in this file.
  • in general terms, I expect arrayFilter would be the name for some "function that filters arrays" ("filter" being not very descriptive, but hey... a function that does something, and I know naming things is hard).
  • as this is a types/* file, naming something ArrayFilter reads to me as the type/signature for a function like the one I described above.
  • then I look at line 56, where it seems to be used as a template constant for validation in runtime —unless I'm misreading what that function does, which is possible while reviewing in GH; but constants have traditionally been named in upper snake case, e.g. ARRAY_FILTER. so maybe it's not a constant either?
  • not a function's signature, not a constant, what does this initial capital mean? is that a zod executable schema? why doesn't it just say so in the name instead of having nonstandard casing?
  • confoosed, I write this comment. sorry? please don't be upset.
  • fin

Tl;DR: reading further down into this file, now I'm not entirely sure of what imports are types, classes/zod objects, or constants... I find the non-standard capitalization make the code truly hard to read without intellisense.
if it helps illustrate my current understanding of casing standards in JS, this is a somewhat succinct summary: https://medium.com/@sodiq.akanmu001/javascript-casing-conventions-a-must-know-for-every-developer-770a4474a7f0

Copy link
Member

@justincorrigible justincorrigible Feb 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, went into the next file (types/sqonFilters), and there it looks like thisarrayFilter is indeed a zod schema of some sort, and the best I can think of is (and pretty please read this as "what would Justin do, he does not at all mean I have to like it"... not as "why is Justin attacking me, what a jerk, who is he to tell me how to do things" etc. please please please):

  • this arrayFilter example, being a zod executable object is technically a function.
  • that means to me it gets the usual camelCase treatment, and adding *Schema makes it clearer what the variable contains. e.g. arrayFilterSchema.
  • perhaps arrayFilterZodSchema or arrayFilterZSchema would be better? unclear.
  • it's not a type or a class, so I wouldn't use initial capital. that leads to confusion, in scopes/files where Z-usage is not obvious (such as reviewing PRs or onboarding new maintainers).
  • it's technically not just a constant either, so upper snake doesn't apply either.

what do you think? am I making any sense?
I'm more than willing to be convinced otherwise if you have good arguments, but I'm just speaking from what I'm used to being convention in JS-land. again, sorry if this upsets you, but what's the point of PR reviews otherwise if not to give honest constructive feedback/thoughts?

ArrayFilter,
ArrayFilterKeys,
ArrayFilterValue,
FilterKeys,
FilterOperator,
ScalarFilter,
ScalarFilterKeys,
ScalarFilterValue,
} from './sqonFilters';
import { Clean, Values } from './util';

/* **** *
* Keys *
* **** */
export const ArrayFilterKeys = {
In: 'in',
} as const;
export type ArrayFilterKey = Values<typeof ArrayFilterKeys>;

export const ScalarFilterKeys = {
GreaterThan: 'gt',
LesserThan: 'lt',
} as const;
export type ScalarFilterKey = Values<typeof ScalarFilterKeys>;

export type FilterKey = ScalarFilterKey | ArrayFilterKey;
export const FilterKeys = Object.assign({}, ArrayFilterKeys, ScalarFilterKeys);

export const CombinationKeys = {
And: 'and',
Or: 'or',
Not: 'not',
} as const;
export type CombinationKey = Values<typeof CombinationKeys>;

export const Keys = Object.assign({}, ArrayFilterKeys, ScalarFilterKeys, CombinationKeys);

/* ****************** *
* Filters *
* - Filter Values *
* - Specific Filters *
* ****************** */

/* ===== Filter Values ==== */

// The array value wants to be able to accept a single value or an array of values
// Arranger also doesnt care if the values are mixed numbers and strings, that will be sorted out by elasticsearch
// and in practice won't be mixed, so to simlpify type validation we use this nested union structure:
// string | number | (string | number)[]
export type ArrayFilterValue = zod.infer<typeof ArrayFilterValue>;
export const ArrayFilterValue = zod.union([
zod.union([zod.string(), zod.number()]).array(),
zod.string(),
zod.number(),
]);

export type ScalarFilterValue = zod.infer<typeof ScalarFilterValue>;
export const ScalarFilterValue = zod.number();

export type FilterValue = zod.infer<typeof FilterValue>;
export const FilterValue = zod.union([ArrayFilterValue, ScalarFilterValue]);

export type FilterTypeMap = {
[ArrayFilterKeys.In]: InFilter;
[ScalarFilterKeys.GreaterThan]: GreaterThanFilter;
[ScalarFilterKeys.LesserThan]: LesserThanFilter;
};

/* ===== Specific Filters ==== */

export type InFilterContent = zod.infer<typeof InFilterContent>;
export const InFilterContent = zod.object({
fieldName: zod.string(),
value: ArrayFilterValue,
});
export type InFilter = zod.infer<typeof InFilter>;
export const InFilter = zod.object({
op: zod.literal(ArrayFilterKeys.In),
content: InFilterContent,
});

export type GreaterThanFilterContent = zod.infer<typeof GreaterThanFilterContent>;
export const GreaterThanFilterContent = zod.object({
fieldName: zod.string(),
value: ScalarFilterValue,
});
export type GreaterThanFilter = zod.infer<typeof GreaterThanFilter>;
export const GreaterThanFilter = zod.object({
op: zod.literal(ScalarFilterKeys.GreaterThan),
content: GreaterThanFilterContent,
});

export type LesserThanFilterContent = zod.infer<typeof LesserThanFilterContent>;
export const LesserThanFilterContent = zod.object({
fieldName: zod.string(),
value: ScalarFilterValue,
});
export type LesserThanFilter = zod.infer<typeof LesserThanFilter>;
export const LesserThanFilter = zod.object({
op: zod.literal(ScalarFilterKeys.LesserThan),
content: LesserThanFilterContent,
});

export type ArrayFilter = zod.infer<typeof ArrayFilter>;
export const ArrayFilter = InFilter; // zod.union([InFilter]); // If other arrays are added, expand this to be a union
export type ScalarFilter = zod.infer<typeof ScalarFilter>;
export const ScalarFilter = zod.union([GreaterThanFilter, LesserThanFilter]); // If other arrays are added, expand this to be a union

export type FilterContent = zod.infer<typeof FilterContent>;
export const FilterContent = zod.union([InFilterContent, GreaterThanFilterContent, LesserThanFilterContent]);
export type FilterOperator = zod.infer<typeof FilterOperator>;
export const FilterOperator = zod.discriminatedUnion('op', [InFilter, GreaterThanFilter, LesserThanFilter]);
export const Keys = Object.assign({}, FilterKeys, CombinationKeys);

/* ************ *
* Combinations *
Expand All @@ -128,24 +49,21 @@ export type SQON = Clean<Operator>;

/* ===== Convenient Type Guards ===== */
export const isCombination = (operator: Operator): operator is CombinationOperator =>
CombinationOperator.safeParse(operator).success;
matchesSchema(CombinationOperator, operator);

export const isFilter = (operator: Operator): operator is FilterOperator => FilterOperator.safeParse(operator).success;
export const isFilter = (operator: Operator): operator is FilterOperator => matchesSchema(FilterOperator, operator);

export const isArrayFilter = (operator: Operator): operator is ArrayFilter => ArrayFilter.safeParse(operator).success;
export const isArrayFilter = (operator: Operator): operator is ArrayFilter => matchesSchema(ArrayFilter, operator);

export const isScalarFilter = (operator: Operator): operator is ScalarFilter =>
ScalarFilter.safeParse(operator).success;
export const isScalarFilter = (operator: Operator): operator is ScalarFilter => matchesSchema(ScalarFilter, operator);

const arrayFilterKeys: string[] = Object.values(ArrayFilterKeys);
export const isArrayFilterKey = (input: unknown): input is ArrayFilterKey =>
export const isArrayFilterKey = (input: unknown): input is ArrayFilterKeys =>
typeof input === 'string' && arrayFilterKeys.includes(input);

const scalarFilterKeys: string[] = Object.values(ScalarFilterKeys);
export const isScalarFilterKey = (input: unknown): input is ScalarFilterKey =>
export const isScalarFilterKey = (input: unknown): input is ScalarFilterKeys =>
typeof input === 'string' && scalarFilterKeys.includes(input);

export const isArrayFilterValue = (value: unknown): value is ArrayFilterValue =>
ArrayFilterValue.safeParse(value).success;
export const isArrayFilterValue = (value: unknown): value is ArrayFilterValue => matchesSchema(ArrayFilterValue, value);
export const isScalarFilterValue = (value: unknown): value is ScalarFilterValue =>
ScalarFilterValue.safeParse(value).success;
matchesSchema(ScalarFilterValue, value);
113 changes: 113 additions & 0 deletions src/types/sqonFilters.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import { z as zod, type ZodSchema } from 'zod';
import type { Values } from './util';

/* *********** *
* Filter Keys *
* *********** */
export const FilterKeys = {
In: 'in',
GreaterThan: 'gt',
GreaterThanSymbol: '>',
LesserThan: 'lt',
LesserThanSymbol: '<',
} as const;
export type FilterKey = Values<typeof FilterKeys>;

export const InFilterKeys = [FilterKeys.In];
export const GreaterThanFilterKeys = [FilterKeys.GreaterThan, FilterKeys.GreaterThanSymbol];
export const LesserThanFilterKeys = [FilterKeys.LesserThan, FilterKeys.LesserThanSymbol];

export const ArrayFilterKeys = [...InFilterKeys];
export type ArrayFilterKeys = (typeof ArrayFilterKeys)[number];
export const ScalarFilterKeys = [...GreaterThanFilterKeys, ...LesserThanFilterKeys];
export type ScalarFilterKeys = (typeof ScalarFilterKeys)[number];

export const FilterKeySets = [InFilterKeys, GreaterThanFilterKeys, LesserThanFilterKeys];

/* ===== Filter Values ==== */

export type FilterSchema<Op extends string, Content extends ZodSchema> = ReturnType<
typeof zod.object<{ op: zod.ZodLiteral<Op>; content: Content }>
>;
/**
* Utility to create a zod schema for the value of a filter, formatting consistently as:
* `{op, content}`
* @param op A string literal that will define when the
* @param content
* @returns
*/
const defineFilter = <Op extends string, Content extends ZodSchema>(
op: Op,
content: Content,
): FilterSchema<Op, Content> => {
return zod.object({
op: zod.literal(op),
content,
});
};

// The array value wants to be able to accept a single value or an array of values
// Arranger also doesnt care if the values are mixed numbers and strings, that will be sorted out by elasticsearch
// and in practice won't be mixed, so to simlpify type validation we use this nested union structure:
// string | number | (string | number)[]
export const ArrayFilterValue = zod.union([
zod.union([zod.string(), zod.number()]).array(),
zod.string(),
zod.number(),
]);
export type ArrayFilterValue = zod.infer<typeof ArrayFilterValue>;

export const ArrayFilterContent = zod.object({
fieldName: zod.string(),
value: ArrayFilterValue,
});
export type ArrayFilterContent = zod.infer<typeof ArrayFilterContent>;

// Scalar filter value differs from the array filter because it only accepts numbers or arrays of numbers
export const ScalarFilterValue = zod.number();
export type ScalarFilterValue = zod.infer<typeof ScalarFilterValue>;

export const ScalarFilterContent = zod.object({
fieldName: zod.string(),
value: ScalarFilterValue,
});
export type ScalarFilterContent = zod.infer<typeof ScalarFilterContent>;

export const InFilter = defineFilter(FilterKeys.In, ArrayFilterContent);
export type InFilter = zod.infer<typeof InFilter>;

export const ArrayFilter = zod.discriminatedUnion('op', [InFilter]);
export type ArrayFilter = zod.infer<typeof ArrayFilter>;

export const GreaterThanFilter = zod.discriminatedUnion('op', [
defineFilter(FilterKeys.GreaterThan, ScalarFilterContent),
defineFilter(FilterKeys.GreaterThanSymbol, ScalarFilterContent),
]);
export type GreaterThanFilter = zod.infer<typeof GreaterThanFilter>;
export const LesserThanFilter = zod.discriminatedUnion('op', [
defineFilter(FilterKeys.LesserThan, ScalarFilterContent),
defineFilter(FilterKeys.LesserThanSymbol, ScalarFilterContent),
]);
export type LesserThanFilter = zod.infer<typeof LesserThanFilter>;

export const ScalarFilter = zod.discriminatedUnion('op', [...GreaterThanFilter.options, ...LesserThanFilter.options]);
export type ScalarFilter = zod.infer<typeof ScalarFilter>;

export const FilterValue = zod.union([ScalarFilterValue, ArrayFilterValue]);
export type FilterValue = zod.infer<typeof FilterValue>;
export const FilterContent = zod.union([InFilter, ...GreaterThanFilter.options, LesserThanFilter]);
export type FilterContent = zod.infer<typeof FilterContent>;
export const FilterOperator = zod.discriminatedUnion('op', [
InFilter,
...GreaterThanFilter.options,
...LesserThanFilter.options,
]);
export type FilterOperator = zod.infer<typeof FilterOperator>;

export type FilterTypeMap = {
[FilterKeys.In]: InFilter;
[FilterKeys.GreaterThan]: GreaterThanFilter;
[FilterKeys.GreaterThanSymbol]: GreaterThanFilter;
[FilterKeys.LesserThan]: LesserThanFilter;
[FilterKeys.LesserThanSymbol]: LesserThanFilter;
};
4 changes: 2 additions & 2 deletions src/types/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ export type Keys<T> = T extends infer U ? keyof U : never;
* type ModelAsConstValues = Values<typeof modelAsConst>; // 'hello' | 100
* ```
*/
export type Values<T> = T extends infer U ? U[keyof U] : never;
export type Values<T> = T[keyof T];

/**
* Strip out aliases from the TS reported type, to one level.
* This will display type as an object with key: value pairs instead as an alias name.
*/
export type Clean<T> = T extends infer U ? { [K in keyof U]: U[K] } : never;
export type Clean<T> = { [K in keyof T]: T[K] };
2 changes: 1 addition & 1 deletion src/utils/checkMatchingFilter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { FilterOperator, isArrayFilter } from '../types/sqon';
import { FilterOperator } from '../types';
import asArray from './asArray';
import filterDuplicates from './filterDuplicates';

Expand Down
5 changes: 1 addition & 4 deletions src/utils/createFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@ import {
FilterOperator,
FilterTypeMap,
isArrayFilterKey,
isArrayFilterValue,
isScalarFilterKey,
isScalarFilterValue,
} from '../types/sqon';
} from '../types';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should all these be import type { as well? not sure anymore...
after seeing zod runtime stuff + types being used (which I really like, in case it's not clear), it intuitively feels like we'd do well establishing a clearer separation in folder structure, something like types/ vs schemas/ (or zodSchemas/?).

until we have intellisense in the PR reviewing flow (and even after that, if it ever happens), I believe that declarative distinction would reduce cognitive burden by orders of magnitude.

could become a nice pattern for future projects, if it works here

import asArray from './asArray';

export const createFilter = <Key extends FilterKey>(
Expand All @@ -24,5 +23,3 @@ export const createFilter = <Key extends FilterKey>(
throw new TypeError(`Cannot assign the value "${value}" to a filter of type "${op}".`);
}
};

createFilter('a', 'in', ['1']);
37 changes: 37 additions & 0 deletions src/utils/isSameFilter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { FilterKeySets, GreaterThanFilterKeys, type FilterOperator } from '../types';
import filterDuplicates from './filterDuplicates';

/**
* Determine if every value in the values array is found in the options array.
*
* @param options
* @param values
* @returns
*/
function allIncluded<T>(options: T[], values: T[]): boolean {
return values.filter((value) => options.includes(value)).length === values.length;

// let count = 0;
// for (const option of options) {
// count += values.filter((i) => i === option).length;
// if (count >= values.length) {
// return true;
// }
// }
// return false;
}

export function isSameFilter(filterA: FilterOperator, filterB: FilterOperator): boolean {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just realized, is filter a synonym to SQON? if not, what's the difference?

if (filterA.op === filterB.op) {
return true;
}

const operations: string[] = [filterA.op, filterB.op];
for (const keySet of FilterKeySets) {
if (allIncluded(keySet, operations)) {
return true;
}
}

return false;
}
5 changes: 5 additions & 0 deletions src/utils/matchesSchema.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import type { ZodSchema } from 'zod';

export function matchesSchema<T>(schema: ZodSchema<T>, value: unknown): value is T {
return schema.safeParse(value).success;
}
Loading