From ca6cc12fa0feb00744f3b1144078f98dd7992fd8 Mon Sep 17 00:00:00 2001 From: ghiscoding Date: Thu, 27 Aug 2026 19:35:39 -0400 Subject: [PATCH] refactor(common): make SlickDataView filtering CSP-safe --- demos/vanilla/src/examples/example03.ts | 3 - docs/developer-guides/csp-compliance.md | 21 +- .../docs/developer-guides/csp-compliance.md | 25 +- .../docs/developer-guides/csp-compliance.md | 25 +- .../docs/developer-guides/csp-compliance.md | 34 +-- .../docs/developer-guides/csp-compliance.md | 32 +-- package.json | 3 +- .../src/core/__tests__/slickDataView.spec.ts | 50 +++- packages/common/src/core/slickDataView.ts | 218 +++--------------- test/benchmarks/README.md | 23 ++ test/benchmarks/slickDataView.bench.ts | 108 +++++++++ test/vitest.benchmark.config.mts | 13 ++ 12 files changed, 228 insertions(+), 327 deletions(-) create mode 100644 test/benchmarks/README.md create mode 100644 test/benchmarks/slickDataView.bench.ts create mode 100644 test/vitest.benchmark.config.mts diff --git a/demos/vanilla/src/examples/example03.ts b/demos/vanilla/src/examples/example03.ts index 5a774f282b..439be5581e 100644 --- a/demos/vanilla/src/examples/example03.ts +++ b/demos/vanilla/src/examples/example03.ts @@ -346,9 +346,6 @@ export default class Example03 { autoResize: { container: '.demo-container', }, - dataView: { - useCSPSafeFilter: true, - }, enableFormattedDataCache: false, // enable it when you have a large dataset (e.g. we'll enable it when loading over 10K) headerMenu: { hideFreezeColumnsCommand: false, diff --git a/docs/developer-guides/csp-compliance.md b/docs/developer-guides/csp-compliance.md index 7f3a687f31..800e2a77c7 100644 --- a/docs/developer-guides/csp-compliance.md +++ b/docs/developer-guides/csp-compliance.md @@ -14,8 +14,6 @@ this.gridOptions = { > **Note** If you're wondering about the `ADD_ATTR: ['level']`, well the "level" is a custom attribute used by SlickGrid Grouping/Draggable Grouping to track the grouping level depth and it must be kept. -> **Note** the DataView is not CSP safe by default, it is opt-in via the `useCSPSafeFilter` option. - ```typescript import DOMPurify from 'dompurify'; import { Slicker, SlickVanillaGridBundle } from '@slickgrid-universal/vanilla-bundle'; @@ -36,24 +34,9 @@ with this code in place, we can use the following CSP meta tag (which is what we ``` #### DataView -Since we use the DataView, you will also need to enable a new `useCSPSafeFilter` flag to be CSP safe as the name suggest. This option is opt-in because it has a slight performance impact when enabling this option (it shouldn't be noticeable unless you use a very large dataset). - -```typescript -import DOMPurify from 'dompurify'; -import { Slicker, SlickVanillaGridBundle } from '@slickgrid-universal/vanilla-bundle'; - -// DOM Purify is already configured in Slickgrid-Universal with the configuration shown below -this.gridOptions = { - // you could also optionally use the sanitizerOptions instead - // sanitizerOptions: { RETURN_TRUSTED_TYPE: true } - dataView: { - useCSPSafeFilter: true - }, -} -this.sgb = new Slicker.GridBundle(gridContainerElm, this.columns, this.gridOptions, this.dataset); -``` +DataView filtering is CSP-safe by default and does not use runtime code generation. No DataView option is required. The deprecated `inlineFilters` and `useCSPSafeFilter` options remain accepted for backward compatibility but are ignored. ### Custom Formatter using native HTML We now also allow passing native HTML Element as a Custom Formatter instead of HTML string in order to avoid the use of `innerHTML` and stay CSP safe. We also have a new grid option named `enableHtmlRendering`, which is enabled by default and is allowing the use of `innerHTML` in the library (by Formatters and others), however when disabled it will totally restrict the use of `innerHTML` which will help to stay CSP safe. -You can take a look at the original SlickGrid library with this new [Filtered DataView with HTML Formatter - CSP Header (Content Security Policy)](https://6pac.github.io/SlickGrid/examples/example4-model-html-formatters.html) example which uses this new approach. There was no new Example created in Slickgrid-Universal specifically for this but the approach is the same. \ No newline at end of file +You can take a look at the original SlickGrid library with this new [Filtered DataView with HTML Formatter - CSP Header (Content Security Policy)](https://6pac.github.io/SlickGrid/examples/example4-model-html-formatters.html) example which uses this new approach. There was no new Example created in Slickgrid-Universal specifically for this but the approach is the same. diff --git a/frameworks/angular-slickgrid/docs/developer-guides/csp-compliance.md b/frameworks/angular-slickgrid/docs/developer-guides/csp-compliance.md index 51e26b9518..595244e700 100644 --- a/frameworks/angular-slickgrid/docs/developer-guides/csp-compliance.md +++ b/frameworks/angular-slickgrid/docs/developer-guides/csp-compliance.md @@ -14,8 +14,6 @@ this.gridOptions = { > **Note** If you're wondering about the `ADD_ATTR: ['level']`, well the "level" is a custom attribute used by SlickGrid Grouping/Draggable Grouping to track the grouping level depth and it must be kept. -> **Note** the DataView is not CSP safe by default, it is opt-in via the `useCSPSafeFilter` option. - ```typescript import DOMPurify from 'dompurify'; import { Slicker, SlickVanillaGridBundle } from '@slickgrid-universal/vanilla-bundle'; @@ -35,28 +33,7 @@ with this code in place, we can use the following CSP meta tag (which is what we ``` #### DataView -Since we use the DataView, you will also need to enable a new `useCSPSafeFilter` flag to be CSP safe as the name suggest. This option is opt-in because it has a slight performance impact when enabling this option (it shouldn't be noticeable unless you use a very large dataset). - -```typescript -import DOMPurify from 'dompurify'; -import { GridOption } from 'angular-slickgrid'; - -export class Example1 { - gridOptions: GridOption; - - prepareGrid() { - // ... - - this.gridOptions = { - // you could also optionally use the sanitizerOptions instead - // sanitizerOptions: { RETURN_TRUSTED_TYPE: true } - dataView: { - useCSPSafeFilter: true - }, - } - } -} -``` +DataView filtering is CSP-safe by default and does not use runtime code generation. No DataView option is required. The deprecated `inlineFilters` and `useCSPSafeFilter` options remain accepted for backward compatibility but are ignored. ### Custom Formatter using native HTML We now also allow passing native HTML Element as a Custom Formatter instead of HTML string in order to avoid the use of `innerHTML` and stay CSP safe. We also have a new grid option named `enableHtmlRendering`, which is enabled by default and is allowing the use of `innerHTML` in the library (by Formatters and others), however when disabled it will totally restrict the use of `innerHTML` which will help to stay CSP safe. diff --git a/frameworks/aurelia-slickgrid/docs/developer-guides/csp-compliance.md b/frameworks/aurelia-slickgrid/docs/developer-guides/csp-compliance.md index 219fd721b4..595244e700 100644 --- a/frameworks/aurelia-slickgrid/docs/developer-guides/csp-compliance.md +++ b/frameworks/aurelia-slickgrid/docs/developer-guides/csp-compliance.md @@ -14,8 +14,6 @@ this.gridOptions = { > **Note** If you're wondering about the `ADD_ATTR: ['level']`, well the "level" is a custom attribute used by SlickGrid Grouping/Draggable Grouping to track the grouping level depth and it must be kept. -> **Note** the DataView is not CSP safe by default, it is opt-in via the `useCSPSafeFilter` option. - ```typescript import DOMPurify from 'dompurify'; import { Slicker, SlickVanillaGridBundle } from '@slickgrid-universal/vanilla-bundle'; @@ -35,28 +33,7 @@ with this code in place, we can use the following CSP meta tag (which is what we ``` #### DataView -Since we use the DataView, you will also need to enable a new `useCSPSafeFilter` flag to be CSP safe as the name suggest. This option is opt-in because it has a slight performance impact when enabling this option (it shouldn't be noticeable unless you use a very large dataset). - -```typescript -import DOMPurify from 'dompurify'; -import { GridOption } from 'aurelia-slickgrid'; - -export class Example1 { - gridOptions: GridOption; - - prepareGrid() { - // ... - - this.gridOptions = { - // you could also optionally use the sanitizerOptions instead - // sanitizerOptions: { RETURN_TRUSTED_TYPE: true } - dataView: { - useCSPSafeFilter: true - }, - } - } -} -``` +DataView filtering is CSP-safe by default and does not use runtime code generation. No DataView option is required. The deprecated `inlineFilters` and `useCSPSafeFilter` options remain accepted for backward compatibility but are ignored. ### Custom Formatter using native HTML We now also allow passing native HTML Element as a Custom Formatter instead of HTML string in order to avoid the use of `innerHTML` and stay CSP safe. We also have a new grid option named `enableHtmlRendering`, which is enabled by default and is allowing the use of `innerHTML` in the library (by Formatters and others), however when disabled it will totally restrict the use of `innerHTML` which will help to stay CSP safe. diff --git a/frameworks/slickgrid-react/docs/developer-guides/csp-compliance.md b/frameworks/slickgrid-react/docs/developer-guides/csp-compliance.md index 4ec7854aa3..5c92922cba 100644 --- a/frameworks/slickgrid-react/docs/developer-guides/csp-compliance.md +++ b/frameworks/slickgrid-react/docs/developer-guides/csp-compliance.md @@ -14,8 +14,6 @@ const gridOptions = { > **Note** If you're wondering about the `ADD_ATTR: ['level']`, well the "level" is a custom attribute used by SlickGrid Grouping/Draggable Grouping to track the grouping level depth and it must be kept. -> **Note** the DataView is not CSP safe by default, it is opt-in via the `useCSPSafeFilter` option. - ```typescript import DOMPurify from 'dompurify'; import { Slicker, SlickVanillaGridBundle } from '@slickgrid-universal/vanilla-bundle'; @@ -34,37 +32,7 @@ with this code in place, we can use the following CSP meta tag (which is what we ``` #### DataView -Since we use the DataView, you will also need to enable a new `useCSPSafeFilter` flag to be CSP safe as the name suggest. This option is opt-in because it has a slight performance impact when enabling this option (it shouldn't be noticeable unless you use a very large dataset). - -```typescript -import DOMPurify from 'dompurify'; -import { GridOption } from 'slickgrid-react'; - -const Example: React.FC = () => { - const [dataset, setDataset] = useState([]); - const [columns, setColumns] = useState([]); - const [options, setOptions] = useState(undefined); - const reactGridRef = useRef(null); - -useEffect(() => defineGrid(), []); - - function reactGridReady(reactGrid: SlickgridReactInstance) { - reactGridRef.current = reactGrid; - } - - function defineGrid() { - // ... - - setOptions({ - // you could also optionally use the sanitizerOptions instead - // sanitizerOptions: { RETURN_TRUSTED_TYPE: true } - dataView: { - useCSPSafeFilter: true - }, - }); - } -} -``` +DataView filtering is CSP-safe by default and does not use runtime code generation. No DataView option is required. The deprecated `inlineFilters` and `useCSPSafeFilter` options remain accepted for backward compatibility but are ignored. ### Custom Formatter using native HTML We now also allow passing native HTML Element as a Custom Formatter instead of HTML string in order to avoid the use of `innerHTML` and stay CSP safe. We also have a new grid option named `enableHtmlRendering`, which is enabled by default and is allowing the use of `innerHTML` in the library (by Formatters and others), however when disabled it will totally restrict the use of `innerHTML` which will help to stay CSP safe. diff --git a/frameworks/slickgrid-vue/docs/developer-guides/csp-compliance.md b/frameworks/slickgrid-vue/docs/developer-guides/csp-compliance.md index ad0edc2a07..ed0b9ea741 100644 --- a/frameworks/slickgrid-vue/docs/developer-guides/csp-compliance.md +++ b/frameworks/slickgrid-vue/docs/developer-guides/csp-compliance.md @@ -14,8 +14,6 @@ this.gridOptions = { > **Note** If you're wondering about the `ADD_ATTR: ['level']`, well the "level" is a custom attribute used by SlickGrid Grouping/Draggable Grouping to track the grouping level depth and it must be kept. -> **Note** the DataView is not CSP safe by default, it is opt-in via the `useCSPSafeFilter` option. - ```typescript import DOMPurify from 'dompurify'; import { Slicker, SlickVanillaGridBundle } from '@slickgrid-universal/vanilla-bundle'; @@ -37,35 +35,7 @@ with this code in place, we can use the following CSP meta tag (which is what we ``` #### DataView -Since we use the DataView, you will also need to enable a new `useCSPSafeFilter` flag to be CSP safe as the name suggest. This option is opt-in because it has a slight performance impact when enabling this option (it shouldn't be noticeable unless you use a very large dataset). - -```typescript - -``` +DataView filtering is CSP-safe by default and does not use runtime code generation. No DataView option is required. The deprecated `inlineFilters` and `useCSPSafeFilter` options remain accepted for backward compatibility but are ignored. ### Custom Formatter using native HTML We now also allow passing native HTML Element as a Custom Formatter instead of HTML string in order to avoid the use of `innerHTML` and stay CSP safe. We also have a new grid option named `enableHtmlRendering`, which is enabled by default and is allowing the use of `innerHTML` in the library (by Formatters and others), however when disabled it will totally restrict the use of `innerHTML` which will help to stay CSP safe. diff --git a/package.json b/package.json index 64e497310f..62c7e9b97f 100644 --- a/package.json +++ b/package.json @@ -32,6 +32,7 @@ "build:universal": "tsc --build ./tsconfig.packages.json && pnpm sass:bundle", "build:frameworks": "pnpm -r --stream --filter=\"./{demos,frameworks,frameworks-plugins}/**\" run build", "build:watch": "tsc --build ./tsconfig.packages.json --watch", + "bench:data-view": "vitest bench --config ./test/vitest.benchmark.config.mts --run ./test/benchmarks/slickDataView.bench.ts", "angular:watch": "pnpm -r --parallel run angular:dev", "aurelia:watch": "pnpm -r --parallel run aurelia:dev", "react:watch": "pnpm -r --parallel run react:dev", @@ -148,4 +149,4 @@ "type": "ko_fi", "url": "https://ko-fi.com/ghiscoding" } -} \ No newline at end of file +} diff --git a/packages/common/src/core/__tests__/slickDataView.spec.ts b/packages/common/src/core/__tests__/slickDataView.spec.ts index 7369d21885..80124d4927 100644 --- a/packages/common/src/core/__tests__/slickDataView.spec.ts +++ b/packages/common/src/core/__tests__/slickDataView.spec.ts @@ -1734,7 +1734,7 @@ describe('SlickDatView core file', () => { expect(refreshSpy).toHaveBeenCalled(); }); - it('should be able to set a filter with CSP Safe approach and expect items to be filtered', () => { + it('should keep the deprecated CSP-safe option backward compatible', () => { const items = [ { id: 1, name: 'Bob', age: 33 }, { id: 4, name: 'John', age: 20 }, @@ -1755,6 +1755,52 @@ describe('SlickDatView core file', () => { ]); }); + it('should always use CSP-safe filtering when deprecated inline filter options are enabled', () => { + const minimumId = 2; + const items = [ + { id: 1, name: 'Bob', age: 33 }, + { id: 4, name: 'John', age: 20 }, + { id: 3, name: 'Jane', age: 24 }, + ]; + const filter = (item: any) => item.id >= minimumId; + const functionSpy = vi.spyOn(globalThis, 'Function'); + + dv = new SlickDataView({ inlineFilters: true, useCSPSafeFilter: false }); + dv.setItems(items); + dv.setFilter(filter); + + expect(functionSpy).not.toHaveBeenCalled(); + expect(dv.getFilter()).toBe(filter); + expect(dv.getFilteredItems()).toEqual([ + { id: 4, name: 'John', age: 20 }, + { id: 3, name: 'Jane', age: 24 }, + ]); + + functionSpy.mockRestore(); + }); + + it('should cache successful filters when the filter is expanding', () => { + const items = [ + { id: 1, name: 'Bob', age: 33 }, + { id: 4, name: 'John', age: 20 }, + { id: 3, name: 'Jane', age: 24 }, + ]; + const filter = vi.fn((item: any) => item.id >= 2); + + dv.setItems(items); + dv.setRefreshHints({ isFilterExpanding: true }); + dv.setFilter(filter); + + expect(filter).toHaveBeenCalledTimes(3); + + dv.setRefreshHints({ isFilterExpanding: true }); + dv.refresh(); + + expect(filter).toHaveBeenCalledTimes(4); + expect(filter).toHaveBeenLastCalledWith(items[0], undefined); + expect(dv.getFilteredItems()).toEqual([items[1], items[2]]); + }); + it('should be able to set a filter and extra filter arguments and expect items to be filtered', () => { const searchString = 'Ob'; // we'll provide "searchString" as filter args function myFilter(item: any, args: any) { @@ -1782,7 +1828,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', () => { + it('should keep deprecated inline and CSP-safe options backward compatible with filter arguments', () => { const searchString = 'Ob'; // we'll provide "searchString" as filter args const myFilter = (item: any, args: any) => item.name.toLowerCase().includes(args.searchString?.toLowerCase()); const items = [ diff --git a/packages/common/src/core/slickDataView.ts b/packages/common/src/core/slickDataView.ts index 837a70611d..ca702bdb0e 100644 --- a/packages/common/src/core/slickDataView.ts +++ b/packages/common/src/core/slickDataView.ts @@ -1,13 +1,4 @@ -import { - extend, - getFunctionDetails, - getHtmlStringOutput, - isDefined, - isHtml, - isPrimitiveOrHTML, - stripTags, - type AnyFunction, -} from '@slickgrid-universal/utils'; +import { extend, getHtmlStringOutput, isDefined, isHtml, isPrimitiveOrHTML, stripTags } from '@slickgrid-universal/utils'; import { SlickGroupItemMetadataProvider } from '../extensions/slickGroupItemMetadataProvider.js'; import { exportWithFormatterWhenDefined } from '../formatters/formatterUtilities.js'; import type { CssStyleHash, CustomDataView } from '../interfaces/gridOption.interface.js'; @@ -60,8 +51,12 @@ function isLiveDomFormatterResult( export interface DataViewOption { /** - * Defaults to false, are we using inline filters? - * Note: please use with great care as this will break built-in filters + * @deprecated Filters are always CSP-safe and this option is now ignored. + * Next major cleanup: + * - Remove both `inlineFilters` and `useCSPSafeFilter` from `DataViewOption` and the default options. + * - Remove framework and vanilla-bundle code that forwards `inlineFilters`, along with its compatibility tests and documentation. + * - Remove the deprecated `FilterCspFn` and `FilterWithCspCachingFn` type aliases. + * - Consider renaming the protected `*CSPSafe` methods to neutral names and update their tests and benchmarks. */ inlineFilters: boolean; @@ -72,14 +67,16 @@ export interface DataViewOption { groupItemMetadataProvider: SlickGroupItemMetadataProvider | null; /** - * defaults to false, option to use CSP Safe approach, - * Note: it is an opt-in option because it is slightly slower (perf impact) when compared to the non-CSP safe approach. + * @deprecated Filters are always CSP-safe and this option is now ignored. Remove it in the next major together with + * `inlineFilters` by following that option's cleanup checklist. */ useCSPSafeFilter: boolean; } export type FilterFn = (item: T, args: any) => boolean; +/** @deprecated Filtering is always CSP-safe. Remove this unused alias in the next major. */ export type FilterCspFn = (item: T[], args: any) => T[]; +/** @deprecated Filtering is always CSP-safe. Remove this unused alias in the next major. */ export type FilterWithCspCachingFn = (item: T[], args: any, filterCache: any[]) => T[]; export type DataIdType = number | string; export type SlickDataItem = SlickNonDataItem | SlickGroup | SlickGroupTotals | any; @@ -106,7 +103,6 @@ export class SlickDataView implements CustomD protected idxById: Map = new Map(); // indexes by id protected rowsById: { [id: DataIdType]: number } | undefined = undefined; // rows by id; lazy-calculated protected filter: FilterFn | null = null; // filter function - protected filterCSPSafe: FilterFn | null = null; // filter function protected updated: { [id: DataIdType]: boolean } | null = null; // updated item ids protected suspend = false; // suspends the recalculation protected isBulkSuspend = false; // delays protectedious operations like the @@ -119,10 +115,6 @@ export class SlickDataView implements CustomD protected prevRefreshHints: DataViewHints = {}; protected filterArgs: any; protected filteredItems: TData[] = []; - protected compiledFilter?: FilterFn | null; - protected compiledFilterCSPSafe?: FilterCspFn | null; - protected compiledFilterWithCaching?: FilterFn | null; - protected compiledFilterWithCachingCSPSafe?: FilterWithCspCachingFn | null; protected filterCache: any[] = []; protected _grid?: SlickGrid; // grid object will be defined after using "syncGridSelection()" or "setGrid()" method protected _gridOptions?: ReturnType; // cached grid options, refreshed via onSetOptions subscription @@ -238,15 +230,10 @@ export class SlickDataView implements CustomD this.idxById = null as any; this.rowsById = null as any; this.filter = null as any; - this.filterCSPSafe = null as any; this.updated = null as any; this.sortComparer = null as any; this.filterCache = []; this.filteredItems = []; - this.compiledFilter = null; - this.compiledFilterCSPSafe = null; - this.compiledFilterWithCaching = null; - this.compiledFilterWithCachingCSPSafe = null; this.clearFormattedDataCache(); if (this._grid) { this._grid.onSelectedRowsChanged?.unsubscribe(); @@ -444,7 +431,7 @@ export class SlickDataView implements CustomD /** Get current Filter used by the DataView */ getFilter(): FilterFn | null { - return this._options.useCSPSafeFilter ? this.filterCSPSafe : this.filter; + return this.filter; } /** @@ -452,14 +439,7 @@ export class SlickDataView implements CustomD * @param {Function} fn - filter callback function */ setFilter(filterFn: FilterFn): void { - this.filterCSPSafe = filterFn; this.filter = filterFn; - if (this._options.inlineFilters) { - this.compiledFilterCSPSafe = this.compileFilterCSPSafe; - this.compiledFilterWithCachingCSPSafe = this.compileFilterWithCachingCSPSafe; - this.compiledFilter = this.compileFilter(this._options.useCSPSafeFilter); - this.compiledFilterWithCaching = this.compileFilterWithCaching(this._options.useCSPSafeFilter); - } this.refresh(); } @@ -1149,14 +1129,12 @@ export class SlickDataView implements CustomD protected compileAccumulatorLoopCSPSafe(aggregator: Aggregator): (items: any[]) => void { if (aggregator.accumulate) { return function (items: any[]) { - let result; if (Array.isArray(items)) { for (let i = 0; i < items.length; i++) { const item = items[i]; - result = aggregator.accumulate!.call(aggregator, item); + aggregator.accumulate!(item); } } - return result; }; } else { return function noAccumulator() {}; @@ -1165,110 +1143,25 @@ export class SlickDataView implements CustomD protected compileFilterCSPSafe(items: TData[], args: any): TData[] { /* v8 ignore if */ - if (typeof this.filterCSPSafe !== 'function') { + if (typeof this.filter !== 'function') { return []; } const retval: TData[] = []; const il = items.length; for (let _i = 0; _i < il; _i++) { - if (this.filterCSPSafe(items[_i], args)) { - retval.push(items[_i]); + const item = items[_i]; + if (this.filter(item, args)) { + retval.push(item); } } return retval; } - protected compileFilter(stopRunningIfCSPSafeIsActive = false): FilterFn | null { - if (stopRunningIfCSPSafeIsActive) { - return null; - } - const filterInfo = getFunctionDetails(this.filter as FilterFn); - - const filterPath1 = '{ continue _coreloop; }$1'; - const filterPath2 = '{ _retval[_idx++] = $item$; continue _coreloop; }$1'; - // make some allowances for minification - there's only so far we can go with RegEx - const filterBody = filterInfo.body - .replace(/return false\s*([;}]|\}|$)/gi, filterPath1) - .replace(/return!1([;}]|\}|$)/gi, filterPath1) - .replace(/return true\s*([;}]|\}|$)/gi, filterPath2) - .replace(/return!0([;}]|\}|$)/gi, filterPath2) - .replace(/return ([^;}]+?)\s*([;}]|$)/gi, '{ if ($1) { _retval[_idx++] = $item$; }; continue _coreloop; }$2'); - - // This preserves the function template code after JS compression, - // so that replace() commands still work as expected. - let tpl = [ - // 'function(_items, _args) { ', - 'var _retval = [], _idx = 0; ', - 'var $item$, $args$ = _args; ', - '_coreloop: ', - 'for (var _i = 0, _il = _items.length; _i < _il; _i++) { ', - '$item$ = _items[_i]; ', - '$filter$; ', - '} ', - 'return _retval; ', - // '}' - ].join(''); - tpl = tpl.replace(/\$filter\$/gi, filterBody); - tpl = tpl.replace(/\$item\$/gi, filterInfo.params[0]); - tpl = tpl.replace(/\$args\$/gi, filterInfo.params[1]); - const fn: any = new Function('_items,_args', tpl); - const fnName = 'compiledFilter'; - fn.displayName = fnName; - fn.name = this.setFunctionName(fn, fnName); - return fn; - } - - protected compileFilterWithCaching(stopRunningIfCSPSafeIsActive = false): FilterFn | null { - if (stopRunningIfCSPSafeIsActive) { - return null; - } - - const filterInfo = getFunctionDetails(this.filter as FilterFn); - - const filterPath1 = '{ continue _coreloop; }$1'; - const filterPath2 = '{ _cache[_i] = true;_retval[_idx++] = $item$; continue _coreloop; }$1'; - // make some allowances for minification - there's only so far we can go with RegEx - const filterBody = filterInfo.body - .replace(/return false\s*([;}]|\}|$)/gi, filterPath1) - .replace(/return!1([;}]|\}|$)/gi, filterPath1) - .replace(/return true\s*([;}]|\}|$)/gi, filterPath2) - .replace(/return!0([;}]|\}|$)/gi, filterPath2) - .replace(/return ([^;}]+?)\s*([;}]|$)/gi, '{ if ((_cache[_i] = $1)) { _retval[_idx++] = $item$; }; continue _coreloop; }$2'); - - // This preserves the function template code after JS compression, - // so that replace() commands still work as expected. - let tpl = [ - // 'function(_items, _args, _cache) { ', - 'var _retval = [], _idx = 0; ', - 'var $item$, $args$ = _args; ', - '_coreloop: ', - 'for (var _i = 0, _il = _items.length; _i < _il; _i++) { ', - '$item$ = _items[_i]; ', - 'if (_cache[_i]) { ', - '_retval[_idx++] = $item$; ', - 'continue _coreloop; ', - '} ', - '$filter$; ', - '} ', - 'return _retval; ', - // '}' - ].join(''); - tpl = tpl.replace(/\$filter\$/gi, filterBody); - tpl = tpl.replace(/\$item\$/gi, filterInfo.params[0]); - tpl = tpl.replace(/\$args\$/gi, filterInfo.params[1]); - - const fn: any = new Function('_items,_args,_cache', tpl); - const fnName = 'compiledFilterWithCaching'; - fn.displayName = fnName; - fn.name = this.setFunctionName(fn, fnName); - return fn; - } - protected compileFilterWithCachingCSPSafe(items: TData[], args: any, filterCache: any[]): TData[] { /* v8 ignore if */ - if (typeof this.filterCSPSafe !== 'function') { + if (typeof this.filter !== 'function') { return []; } @@ -1276,54 +1169,12 @@ export class SlickDataView implements CustomD const il = items.length; for (let _i = 0; _i < il; _i++) { - if (filterCache[_i] || this.filterCSPSafe(items[_i], args)) { - retval.push(items[_i]); - } - } - - return retval; - } - - /** - * In ES5 we could set the function name on the fly but in ES6 this is forbidden and we need to set it through differently - * We can use Object.defineProperty and set it the property to writable, see MDN for reference - * https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty - * @param {*} fn - * @param {string} fnName - */ - protected setFunctionName(fn: any, fnName: string): void { - try { - Object.defineProperty(fn, 'name', { writable: true, value: fnName }); - } /* v8 ignore next */ catch { - fn.name = fnName; - } - } - - protected uncompiledFilter(items: TData[], args: any): any[] { - const retval: any[] = []; - let idx = 0; - - for (let i = 0, ii = items.length; i < ii; i++) { - if (this.filter?.(items[i], args)) { - retval[idx++] = items[i]; - } - } - - return retval; - } - - protected uncompiledFilterWithCaching(items: TData[], args: any, cache: any): any[] { - const retval: any[] = []; - let idx = 0; - let item: TData; - - for (let i = 0, ii = items.length; i < ii; i++) { - item = items[i]; - if (cache[i]) { - retval[idx++] = item; - } else if (this.filter?.(item, args)) { - retval[idx++] = item; - cache[i] = true; + const item = items[_i]; + if (filterCache[_i]) { + retval.push(item); + } else if (this.filter(item, args)) { + filterCache[_i] = true; + retval.push(item); } } @@ -1331,26 +1182,13 @@ export class SlickDataView implements CustomD } protected getFilteredAndPagedItems(items: TData[]): { totalRows: number; rows: TData[] } { - if (this._options.useCSPSafeFilter ? this.filterCSPSafe : this.filter) { - let batchFilter: AnyFunction; - let batchFilterWithCaching: AnyFunction; - if (this._options.useCSPSafeFilter) { - batchFilter = (this._options.inlineFilters ? this.compiledFilterCSPSafe : this.uncompiledFilter) as AnyFunction; - batchFilterWithCaching = ( - this._options.inlineFilters ? this.compiledFilterWithCachingCSPSafe : this.uncompiledFilterWithCaching - ) as AnyFunction; - } else { - batchFilter = (this._options.inlineFilters ? this.compiledFilter : this.uncompiledFilter) as AnyFunction; - batchFilterWithCaching = ( - this._options.inlineFilters ? this.compiledFilterWithCaching : this.uncompiledFilterWithCaching - ) as AnyFunction; - } + if (this.filter) { if (this.refreshHints.isFilterNarrowing) { - this.filteredItems = batchFilter.call(this, this.filteredItems, this.filterArgs); + this.filteredItems = this.compileFilterCSPSafe(this.filteredItems, this.filterArgs); } else if (this.refreshHints.isFilterExpanding) { - this.filteredItems = batchFilterWithCaching.call(this, items, this.filterArgs, this.filterCache); + this.filteredItems = this.compileFilterWithCachingCSPSafe(items, this.filterArgs, this.filterCache); } else if (!this.refreshHints.isFilterUnchanged) { - this.filteredItems = batchFilter.call(this, items, this.filterArgs); + this.filteredItems = this.compileFilterCSPSafe(items, this.filterArgs); } } else { // special case: if not filtering and not paging, the resulting diff --git a/test/benchmarks/README.md b/test/benchmarks/README.md new file mode 100644 index 0000000000..20a34f0ee0 --- /dev/null +++ b/test/benchmarks/README.md @@ -0,0 +1,23 @@ +# Performance benchmarks + +The SlickDataView benchmark measures the production filter and accumulator loops in isolation from paging, grouping, events, and row-difference calculations. + +Run the benchmark on an otherwise idle machine: + +```sh +pnpm bench:data-view +``` + +To compare results across revisions, first save a baseline: + +```sh +pnpm bench:data-view --outputJson /tmp/slick-dataview-before.json +``` + +Then run the comparison from the changed revision: + +```sh +pnpm bench:data-view --compare /tmp/slick-dataview-before.json +``` + +Use the relative results rather than absolute operations per second. Repeat the benchmark at least three times and treat differences smaller than the reported relative margin of error as inconclusive. diff --git a/test/benchmarks/slickDataView.bench.ts b/test/benchmarks/slickDataView.bench.ts new file mode 100644 index 0000000000..810b3b0811 --- /dev/null +++ b/test/benchmarks/slickDataView.bench.ts @@ -0,0 +1,108 @@ +import { afterAll, bench, describe, expect } from 'vitest'; +import { SlickDataView } from '../../packages/common/src/core/slickDataView.js'; +import type { Aggregator } from '../../packages/common/src/interfaces/aggregator.interface.js'; + +interface BenchmarkItem { + active: boolean; + id: number; + name: string; + value: number; +} + +interface BenchmarkFilterArgs { + minimumId: number; + searchTerm: string; +} + +type BenchmarkAccumulatorRunner = (items: BenchmarkItem[]) => void; + +class BenchmarkDataView extends SlickDataView { + runFilter(items: BenchmarkItem[], args: BenchmarkFilterArgs): BenchmarkItem[] { + return this.compileFilterCSPSafe(items, args); + } + + createAccumulatorRunner(aggregator: Aggregator): BenchmarkAccumulatorRunner { + return this.compileAccumulatorLoopCSPSafe(aggregator); + } +} + +const benchmarkOptions = { + time: 1_500, + warmupTime: 500, +}; +const items = Array.from({ length: 100_000 }, (_, id) => ({ + active: id % 2 === 0, + id, + name: `row-${id}`, + value: id % 101, +})); +const filterArgs: BenchmarkFilterArgs = { + minimumId: 50_000, + searchTerm: '99', +}; +const numericFilter = (item: BenchmarkItem, args: BenchmarkFilterArgs): boolean => { + if (!item.active) { + return false; + } + return item.id >= args.minimumId; +}; +const stringFilter = (item: BenchmarkItem, args: BenchmarkFilterArgs): boolean => item.name.toLowerCase().includes(args.searchTerm); + +const numericDataView = new BenchmarkDataView(); +numericDataView.setFilter(numericFilter); +const stringDataView = new BenchmarkDataView(); +stringDataView.setFilter(stringFilter); +let observedResult = 0; + +describe('SlickDataView filter loop (100,000 items)', () => { + bench( + 'production loop - numeric with early return', + () => { + observedResult += numericDataView.runFilter(items, filterArgs).length; + }, + benchmarkOptions + ); + + bench( + 'production loop - string predicate', + () => { + observedResult += stringDataView.runFilter(items, filterArgs).length; + }, + benchmarkOptions + ); +}); + +const accumulator: Aggregator & { total: number } = { + accumulate(item: BenchmarkItem): void { + this.total += item.value; + }, + field: 'value', + init(): void { + this.total = 0; + }, + storeResult(): void {}, + total: 0, + type: 'sum', +}; +const accumulatorRunner = numericDataView.createAccumulatorRunner(accumulator); + +describe('SlickDataView accumulator loop (100,000 items)', () => { + bench( + 'production loop', + () => { + accumulator.init(); + accumulatorRunner.call(accumulator, items); + observedResult += accumulator.total; + }, + benchmarkOptions + ); +}); + +afterAll(() => { + expect(numericDataView.runFilter(items, filterArgs)).toHaveLength(25_000); + expect(stringDataView.runFilter(items, filterArgs)).toHaveLength(3_691); + expect(observedResult).toBeGreaterThan(0); + + numericDataView.destroy(); + stringDataView.destroy(); +}); diff --git a/test/vitest.benchmark.config.mts b/test/vitest.benchmark.config.mts new file mode 100644 index 0000000000..3e34505465 --- /dev/null +++ b/test/vitest.benchmark.config.mts @@ -0,0 +1,13 @@ +import { defineConfig } from 'vitest/config'; + +export default defineConfig({ + test: { + benchmark: { + include: ['test/benchmarks/**/*.bench.ts'], + }, + environment: 'node', + fileParallelism: false, + maxWorkers: 1, + watch: false, + }, +});