From 25b9a10fdd14585f1b303361b2814e860c6e7031 Mon Sep 17 00:00:00 2001 From: Ghislain B Date: Tue, 2 Jan 2024 17:49:15 -0500 Subject: [PATCH] fix(core): DataView `inlineFilters` should allow ES6 arrow functions (#1304) - the sample code below was not working prior to this PR ```ts dataView = new SlickDataView({ inlineFilters: true }); dataView.setFilter((item, args) => item["percentComplete"] > args.percentCompleteThreshold); ``` --- .../src/core/__tests__/slickDataView.spec.ts | 8 +- packages/common/src/core/slickDataview.ts | 18 +- packages/common/src/core/slickGrid.ts | 12 +- packages/utils/src/__tests__/utils.spec.ts | 194 +++++++++++++++--- packages/utils/src/domUtils.ts | 2 +- packages/utils/src/index.ts | 2 +- packages/utils/src/models/index.ts | 2 + .../interfaces.ts} | 0 .../{types/infer.type.ts => models/types.ts} | 2 + packages/utils/src/types/index.ts | 2 - packages/utils/src/utils.ts | 46 +++++ 11 files changed, 239 insertions(+), 49 deletions(-) create mode 100644 packages/utils/src/models/index.ts rename packages/utils/src/{types/htmlElementPosition.interface.ts => models/interfaces.ts} (100%) rename packages/utils/src/{types/infer.type.ts => models/types.ts} (81%) delete mode 100644 packages/utils/src/types/index.ts diff --git a/packages/common/src/core/__tests__/slickDataView.spec.ts b/packages/common/src/core/__tests__/slickDataView.spec.ts index 3a6336fba..3c985c6ee 100644 --- a/packages/common/src/core/__tests__/slickDataView.spec.ts +++ b/packages/common/src/core/__tests__/slickDataView.spec.ts @@ -1471,9 +1471,7 @@ describe('SlickDatView core file', () => { it('should be able to set a filter as CSP Safe and extra filter arguments and expect items to be filtered', () => { const searchString = 'Ob'; // we'll provide "searchString" as filter args - function myFilter(item, args) { - return item.name.toLowerCase().includes(args.searchString?.toLowerCase()); - } + const myFilter = (item, args) => item.name.toLowerCase().includes(args.searchString?.toLowerCase()); const items = [{ id: 1, name: 'Bob', age: 33 }, { id: 0, name: 'Hobby', age: 44 }, { id: 4, name: 'John', age: 20 }, { id: 3, name: 'Jane', age: 24 }]; dv = new SlickDataView({ inlineFilters: true, useCSPSafeFilter: true }); @@ -1568,7 +1566,7 @@ describe('SlickDatView core file', () => { dv.refresh(); // change filter without changing pagination & expect pageNum to be recalculated - dv.setFilter(function (item) { return item.id >= 10 }); + dv.setFilter((item) => item.id >= 10); expect(onPagingInfoSpy).toHaveBeenCalledWith({ dataView: dv, pageNum: 0, pageSize: 2, totalPages: 1, totalRows: 1 }, null, dv); expect(onRowCountChangeSpy).toHaveBeenCalledWith({ dataView: dv, previous: 2, current: 1, itemCount: 5, callingOnRowsChanged: true }, null, dv); expect(onRowsChangeSpy).toHaveBeenCalledWith({ dataView: dv, rows: [0, 1], itemCount: 5, calledOnRowCountChanged: true }, null, dv); @@ -1622,7 +1620,7 @@ describe('SlickDatView core file', () => { // change filter without changing pagination will result in 2 changes but only 1 defined as changed because we ignore diffs from 0-1 dv.setRefreshHints({ ignoreDiffsBefore: 1, ignoreDiffsAfter: 3 }); items[0].id = 8; - dv.setFilter(function (item) { return item.id >= 0 || item.name.includes('a') }); + dv.setFilter((item) => item.id >= 0 || item.name.includes('a')); expect(onPagingInfoSpy).toHaveBeenCalledWith({ dataView: dv, pageNum: 0, pageSize: 2, totalPages: 3, totalRows: 5 }, null, dv); expect(onRowCountChangeSpy).toHaveBeenCalledWith({ dataView: dv, previous: 2, current: 1, itemCount: 5, callingOnRowsChanged: true }, null, dv); expect(onRowsChangeSpy).toHaveBeenCalledWith({ dataView: dv, rows: [1], itemCount: 5, calledOnRowCountChanged: true }, null, dv); diff --git a/packages/common/src/core/slickDataview.ts b/packages/common/src/core/slickDataview.ts index 3b2313b42..c90c62055 100644 --- a/packages/common/src/core/slickDataview.ts +++ b/packages/common/src/core/slickDataview.ts @@ -1,6 +1,6 @@ /* eslint-disable no-new-func */ /* eslint-disable no-bitwise */ -import { extend, isDefined } from '@slickgrid-universal/utils'; +import { type AnyFunction, extend, getFunctionDetails, isDefined } from '@slickgrid-universal/utils'; import { SlickGroupItemMetadataProvider } from '../extensions/slickGroupItemMetadataProvider'; import type { @@ -52,7 +52,6 @@ export type FilterWithCspCachingFn = (item: T[], args: any, filterCache: any[ export type DataIdType = number | string; export type SlickDataItem = SlickNonDataItem | SlickGroup | SlickGroupTotals | any; export type GroupGetterFn = (val: any) => string | number; -export type AnyFunction = (...args: any[]) => any; /** * A sample Model implementation. @@ -1007,17 +1006,6 @@ export class SlickDataView implements CustomD return groupedRows; } - protected getFunctionInfo(fn: AnyFunction) { - const fnStr = fn.toString(); - const usingEs5 = fnStr.indexOf('function') >= 0; // with ES6, the word function is not present - const fnRegex = usingEs5 ? /^function[^(]*\(([^)]*)\)\s*{([\s\S]*)}$/ : /^[^(]*\(([^)]*)\)\s*{([\s\S]*)}$/; - const matches = fn.toString().match(fnRegex) || []; - return { - params: matches[1].split(','), - body: matches[2] - }; - } - protected compileAccumulatorLoopCSPSafe(aggregator: Aggregator) { if (aggregator.accumulate) { return function (items: any[]) { @@ -1056,7 +1044,7 @@ export class SlickDataView implements CustomD if (stopRunningIfCSPSafeIsActive) { return null; } - const filterInfo = this.getFunctionInfo(this.filter as FilterFn); + const filterInfo = getFunctionDetails(this.filter as FilterFn); const filterPath1 = '{ continue _coreloop; }$1'; const filterPath2 = '{ _retval[_idx++] = $item$; continue _coreloop; }$1'; @@ -1098,7 +1086,7 @@ export class SlickDataView implements CustomD return null; } - const filterInfo = this.getFunctionInfo(this.filter as FilterFn); + const filterInfo = getFunctionDetails(this.filter as FilterFn); const filterPath1 = '{ continue _coreloop; }$1'; const filterPath2 = '{ _cache[_i] = true;_retval[_idx++] = $item$; continue _coreloop; }$1'; diff --git a/packages/common/src/core/slickGrid.ts b/packages/common/src/core/slickGrid.ts index 1c04a4c7b..4fdca4f07 100644 --- a/packages/common/src/core/slickGrid.ts +++ b/packages/common/src/core/slickGrid.ts @@ -2,7 +2,17 @@ import Sortable, { SortableEvent } from 'sortablejs'; import DOMPurify from 'dompurify'; import { BindingEventService } from '@slickgrid-universal/binding'; -import { createDomElement, emptyElement, extend, getInnerSize, getOffset, insertAfterElement, isDefined, isPrimitiveOrHTML, classNameToList } from '@slickgrid-universal/utils'; +import { + classNameToList, + createDomElement, + emptyElement, + extend, + getInnerSize, + getOffset, + insertAfterElement, + isDefined, + isPrimitiveOrHTML, +} from '@slickgrid-universal/utils'; import { type BasePubSub, diff --git a/packages/utils/src/__tests__/utils.spec.ts b/packages/utils/src/__tests__/utils.spec.ts index 1ceaf627e..2402d9afa 100644 --- a/packages/utils/src/__tests__/utils.spec.ts +++ b/packages/utils/src/__tests__/utils.spec.ts @@ -7,6 +7,7 @@ import { deepCopy, deepMerge, emptyObject, + getFunctionDetails, hasData, isDefined, isEmptyObject, @@ -27,8 +28,12 @@ import { uniqueObjectArray, } from '../utils'; +function removeExtraSpaces(text: string) { + return `${text}`.replace(/\s+/g, ' ').replace(/\r\n/g, '').trim(); +} + describe('Service/Utilies', () => { - describe('addToArrayWhenNotExists', () => { + describe('addToArrayWhenNotExists() method', () => { it('should add an item to the array when input item has an "id" and is not in the array', () => { const array = [{ id: 1, firstName: 'John' }]; addToArrayWhenNotExists(array, { id: 2, firstName: 'Jane' }); @@ -60,7 +65,7 @@ describe('Service/Utilies', () => { }); }); - describe('addWhiteSpaces method', () => { + describe('addWhiteSpaces() method', () => { it('should return the an empty string when argument provided is lower or equal to 0', () => { expect(addWhiteSpaces(-2)).toBe(''); expect(addWhiteSpaces(0)).toBe(''); @@ -75,7 +80,7 @@ describe('Service/Utilies', () => { }); }); - describe('arrayRemoveItemByIndex method', () => { + describe('arrayRemoveItemByIndex() method', () => { it('should remove an item from the array', () => { const input = [{ field: 'field1', name: 'Field 1' }, { field: 'field2', name: 'Field 2' }, { field: 'field3', name: 'Field 3' }]; const expected = [{ field: 'field1', name: 'Field 1' }, { field: 'field3', name: 'Field 3' }]; @@ -85,7 +90,148 @@ describe('Service/Utilies', () => { }); }); - describe('hasData method', () => { + describe('getFunctionDetails() method', () => { + test('regular function without arguments and with defined body', () => { + function fn() { + return true; + } + const result = getFunctionDetails(fn); + + expect(result.params).toEqual([]); + expect(result.isAsync).toBe(false); + expect(result.body).toInclude('return true;'); + }); + + test('regular function with defined arguments and body', () => { + function fn(input: string, args: any) { + if (input.length > 1) { + return true; + } + return input['age'].toString().includes(args.searchString) + } + const result = getFunctionDetails(fn); + + expect(result.params).toEqual(['input', 'args']); + expect(result.isAsync).toBe(false); + expect(removeExtraSpaces(result.body)).toInclude(removeExtraSpaces(` + if (input.length > 1) { + return true; + } + return input['age'].toString().includes(args.searchString) + `)); + }); + + test('regular async function with arguments', () => { + async function fn(input: string, args: any) { + if (input.length > 1) { + return true; + } + return input['age'].toString().includes(args.searchString) + } + const result = getFunctionDetails(fn); + + expect(result.params).toEqual(['input', 'args']); + expect(result.isAsync).toBe(true); + expect(removeExtraSpaces(result.body)).toInclude(removeExtraSpaces(` + if (input.length > 1) { + return true; + } + return input['age'].toString().includes(args.searchString) + `)); + }); + + test('nested ES6 arrow async functions with arguments', () => { + const fn = async (x) => async (y) => async (z) => z(x)(y); + const result = getFunctionDetails(fn); + + expect(result.params).toEqual(['x']); + expect(result.isAsync).toBe(true); + expect(removeExtraSpaces(result.body)).toInclude(removeExtraSpaces(` + return async (y) => async (z) => z(x)(y) + `)); + }); + + test('ES6 arrow function returning object in brackets', () => { + const fn = () => ({ status: 200, body: 'hello world' }); + const result = getFunctionDetails(fn); + + expect(result.params).toEqual([]); + expect(result.isAsync).toBe(false); + expect(removeExtraSpaces(result.body)).toInclude(removeExtraSpaces(` + return { status: 200, body: 'hello world' } + `)); + }); + + test('ES6 arrow function returning object in brackets minified without spaces', () => { + const fn = `()=>({status: 200, body: 'hello world'})`; + const result = getFunctionDetails(fn as any); + + expect(result.params).toEqual([]); + expect(result.isAsync).toBe(false); + expect(removeExtraSpaces(result.body)).toInclude(removeExtraSpaces(` + return {status: 200, body: 'hello world'} + `)); + }); + + test('ES6 arrow async function and spread arguments', () => { + const fn = (async (a: number, b: number, ...rest: number[]) => { + let sum = a + b; + for (let n of rest) { + sum += n; + } + return sum; + }); + const result = getFunctionDetails(fn); + + expect(result.params).toEqual(['a', 'b', '...rest']); + expect(result.isAsync).toBe(true); + expect(removeExtraSpaces(result.body)).toInclude(removeExtraSpaces(` + let sum = a + b; + for (let n of rest) { + sum += n; + } + return sum; + `)); + }); + + test('one liner ES6 arrow function without arguments', () => { + const fn = () => true; + const result = getFunctionDetails(fn); + + expect(result.params).toEqual([]); + expect(result.body).toInclude('return true'); + }); + + test('one liner ES6 arrow function with arguments', () => { + const fn = (input) => input ? input.lenght > 1 : false; + const result = getFunctionDetails(fn); + + expect(result.params).toEqual(['input']); + expect(result.isAsync).toBe(false); + expect(result.body).toInclude('return input ? input.lenght > 1 : false'); + }); + + test('ES6 arrow function written in TypeScript', () => { + const fn = (input: string, args: any) => { + if (input.length > 1) { + return true; + } + return input['age'].toString().includes(args.searchString); + } + const result = getFunctionDetails(fn); + + expect(result.params).toEqual(['input', 'args']); + expect(result.isAsync).toBe(false); + expect(removeExtraSpaces(result.body)).toInclude(removeExtraSpaces(` + if (input.length > 1) { + return true; + } + return input['age'].toString().includes(args.searchString); + `)); + }); + }); + + describe('hasData() method', () => { it('should return True when input has test, or is a boolean (true or false) or if it is an object', () => { expect(hasData('test')).toBe(true); expect(hasData(true)).toBe(true); @@ -99,7 +245,7 @@ describe('Service/Utilies', () => { }); }); - describe('isDefined method', () => { + describe('isDefined() method', () => { it('should be truthy when comparing against any defined variable', () => { const result1 = isDefined({ firstName: 'John', lastName: 'Doe' }); const result2 = isDefined('hello'); @@ -119,7 +265,7 @@ describe('Service/Utilies', () => { }); }); - describe('isEmptyObject method', () => { + describe('isEmptyObject() method', () => { it('should return True when comparing against an object that has properties', () => { const result = isEmptyObject({ firstName: 'John', lastName: 'Doe' }); expect(result).toBeFalse(); @@ -136,7 +282,7 @@ describe('Service/Utilies', () => { }); }); - describe('isObject method', () => { + describe('isObject() method', () => { it('should return false when input is undefined', () => { expect(isObject(undefined)).toBeFalse(); }); @@ -170,7 +316,7 @@ describe('Service/Utilies', () => { }); }); - describe('isObjectEmpty method', () => { + describe('isObjectEmpty() method', () => { it('should return True when input is undefined', () => { const result = isObjectEmpty(undefined); expect(result).toBeTrue(); @@ -192,7 +338,7 @@ describe('Service/Utilies', () => { }); }); - describe('isPrimitiveValue method', () => { + describe('isPrimitiveValue() method', () => { it('should return True when input is undefined', () => { const result = isPrimitiveValue(undefined); expect(result).toBeTrue(); @@ -224,7 +370,7 @@ describe('Service/Utilies', () => { }); }); - describe('isPrimitiveOrHTML method', () => { + describe('isPrimitiveOrHTML() method', () => { it('should return True when input is undefined', () => { const result = isPrimitiveOrHTML(undefined); expect(result).toBeTrue(); @@ -268,7 +414,7 @@ describe('Service/Utilies', () => { }); }); - describe('isNumber method', () => { + describe('isNumber() method', () => { it('should return True when comparing a number from a number/string variable when strict mode is disable', () => { const result1 = isNumber(22); const result2 = isNumber('33'); @@ -295,7 +441,7 @@ describe('Service/Utilies', () => { }); }); - describe('deepCopy method', () => { + describe('deepCopy() method', () => { it('should return original input when it is not an object neither an array', () => { const msg = 'hello world'; const age = 20; @@ -328,7 +474,7 @@ describe('Service/Utilies', () => { }); }); - describe('deepMerge method', () => { + describe('deepMerge() method', () => { it('should return undefined when both inputs are undefined', () => { const obj1 = undefined; const obj2 = null; @@ -417,14 +563,14 @@ describe('Service/Utilies', () => { }); }); - describe('emptyObject method', () => { + describe('emptyObject() method', () => { it('should empty all object properties', () => { const obj = { firstName: 'John', address: { zip: 123456, streetNumber: '123 Belleville Blvd' } }; expect(emptyObject(obj)).toEqual({}); }); }); - describe('parseBoolean method', () => { + describe('parseBoolean() method', () => { it('should return false when input value is not parseable to a boolean', () => { const output = parseBoolean('abc'); expect(output).toBe(false); @@ -464,7 +610,7 @@ describe('Service/Utilies', () => { }); }); - describe('removeAccentFromText method', () => { + describe('removeAccentFromText() method', () => { it('should return a normalized string without accent', () => { const input1 = 'José'; const input2 = 'Chêvre'; @@ -486,7 +632,7 @@ describe('Service/Utilies', () => { }); }); - describe('setDeepValue method', () => { + describe('setDeepValue() method', () => { let obj: any = {}; beforeEach(() => { obj = { id: 1, user: { firstName: 'John', lastName: 'Doe', age: null, address: { number: 123, street: 'Broadway' } } }; @@ -546,7 +692,7 @@ describe('Service/Utilies', () => { }); }); - describe('titleCase method', () => { + describe('titleCase() method', () => { const sentence = 'the quick brown fox'; it('should return empty string when input is empty', () => { @@ -572,7 +718,7 @@ describe('Service/Utilies', () => { }); }); - describe('toCamelCase method', () => { + describe('toCamelCase() method', () => { const sentence = 'the quick brown fox'; it('should return empty string when input is empty', () => { @@ -597,7 +743,7 @@ describe('Service/Utilies', () => { }); }); - describe('toKebabCase method', () => { + describe('toKebabCase() method', () => { const sentence = 'the quick brown fox'; it('should return empty string when input is empty', () => { @@ -622,7 +768,7 @@ describe('Service/Utilies', () => { }); }); - describe('toSentenceCase method', () => { + describe('toSentenceCase() method', () => { const camelCaseSentence = 'theQuickBrownFox'; const kebabCaseSentence = 'the-quick-brown-fox'; @@ -653,7 +799,7 @@ describe('Service/Utilies', () => { }); }); - describe('toSnakeCase method', () => { + describe('toSnakeCase() method', () => { const sentence = 'the quick brown fox'; it('should return empty string when input is empty', () => { @@ -678,7 +824,7 @@ describe('Service/Utilies', () => { }); }); - describe('uniqueArray method', () => { + describe('uniqueArray() method', () => { it('should return original value when input is not an array', () => { const output1 = uniqueArray(null as any); const output2 = uniqueArray(undefined as any); @@ -703,7 +849,7 @@ describe('Service/Utilies', () => { }); }); - describe('uniqueObjectArray method', () => { + describe('uniqueObjectArray() method', () => { it('should return original value when input is not an array', () => { const output1 = uniqueObjectArray(null as any); const output2 = uniqueObjectArray(undefined as any); diff --git a/packages/utils/src/domUtils.ts b/packages/utils/src/domUtils.ts index 0fa7a18a7..7a46fef4a 100644 --- a/packages/utils/src/domUtils.ts +++ b/packages/utils/src/domUtils.ts @@ -1,4 +1,4 @@ -import type { HtmlElementPosition, InferDOMType } from './types/index'; +import type { HtmlElementPosition, InferDOMType } from './models/index'; /** calculate available space for each side of the DOM element */ export function calculateAvailableSpace(element: HTMLElement): { top: number; bottom: number; left: number; right: number; } { diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 59c437ec7..6e1f01574 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -1,5 +1,5 @@ export * from './domUtils'; export * from './nodeExtend'; export * from './stripTagsUtil'; -export * from './types'; +export * from './models'; export * from './utils'; \ No newline at end of file diff --git a/packages/utils/src/models/index.ts b/packages/utils/src/models/index.ts new file mode 100644 index 000000000..90f11473c --- /dev/null +++ b/packages/utils/src/models/index.ts @@ -0,0 +1,2 @@ +export * from './interfaces'; +export * from './types'; diff --git a/packages/utils/src/types/htmlElementPosition.interface.ts b/packages/utils/src/models/interfaces.ts similarity index 100% rename from packages/utils/src/types/htmlElementPosition.interface.ts rename to packages/utils/src/models/interfaces.ts diff --git a/packages/utils/src/types/infer.type.ts b/packages/utils/src/models/types.ts similarity index 81% rename from packages/utils/src/types/infer.type.ts rename to packages/utils/src/models/types.ts index da5426369..050606d3a 100644 --- a/packages/utils/src/types/infer.type.ts +++ b/packages/utils/src/models/types.ts @@ -4,3 +4,5 @@ export type InferDOMType = T extends CSSStyleDeclaration ? Partial : T extends infer R ? R : any; /* eslint-enable @typescript-eslint/indent */ + +export type AnyFunction = (...args: any[]) => any; \ No newline at end of file diff --git a/packages/utils/src/types/index.ts b/packages/utils/src/types/index.ts deleted file mode 100644 index 36954bc65..000000000 --- a/packages/utils/src/types/index.ts +++ /dev/null @@ -1,2 +0,0 @@ -export * from './htmlElementPosition.interface'; -export * from './infer.type'; diff --git a/packages/utils/src/utils.ts b/packages/utils/src/utils.ts index d6c23f5d9..b7eebdf68 100644 --- a/packages/utils/src/utils.ts +++ b/packages/utils/src/utils.ts @@ -1,3 +1,5 @@ +import { AnyFunction } from './models/types'; + /** * Add an item to an array only when the item does not exists, when the item is an object we will be using their "id" to compare * @param inputArray @@ -151,6 +153,50 @@ export function emptyObject(obj: any) { return obj; } +/** + * Get the function details (param & body) of a function. + * It supports regular function and also ES6 arrow functions + * @param {Function} fn - function to analyze + * @param {Boolean} [addReturn] - when using ES6 function as single liner, we could add the missing `return ...` + * @returns + */ +export function getFunctionDetails(fn: AnyFunction, addReturn = true) { + let isAsyncFn = false; + + const getFunctionBody = (func: AnyFunction) => { + const fnStr = func.toString(); + isAsyncFn = fnStr.includes('async '); + + // when fn is one liner arrow fn returning an object in brackets e.g. `() => ({ hello: 'world' })` + if ((fnStr.replaceAll(' ', '').includes('=>({'))) { + const matches = fnStr.match(/(({.*}))/g) || []; + return matches.length >= 1 ? `return ${matches[0]!.trimStart()}` : fnStr; + } + const isOneLinerArrowFn = (!fnStr.includes('{') && fnStr.includes('=>')); + const body = fnStr.substring( + (fnStr.indexOf('{') + 1) || (fnStr.indexOf('=>') + 2), + fnStr.includes('}') ? fnStr.lastIndexOf('}') : fnStr.length + ); + if (addReturn && isOneLinerArrowFn && !body.startsWith('return')) { + return 'return ' + body.trimStart(); // add the `return ...` to the body for ES6 arrow fn + } + return body; + }; + + const getFunctionParams = (func: AnyFunction): string[] => { + const STRIP_COMMENTS = /(\/\/.*$)|(\/\*[\s\S]*?\*\/)|(\s*=[^,\)]*(('(?:\\'|[^'\r\n])*')|("(?:\\"|[^"\r\n])*"))|(\s*=[^,\)]*))/mg; + const ARG_NAMES = /([^\s,]+)/g; + const fnStr = func.toString().replace(STRIP_COMMENTS, ''); + return fnStr.slice(fnStr.indexOf('(') + 1, fnStr.indexOf(')')).match(ARG_NAMES) ?? []; + }; + + return { + params: getFunctionParams(fn), + body: getFunctionBody(fn), + isAsync: isAsyncFn, + }; +} + /** * Check if an object is empty * @param obj - input object