Repository navigation
Conversation
dbb02a5 to
8b98d23
Compare
| // eslint-disable-next-line @stylistic/max-len | ||
| // eslint-disable-next-line @typescript-eslint/no-misused-promises -- fail ignores the result | ||
| }).fail(d.reject); |
There was a problem hiding this comment.
can we cast d.reject like in data_controller.ts and drop both disables?
| // eslint-disable-next-line @stylistic/max-len | |
| // eslint-disable-next-line @typescript-eslint/no-misused-promises -- fail ignores the result | |
| }).fail(d.reject); | |
| }).fail(d.reject as (...args: unknown[]) => void); |
| const displaySelector: HeaderFilterGroupSelector | undefined = lookup | ||
| // @ts-expect-error compileGetter() is overloaded and cannot pick one for a union argument | ||
| ? compileGetter<HeaderFilterGroupSelector>(lookup.displayExpr) | ||
| : undefined; |
There was a problem hiding this comment.
compileGetter returns a function argument as is, so maybe narrowing with isFunction lets us remove the directive?
| const displaySelector: HeaderFilterGroupSelector | undefined = lookup | |
| // @ts-expect-error compileGetter() is overloaded and cannot pick one for a union argument | |
| ? compileGetter<HeaderFilterGroupSelector>(lookup.displayExpr) | |
| : undefined; | |
| const displayExpr = lookup?.displayExpr; | |
| const displaySelector: HeaderFilterGroupSelector | undefined = isFunction(displayExpr) | |
| ? displayExpr | |
| : lookup && compileGetter(displayExpr); |
| // @ts-expect-error updateColumnIndexes sets the index of every column | ||
| this._headerFilterController.showHeaderFilterMenu(column.index, false); |
There was a problem hiding this comment.
what about a guard like in m_sorting.ts instead of the directive?
| // @ts-expect-error updateColumnIndexes sets the index of every column | |
| this._headerFilterController.showHeaderFilterMenu(column.index, false); | |
| if (isDefined(column.index)) { | |
| this._headerFilterController.showHeaderFilterMenu(column.index, false); | |
| } |
| // @ts-expect-error updateColumnIndexes sets the index of every column | ||
| this._headerFilterController.showHeaderFilterMenu(groupColumn.index, true); |
There was a problem hiding this comment.
same here, and probably for columnIndex in m_grouping.ts too
| // @ts-expect-error updateColumnIndexes sets the index of every column | |
| this._headerFilterController.showHeaderFilterMenu(groupColumn.index, true); | |
| if (isDefined(groupColumn.index)) { | |
| this._headerFilterController.showHeaderFilterMenu(groupColumn.index, true); | |
| } |
| private _createGroupPanelItem( | ||
| $rootElement: dxElementWrapper, | ||
| groupColumn: Column, | ||
| ): dxElementWrapper { | ||
| // @ts-expect-error DataGrid's method |
There was a problem hiding this comment.
sorting already declares it protected with this reason text - should we align so one grep finds every DataGrid grouping leak?
| private _createGroupPanelItem( | |
| $rootElement: dxElementWrapper, | |
| groupColumn: Column, | |
| ): dxElementWrapper { | |
| // @ts-expect-error DataGrid's method | |
| protected _createGroupPanelItem( | |
| $rootElement: dxElementWrapper, | |
| groupColumn: Column, | |
| ): dxElementWrapper { | |
| // @ts-expect-error _createGroupPanelItem is defined by DataGrid grouping |
| export interface NormalizeGroupItemOptions | ||
| extends Pick<ProcessGroupItemsOptions, 'column' | 'headerFilterOptions'> { | ||
| path: unknown[]; | ||
| valueSelector?: HeaderFilterGroupSelector; | ||
| displaySelector?: HeaderFilterGroupSelector; | ||
| } |
There was a problem hiding this comment.
what do you think about declaring the fields directly instead of Pick?
| export interface NormalizeGroupItemOptions | |
| extends Pick<ProcessGroupItemsOptions, 'column' | 'headerFilterOptions'> { | |
| path: unknown[]; | |
| valueSelector?: HeaderFilterGroupSelector; | |
| displaySelector?: HeaderFilterGroupSelector; | |
| } | |
| export interface NormalizeGroupItemOptions { | |
| column: Column; | |
| headerFilterOptions?: HeaderFilter; | |
| path: unknown[]; | |
| valueSelector?: HeaderFilterGroupSelector; | |
| displaySelector?: HeaderFilterGroupSelector; | |
| } |
| } | ||
|
|
||
| public getDataSource(column) { | ||
| public getDataSource(column: HeaderFilterColumn): HeaderFilterDataSourceOptions | undefined { |
There was a problem hiding this comment.
returning | undefined here adds directives in filter_custom_operations.ts and in showHeaderFilterMenuBase - would a guard at those callers work, or should the return stay broad for now?
There was a problem hiding this comment.
This would change the behavior: adding a guard at the call sites would turn today's TypeError into a silent no-op.
| }); | ||
| const column: Column = extend(true, {}, this._columnsController.getColumns()[columnIndex]); | ||
|
|
||
| if (!column) { |
There was a problem hiding this comment.
extend(true, {}, ...) always returns an object, so this check never fires - could hasCellValueCalculator(column) replace it and also remove the calculateCellValue directive below?
| ); | ||
|
|
||
| if (!text) { | ||
| // @ts-expect-error the default options provide texts |
There was a problem hiding this comment.
can we use ?. here instead of the directive, like in core?
| } | ||
|
|
||
| items = origPostProcess && origPostProcess.call(this, items) || items; | ||
| items = origPostProcess?.call(this, items) ?? items; |
There was a problem hiding this comment.
?? items differs from the old || when postProcess returns a falsy non-array value - intended?
No description provided.