Improve filtering type awareness#410
Conversation
- this is for making lexemes type aware
- fixing ts typing issues
- generic evaluate override (uses config)
- update associated tests & usage
- add tests for 'in' -- all query flavors
| - "test" | ||
| - "server-test" | ||
| - "standalone-test" | ||
| - "unit-test" |
There was a problem hiding this comment.
These were taking way too long. Breaking up the server, standalone and unit tests.
|
|
||
| if (value && value.length) { | ||
| this.ops.set(safeColumnId, new SingleColumnSyntaxTree(safeColumnId, value)); | ||
| this.ops.set(safeColumnId, new SingleColumnSyntaxTree(value, column)); |
There was a problem hiding this comment.
Now need the id and type information in the syntax tree in order to (1) apply the default field, (2) apply the right relational operator
| return res; | ||
| } | ||
|
|
||
| export type SingleColumnConfig = RequiredPluck<IVisibleColumn, 'id'> & OptionalPluck<IVisibleColumn, 'type'>; |
There was a problem hiding this comment.
Partial<IVisibleColumn> was problematic as id had to be redefined on top, causing potential misalignment issues + exposed the entire column interface for no good reason. Plucking and optionally plucking props from the type instead to create a new tailor-made partial type.
| export const likeDate: IUnboundedLexeme = R.merge({ | ||
| evaluate: relationalEvaluator(([op, exp]) => { | ||
| const normalizedOp = normalizeDate(op, DATE_OPTIONS); | ||
| const normalizedExp = normalizeDate(exp, DATE_OPTIONS); |
There was a problem hiding this comment.
Try to normalize content with the most flexible configuration (DATE_OPTIONS) possible
There was a problem hiding this comment.
Otherwise, would need to know the source field + column configuration for a given evaluate -- not sure how I'd want to expose that atm
| greaterThan, | ||
| lessOrEqual, | ||
| lessThan, | ||
| likeDate, |
There was a problem hiding this comment.
Adding the like operator to global, multi column and single column queries
| config.module.rules.forEach(rule => { | ||
| if (rule.loader) { | ||
| rule.loader = rule.loader.replace('ts-loader', `ts-loader?${JSON.stringify({ transpileOnly: true })}`); | ||
| } |
There was a problem hiding this comment.
This is a bit hackish but it prevents TS from building up a types mapping, which is very costly in terms of memory usage.. while it's good to do so in the actual build, doing so in Cypress provides very little value and just causes a heap out of memory exception after a while.
- fix terminal logic bug in lexer - fix tests impacted by operand/expression rework
…h-table into improve-filter-type-awareness
| return !R.isNil(normalizedOp) && | ||
| !R.isNil(normalizedExp) && | ||
| // IE11 does not support `startsWith` | ||
| normalizedOp.indexOf(normalizedExp) === 0; |
There was a problem hiding this comment.
if we're worried about perf, possibly faster to avoid indexOf:
(
normalizedOp.length === normalizedExp.length ?
normalizedOp : normalizedOp.substr(0, normalizedExp.length)
) === normalizedExp| !R.isNil(exp) && | ||
| !R.isNil(op) && | ||
| (R.type(exp) === 'String' || R.type(op) === 'String') && | ||
| op.toString().indexOf(exp.toString()) !== -1 |
There was a problem hiding this comment.
Reworked the contains logic vs. what we had previously discussed. With operand disappearing and lhs/rhs always handled with expression, it seemed unnatural to have an asymmetric contains operator that would do {a -> 1} contains "1" --> true but would do "1" contains {a -> 1} --> false.
Instead, it now makes sure that both statements are defined and that at least one is a string -- if those requirements are met, it coerces to string if necessary and does the check. The two queries above will now be true.
alexcjohnson
left a comment
There was a problem hiding this comment.
Beautiful. I'm not going to pretend to understand all the TS jiujitsu here, if it works I'll just watch and learn 🔬 And my one perf comment is non-🚫 - speculative anyway. 💃
Follow up to #397
Feature
equaloperator: if both lhs and rhs are numeric, converts and compares; otherwise compare lhs and rhs as-iscontainsoperator: expects a stringify-able lhs and a string rhs, returns true if the stringified lhs contains the rhslikeoperator: expects a date string lhs and rhs, normalizes lhs and rhs, returns true if the normalized lhs starts with the normalized rhscolumn-type based default operator
Text, Any: contains
Numeric: equal
Date: like
Maintenance
rework the
expressionvsoperandlexeme differentiation so that only expressions remain; the old operand is now one of the expression casesrefactor the terminal & validity logic of the various lexicons so they share the code instead of duplicating it