Skip to content

Commit

Permalink
perf: skip reapplying empty html when target is already empty (#1230)
Browse files Browse the repository at this point in the history
- when calling `applyHtmlCode()` method and our target element is empty and we try to reapply empty, it should always be ok to simply skip this reassignment.
- this should also help with performance since trying to reapply empty using `innerHTML` will probably cause a canvas repaint but if we manage to skip this then a repaint will also be skipped
- this should also help with CSP because we could write a Formatter with native element while still returning empty string when there's no value to return, that will then call `applyHtmlCode` and skip the assignment if the cell is already empty
  • Loading branch information
ghiscoding authored Nov 28, 2023
1 parent 5cb4dd5 commit ba99fae
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 4 deletions.
2 changes: 1 addition & 1 deletion packages/common/src/core/slickCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ export class SlickEditorLock {
}

export function isDefined<T>(value: T | undefined | null): value is T {
return <T>value !== undefined && <T>value !== null;
return <T>value !== undefined && <T>value !== null && <T>value !== '';
}

export class Utils {
Expand Down
15 changes: 12 additions & 3 deletions packages/common/src/core/slickGrid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -474,9 +474,12 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
* 3. value is string and `enableHtmlRendering` is disabled, then use `target.textContent = value;`
* @param target - target element to apply to
* @param val - input value can be either a string or an HTMLElement
* @param options - `emptyTarget`, defaults to true, will empty the target. `sanitizerOptions` is to provide extra options when using `innerHTML` and the sanitizer
* @param options -
* `emptyTarget`, defaults to true, will empty the target.
* `sanitizerOptions` is to provide extra options when using `innerHTML` and the sanitizer.
* `skipEmptyReassignment`, defaults to true, when enabled it will not try to reapply an empty value when the target is already empty
*/
applyHtmlCode(target: HTMLElement, val: string | HTMLElement | DocumentFragment = '', options?: { emptyTarget?: boolean; sanitizerOptions?: unknown; }) {
applyHtmlCode(target: HTMLElement, val: string | HTMLElement | DocumentFragment = '', options?: { emptyTarget?: boolean; sanitizerOptions?: unknown; skipEmptyReassignment?: boolean; }) {
if (target) {
if (val instanceof HTMLElement || val instanceof DocumentFragment) {
// first empty target and then append new HTML element
Expand All @@ -486,6 +489,11 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
}
target.appendChild(val);
} else {
// when it's already empty and we try to reassign empty, it's probably ok to skip the assignment
const skipEmptyReassignment = options?.skipEmptyReassignment !== false;
if (skipEmptyReassignment && !isDefined(val) && !target.innerHTML) {
return; // same result, just skip it
}
let sanitizedText = val;
if (typeof this._options?.sanitizer === 'function') {
sanitizedText = this._options.sanitizer(val || '');
Expand All @@ -494,7 +502,8 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
sanitizedText = DOMPurify.sanitize(val || '', purifyOptions);
}

if (this._options.enableHtmlRendering) {
// apply HTML when enableHtmlRendering is enabled but make sure we do have a value (without a value, it will simply use `textContent` to clear text content)
if (this._options.enableHtmlRendering && sanitizedText) {
target.innerHTML = sanitizedText;
} else {
target.textContent = sanitizedText;
Expand Down

0 comments on commit ba99fae

Please sign in to comment.