fix(common): make SlickDataView filtering CSP-safe by default - #2774
Merged
Conversation
angular-slickgrid
aurelia-slickgrid
slickgrid-react
slickgrid-vue
@slickgrid-universal/angular-row-detail-plugin
@slickgrid-universal/aurelia-row-detail-plugin
@slickgrid-universal/react-row-detail-plugin
@slickgrid-universal/vue-row-detail-plugin
@slickgrid-universal/binding
@slickgrid-universal/common
@slickgrid-universal/composite-editor-component
@slickgrid-universal/custom-footer-component
@slickgrid-universal/custom-tooltip-plugin
@slickgrid-universal/empty-warning-component
@slickgrid-universal/event-pub-sub
@slickgrid-universal/excel-export
@slickgrid-universal/graphql
@slickgrid-universal/odata
@slickgrid-universal/pagination-component
@slickgrid-universal/pdf-export
@slickgrid-universal/row-detail-view-plugin
@slickgrid-universal/rxjs-observable
@slickgrid-universal/sql
@slickgrid-universal/text-export
@slickgrid-universal/utils
@slickgrid-universal/vanilla-bundle
@slickgrid-universal/vanilla-force-bundle
@slickgrid-universal/web-mcp
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2774 +/- ##
=======================================
Coverage 100.0% 100.0%
=======================================
Files 199 199
Lines 25832 25763 -69
Branches 9142 9128 -14
=======================================
- Hits 25832 25763 -69
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
5 tasks
|
🎉 This pull request is included in version 10.10.0 📦 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Make
SlickDataViewfiltering and accumulator compilation CSP-safe by default by removing runtime code generation throughnew Function().The previous CSP-safe and non-CSP implementations are consolidated into a single callback-based implementation. The existing
inlineFiltersanduseCSPSafeFilteroptions remain accepted as deprecated no-ops for backward compatibility.Why
The generated filter and accumulator implementations require CSP policies to allow
unsafe-eval. This prevents applications using a strict Content Security Policy from using every DataView configuration safely.Maintaining separate CSP and non-CSP implementations also duplicates filtering, caching, accumulator, and dispatch logic.
Benchmarking shows that the unified CSP-safe implementation has equivalent or better performance for the default configuration. However, the explicitly enabled generated
Functionpath remains faster for string-heavy predicates. This trade-off is documented in the performance analysis below.Changes
new Function().setFilter(),getFilter(), filtering dispatch, and accumulator setup.inlineFiltersanduseCSPSafeFilter; both remain accepted but are ignored.FilterCspFnandFilterWithCspCachingFnaliases.Function;Performance analysis
The exact pre-change implementation was loaded alongside the updated implementation in the same Node 24 process.
Each comparison filtered 100,000 items. Execution order was rotated across 12 samples to reduce warm-up, garbage-collection, and benchmark-order bias. The comparison was repeated three times.
The following values are the average median execution time. Lower is better.
FunctionDefault configuration
Compared with the previous default
inlineFilters: falsepath:This is the relevant comparison for consumers who did not explicitly enable
inlineFilters.Previous CSP-safe path
Compared with the previous CSP-safe callback implementation:
These differences are small enough to consider the implementations relatively equivalent.
Generated inline path
Compared with
inlineFilters: trueusing generatedFunctioncode:The string predicate is the only tested scenario where generated runtime code showed a consistent advantage. This affects consumers who explicitly enabled
inlineFilters; the option was disabled by default.The absolute measured difference was approximately 0.51 ms per 100,000 items. This is small for an individual filtering pass but could become significant with very large datasets or frequent refreshes.
CSP-safe optimization investigation
Additional CSP-safe implementations were benchmarked to determine whether the generated string-filter performance could be preserved:
push();Array.filter();for...ofiteration.None consistently matched the generated implementation without regressing another workload.
The generated path benefits from embedding the arbitrary predicate body directly inside the item loop. A generic CSP-safe implementation must retain a callback boundary, which JavaScript engines cannot be relied upon to inline for every predicate.
Caching the predicate occasionally reduced the string-filter regression from approximately 16% to 12%, but the improvement was inconsistent and could reduce numeric-filter performance. Indexed writes did not provide a consistent improvement, while preallocation,
Array.filter(), andfor...ofwere slower.Performance conclusion
The performance trade-off is explicit:
inlineFilters: trueoption with string-heavy predicates may experience a 12–16% filtering-loop regression.This regression is accepted in exchange for unconditional CSP compatibility, removal of runtime code generation, and a simpler single implementation.
Backward compatibility
The
inlineFiltersanduseCSPSafeFilterproperties remain part ofDataViewOption, so existing applications continue to compile without configuration changes.Both properties are deprecated and ignored because filtering is now always CSP-safe. Filtering results and filter-argument behavior remain compatible.
The behavioral difference is limited to the performance characteristics of the previous
inlineFilters: truegenerated-code path.Next major release
A cleanup checklist was added directly to the deprecated API comments. The next major release can:
inlineFiltersanduseCSPSafeFilterfromDataViewOptionand its defaults;inlineFilters;FilterCspFnandFilterWithCspCachingFnaliases;*CSPSafemethods to neutral names and update their tests and benchmarks.The unsafe implementations are already removed by this change, so no additional generated filtering or accumulator implementation will need to be removed later.
Validation
pnpm test— 5,965 tests passed.pnpm lint— passed.pnpm prettier:check— passed.pnpm build— passed.pnpm bench:data-view— benchmark completed successfully.git diff --check— passed.SlickDataViewbranch coverage remains 90.83% because of unrelated existing branches.Comments
The benchmark intentionally isolates filter and accumulator loops from paging, grouping, events, and row-difference calculations. Absolute browser results may differ by JavaScript engine, but the benchmark provides a repeatable comparison of the affected code paths.
The benchmark can be run with:
A baseline can be saved and compared across revisions with:
Results should be repeated on an otherwise idle machine. Differences smaller than the reported relative margin of error should be treated as inconclusive.
AI / LLM assistance
Checklist