feature/ts-strict/qgrid-ngx by FinalButterfly · Pull Request #614 · qgrid/ng2

FinalButterfly · 2022-05-18T09:53:04Z

No description provided.

Dalantren · 2022-05-19T11:09:12Z

packages/qgrid-ngx/src/lib/cell-handler/cell-handler.module.ts

-  imports: [
-    CommonModule,
-  ],
+  declarations: [CellHandlerComponent],


put back formatting.
It should be autoformatted on save

Dalantren · 2022-05-20T11:16:39Z

packages/qgrid-ngx/src/lib/cell-handler/cell-handler.component.ts

        if (e.hasChanges('status')) {
          if (e.state.status === 'endBatch' && this.endBatchEdit) {
            this.endBatchEdit();
-            this.endBatchEdit = null;


Check if this will break something

it should be reverted

Dalantren · 2022-05-20T11:18:57Z

packages/qgrid-ngx/src/lib/column/column.component.ts

      if (this.parentHost) {
-        this.parentHost.column.children.push(column);
+        if (this.parentHost.column.children) {
+          this.parentHost.column.children.push(column);


actually you can replace these lines with
this.parentHost.column.children?.push(column);

Dalantren · 2022-05-20T11:19:54Z

packages/qgrid-ngx/src/lib/column/column.component.ts

-          .reduce((memo, key) => {
-            memo[key] = column[key];
+          .filter(key => !isUndefined(this.key) && Object.prototype.hasOwnProperty.call(column, key))
+          .reduce((memo: any, key: string) => {


memo isn't any.
{ [key: string]: Column } if i got this right

Dalantren · 2022-05-20T11:25:05Z

packages/qgrid-ngx/src/lib/column/column.component.ts

    const { column } = this.selfHost;
-    if (column && column.source === 'template') {
-      this.columnList.delete(column.key);
+    if (!(column && column.source === 'template')) {


!column || column.source !== 'template' is more readable, as for me.
If there's no column OR coumn source is not template.
Same thing with column.key.

Btw, why you even need to change this condition?

if (column && column.key && column.source === 'template') { this.columnList.delete(column.key); }

Dalantren · 2022-05-20T11:28:35Z

packages/qgrid-ngx/src/lib/resize/resize.directive.ts

-    context.width = host.clientWidth;
-    context.height = host.clientHeight;
+    if(host) {
+      context.width = host.clientWidth;


You can remove if (host) condition

context.width = host?.clientWidth; context.height = host?.clientHeight;

Dalantren · 2022-05-20T11:31:23Z

packages/qgrid-ngx/src/lib/row/row.component.ts

-                if (target.column.type !== 'row-expand') {
-                  if (this.toggleStatus.canExecute(target.row)) {
-                    this.toggleStatus.execute(target.row);
+              const timestampNewValue = timestamp.newValue ?? 0;


it's better to add delay variable here

const delay = (timestamp.newValue || 0) - (timestamp.oldValue || 0); if (firstClickTarget === target && delay <= dblClickInterval) { ... }

Dalantren · 2022-05-20T11:34:36Z

packages/qgrid-ngx/src/lib/row/tr-core.directive.ts

  @Input('q-grid-core-index') viewIndex: number;
  @Input('q-grid-core-tr') model: any;
-  @Input('q-grid-core-source') source;
+  @Input('q-grid-core-source') source: string;


type BoxParts = keyof { body: Bag; head: Bag; foot: Bag }; OR type BoxParts = 'body' | 'head' | 'foot'; ... @Input('q-grid-core-source') source: BoxParts; ... this.plugin.table.box.bag[this.source as BoxParts]

Btw, check if there's already an interface that describe this thing { body: Bag; head: Bag; foot: Bag } and use it if exist

Dalantren · 2022-05-20T11:35:00Z

packages/qgrid-ngx/src/lib/row/trh-core.directive.ts

  @Input('q-grid-core-index') index: number;
  @Input('q-grid-core-trh') model: any;
-  @Input('q-grid-core-source') source;
+  @Input('q-grid-core-source') source: string;


same here for BoxParts

Dalantren · 2022-05-20T11:37:07Z

packages/qgrid-ngx/tsconfig.json

@@ -0,0 +1,43 @@
+{


Why do we need this file?

I need it, but the repository does not. Accidental commit.

- Fixed a bug with qgrid-core type

Dalantren · 2022-05-20T16:14:01Z

packages/qgrid-ngx/src/lib/body/body-core.component.ts


    observeReply(model.sceneChanged)
-      .subscribe(e => {
+      .subscribe((e: any) => {


e: GridEventArg<SceneState>

Dalantren · 2022-05-20T16:14:12Z

packages/qgrid-ngx/src/lib/body/body-core.component.ts


    observe(model.sceneChanged)
-      .subscribe(e => {
+      .subscribe((e: any) => {


e: GridEventArg<SceneState>

Dalantren · 2022-05-20T16:18:46Z

packages/qgrid-ngx/src/lib/body/body.module.ts

-  exports: [
-    BodyCoreComponent,
-  ],
+  exports: [BodyCoreComponent],


check indentation.
Should be as before

Dalantren · 2022-05-20T16:28:01Z

packages/qgrid-core/src/scene/render/render.d.ts

 export declare class Renderer {
  defaultStrategy: RenderStrategy;
-  readonly rows: { left: any[]; right: any[]; mid: any[] };
+  readonly rows: Record<string, any>;


I'd create an interface for left, right, mid object and use it here.

export interface RowsPosition { left: any[]; mid: any[]; right: any[]; } .... readonly rows: RowsPosition;

Because Record<string, ...> could accept any string as a key;

Dalantren · 2022-05-20T16:33:41Z

packages/qgrid-ngx/src/lib/layer/layer.service.ts

  }

-  create(name) {
+  create(name: string): Layer | undefined {


why undefined? It always return a Layer. If it's not exist it will be created

Dalantren · 2022-05-20T16:41:49Z

packages/qgrid-ngx/src/lib/theme/theme.service.ts

@@ -1,4 +1,4 @@
-import { Injectable } from '@angular/core';
+import { Component, Injectable } from '@angular/core';


Component isn't used

Dalantren · 2022-05-20T16:45:08Z

packages/qgrid-ngx/src/lib/vscroll/vscroll-column.directive.ts

  ngOnChanges(changes: SimpleChanges) {
    if (changes['index']) {
      if (this.port.getItemSize()) {
-        this.ngOnChanges = null;


i'm afraid this would break something.
If you can't assign this method to null create a flag variable and after first visit set it to true;

private onChangesRan = false; ... ngOnChanges(changes: SimpleChanges) { if (this.onChangesRan) { return; } if (changes['index']) { if (this.port.getItemSize()) { this.onChangesRan = true; } } }

Dalantren · 2022-05-20T16:46:13Z

packages/qgrid-ngx/src/lib/vscroll/vscroll-row.directive.ts

  ngOnChanges(changes: SimpleChanges) {
    if (changes['index']) {
      if (this.port.getItemSize()) {
-        this.ngOnChanges = null;


Same for onChanges here

It was done like that for the perfomance, vscroll need the best perfomance, and onChanges needs only once, but it calls a lot

Dalantren · 2022-05-20T16:46:34Z

packages/qgrid-ngx/src/lib/vscroll/vscroll.module.ts

-  providers: [
-    VscrollService,
-  ],
+  providers: [VscrollService],


fix indentation

Dalantren · 2022-05-20T16:48:22Z

packages/qgrid-ngx/src/lib/vscroll/vscroll.pipe.ts

  }

-  transform(data: any[], context: IVscrollContext): ObservableLike<any[]> {
+  transform(data: any[], context: IVscrollContext): ObservableLike<any> | PromiseLike<any> | SubjectLike<any> {


this method returns items$ which is always SubjectLike<any>, isn't it?

klumba12 · 2022-05-27T11:41:08Z

packages/qgrid-core/src/format/format.service.d.ts

-  static number(x: number, format: string): string;
-  static date(x: Date, format: string): string;
-  static currency(x: number, format: string): string;
+  static number(x: number, format: string): string | null;


lets discuss to use Nullable instead of string | null

klumba12 · 2022-05-27T11:41:41Z

packages/qgrid-core/src/plugin/grid.plugin.d.ts


-    observe<TState>(event: Event<TState>): ObservableLike<TState>;
-    observeReply<TState>(event: Event<TState>): ObservableLike<TState>;
+    observe<TState>(event: Event<TState>): ObservableEvent<TState>;


why we change it to ObservableEvent? it should return the minimal required interface

klumba12 · 2022-05-27T11:52:48Z

packages/qgrid-core/src/scene/render/render.d.ts

 import { ColumnView } from '../view/column.view';
 import { RenderStrategy } from './render.strategy';

+export interface RowPosition {


at least it should be called something like RowsSite

klumba12 · 2022-05-27T11:53:28Z

packages/qgrid-ngx/src/lib/body/body-core.component.ts

  }

-  columnId(index: number, item: ColumnView) {
+  columnId(index: number, item: ColumnView): string | undefined {


shorly string?

This is not possible. Have to specify it as a union.

klumba12 · 2022-05-27T11:53:59Z

packages/qgrid-ngx/src/lib/cell-handler/cell-handler.component.ts

        if (e.hasChanges('status')) {
          if (e.state.status === 'endBatch' && this.endBatchEdit) {
            this.endBatchEdit();
-            this.endBatchEdit = null;


it should be reverted

klumba12 · 2022-05-27T12:04:35Z

packages/qgrid-ngx/src/lib/resize/resize.directive.ts

-  @Input('q-grid-resize-path') path;
-  @Input('q-grid-can-resize') canResize;
-  @Input('q-grid-resize-selector') selector;
+  @Input('q-grid-resize') key?: string;


why ? here

klumba12 · 2022-05-27T12:04:56Z

packages/qgrid-ngx/src/lib/resize/resize.directive.ts

-  @Input('q-grid-resize-selector') selector;
+  @Input('q-grid-resize') key?: string;
+  @Input('q-grid-resize-path') path: string;
+  @Input('q-grid-can-resize') canResize: (e: unknown) => boolean;


it's defenitly not unknown

klumba12 · 2022-05-27T12:06:17Z

packages/qgrid-ngx/src/lib/row/trh-core.directive.ts


  ngOnInit() {
-    this.plugin.table.box.bag[this.source].addRow(this);
+    this.plugin.table.box.bag[this.source as keyof { body: Bag; head: Bag; foot: Bag }].addRow(this);


keyof typeof bag?

Bag does not contain said fields, so it's not possible to use keyof typeof, unless we add the said fields into the exported Bag class.

klumba12 · 2022-05-27T12:07:38Z

packages/qgrid-ngx/src/lib/vscroll/vscroll-port-y.directive.ts

  @Input('q-grid-vscroll-port-y') context: VscrollContext;

-  layout: VscrollLayout = null;
+  layout: VscrollLayout;


please check if default value not affects

All ok. It's undefined in the OnInit hook.

I mean if anywere there is a check on null but god undefined, it can be broken

All ok. I couldn't find any code that checks if layout is null.

klumba12 · 2022-05-27T12:08:07Z

packages/qgrid-ngx/src/lib/vscroll/vscroll.pipe.ts

  }

-  transform(data: any[], context: IVscrollContext): ObservableLike<any[]> {
+  transform(data: any[], context: IVscrollContext): SubjectLike<any> {


back to ObservableLike

klumba12 · 2022-05-31T10:52:52Z

packages/qgrid-core/src/utility/kit.d.ts

 export declare function binarySearch<T>(list: Array<any>, value: any): number;
 export declare function getTypeName(ctor: new () => any): string;
+
+export declare type Nullable<T> = T | null;


lets have a separate file types.d.ts in utility folder, with all those helper files

klumba12 · 2022-05-31T10:53:44Z

packages/qgrid-ngx/src/lib/body/td-core.directive.ts

-  @Input('q-grid-core-value') actualValue: any;
-  @Input('q-grid-core-label') actualLabel: any;
+  @Input('q-grid-core-value') actualValue: unknown;
+  @Input('q-grid-core-label') actualLabel: unknown;


not sure about that

klumba12 · 2022-05-31T10:54:17Z

packages/qgrid-ngx/src/lib/cell-handler/cell-handler.component.ts

 })
 export class CellHandlerComponent implements OnInit, AfterViewInit {
-  private endBatchEdit: () => void;
+  private endBatchEdit: (() => void) | null;


by default it undefined, and why Nullable is not used here?

klumba12 · 2022-05-31T10:54:46Z

packages/qgrid-ngx/src/lib/dnd/drag.directive.ts

 })
 export class DragDirective {
-  @Input('q-grid-drag-data') data: any;
+  @Input('q-grid-drag-data') data: unknown;


klumba12 · 2022-05-31T10:55:07Z

packages/qgrid-ngx/src/lib/grid/grid.component.ts

    private stateAccessor: StateAccessor,
    private modelBuilder: GridModelBuilder,
-    @Inject(DOCUMENT) private document: any,
+    @Inject(DOCUMENT) private document: Document,


does it work? when you build it in prod?

klumba12 · 2022-05-31T10:56:02Z

packages/qgrid-ngx/src/lib/template/template.service.ts

  ) { }

-  find(keys: string | string[]): TemplateLink {
+  find(keys: string | string[]): TemplateLink | null {


klumba12 · 2022-05-31T10:56:09Z

packages/qgrid-ngx/src/lib/template/template.directive.ts


  @Input() key = '';
-  @Input() context: any = null;
+  @Input() context: unknown = null;


klumba12 · 2022-05-31T10:56:29Z

packages/qgrid-ngx/src/lib/vscroll/vscroll-port-x.directive.ts


  markup: { [key: string]: HTMLElement } = {};
-  layout: VscrollLayout = null;
+  layout: VscrollLayout;


not sure to remove default values is a good idea

klumba12 · 2022-05-31T10:57:10Z

packages/qgrid-ngx/src/lib/vscroll/vscroll-port-y.directive.ts

  @Input('q-grid-vscroll-port-y') context: VscrollContext;

-  layout: VscrollLayout = null;
+  layout: VscrollLayout;


I mean if anywere there is a check on null but god undefined, it can be broken

klumba12 · 2022-05-31T10:58:55Z

packages/qgrid-ngx/src/lib/vscroll/vscroll-row.directive.ts

  ngOnChanges(changes: SimpleChanges) {
    if (changes['index']) {
      if (this.port.getItemSize()) {
-        this.ngOnChanges = null;


It was done like that for the perfomance, vscroll need the best perfomance, and onChanges needs only once, but it calls a lot

klumba12 · 2022-06-02T12:25:57Z

packages/qgrid-ngx/src/lib/body/td-core.directive.ts

 export class TdCoreDirective implements DomTd, OnInit, OnDestroy, OnChanges {
  @Input('q-grid-core-value') actualValue: any;
-  @Input('q-grid-core-label') actualLabel: any;
+  @Input('q-grid-core-label') actualLabel: string;


It's not required to be string

klumba12 · 2022-06-02T12:26:58Z

packages/qgrid-ngx/src/lib/cell-handler/cell-handler.component.ts

 })
 export class CellHandlerComponent implements OnInit, AfterViewInit {
-  private endBatchEdit: () => void;
+  private endBatchEdit: Nullable<() => void>;


if it's only Nullable but got undefined on start? how lint missed that?

klumba12 · 2022-06-02T12:28:02Z

packages/qgrid-ngx/src/lib/column-list/column-list.service.ts

 export class ColumnListService {
  private host = new Lazy(() => {
-    const canCopy = (key: string, source, target) =>
+    const canCopy = (key: string, source: any, target: any) =>


source, target - Partial<ColumnModel>

klumba12 · 2022-06-02T13:17:43Z

packages/qgrid-ngx/src/lib/column/column.component.ts

  @Input() labelPath: string;

-  @Input() itemLabel: (row: any, value?: any) => any;
+  @Input() itemLabel: (row: any, value?: unknown) => string;


not sure about string

klumba12 · 2022-06-02T13:18:14Z

packages/qgrid-ngx/src/lib/dnd/drag.directive.ts

 })
 export class DragDirective {
-  @Input('q-grid-drag-data') data: any;
+  @Input('q-grid-drag-data') data: string | number | undefined | Node;


it sohuld be data or unknown

klumba12 · 2022-06-02T13:18:56Z

packages/qgrid-ngx/src/lib/foot/tf-core.directive.ts


  $implicit = this;
-  element: HTMLElement = null;
+  element: HTMLElement;


I won't remove default value here also HTMLELement should be changed to Nullable

FinalButterfly added 4 commits May 18, 2022 14:52

feature/ts-strict/qgrid-ngx

b7850f1

Removed commented out lines

302f960

Removed any type where possible

ae105c4

Fixed module files formatting

4622ca5

Dalantren reviewed May 20, 2022

View reviewed changes

FinalButterfly added 2 commits May 20, 2022 17:49

Substituted any with unknown where relevant

906a93d

- Fixed a bug with qgrid-core type

Comment corrections

186d19f

Dalantren reviewed May 20, 2022

View reviewed changes

FinalButterfly added 3 commits May 23, 2022 17:49

Comment corrections, second pass

de08c7e

Reverted .eslintrc.json to original state

b9b36b1

Corrected .eslintrc.json again

eee4a6f

klumba12 reviewed May 27, 2022

View reviewed changes

Comment corrections, first pass

10f3ead

klumba12 reviewed May 31, 2022

View reviewed changes

Comment corrections, second pass

be22350

klumba12 reviewed Jun 2, 2022

View reviewed changes

		@@ -1,4 +1,4 @@
		import { Injectable } from '@angular/core';
		import { Component, Injectable } from '@angular/core';

Conversation

FinalButterfly commented May 18, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dalantren May 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Dalantren May 20, 2022 •

edited

Loading