Request: HitTestInfo type improvements (TypeScript)

Request: HitTestInfo type improvements (TypeScript)

Posted by: mark.brodziak on 20 August 2024, 12:37 am EST

• 

  mark.brodziak

  • Post Options:
  • Link
    Copy

  Posted 20 August 2024, 12:37 am EST - Updated 20 August 2024, 3:02 am EST

  Dear Mescius,

  If possible, can you please consider improvements to the TypeScript correctness across the board, but most specifically in HitTestInfo (based on observed behaviour - you might have more information that might inform a more correct view):

  readonly point: Point;
  // should be
  readonly point: Point | undefined;
  
  readonly panel: GridPanel;
  // should be
  readonly panel: GridPanel | null | undefined;
  
  readonly grid: FlexGrid;
  // should be
  readonly grid: FlexGrid | null;
  
  getRow(): Row;
  // should be
  getRow(): Row | null;
  
  getColumn(binding?: boolean): Column;
  // should be
  getColumn(binding?: boolean): Column | null;
  
  readonly target: Element;
  // should be
  readonly target: Element | undefined;

  Thank you.

• 

  vivek.pannu

  

  • Post Options:
  • Link
    Copy

  Posted 20 August 2024, 9:02 am EST

  Hi Mark,

  The properties mentioned in the code snippet you shared are all read only properties and by design their types are set to the following types:

  readonly point: Point;
  
      readonly cellType: CellType;
      
      readonly panel: GridPanel;
      
      readonly grid: FlexGrid;
      
      readonly row: number;
      
      readonly mergedRange: CellRange | null;
     
      getRow(): Row;
      
      readonly col: number;
     
      getColumn(binding?: boolean): Column;
     
      readonly range: CellRange;
      
      readonly edgeLeft: boolean;
      
      readonly edgeTop: boolean;
      
      readonly edgeRight: boolean;
      
      readonly edgeFarRight: boolean;
      
      readonly edgeBottom: boolean;
     
      readonly edgeFarBottom: boolean;
     
      readonly target: Element;

  The values for these properties are set internally and should not cause any typescript issues. Could you please share some more information, if you are facing any issues due to these types, so that we can have a better understanding of this matter and take appropriate steps, if needed.

  Regards

• 

  mark.brodziak

  • Post Options:
  • Link
    Copy

  Posted 20 August 2024, 7:22 pm EST

  Understood that these are readonly properties, but that’s not really the point.

  Without going through these exhaustively, I can say that we have observed the specified values at runtime when using HitTestInfo. Here is a very basic example forked from your own demo:

  https://jscodemine.mescius.io/sample/0Vep41jik0WgV6R2TCQvrg/

  You can see from this example that getRow() and getColumn() are both sometimes returning null depending on where you hover on the grid.

  We run TypeScript with strict null checks and strict types, meaning that declaring a return type of Column is different to declaring a return type of Column | null, because the former implies that null will not be returned. That is not the case.

  Reviewing and adding null (and/or undefined, depending on the situation) to the various declared types on this class would improve the ergonomics of working with it. Otherwise we essentially have to be vigilant because the compiler will not tell us that we need to handle null (and that has caused bugs for us in the past).

• 

  vivek.pannu

  

  • Post Options:
  • Link
    Copy

  Posted 22 August 2024, 5:15 am EST

  Hi Mark,

  Thank you for sharing the additional information, we have forwarded your request to the engineering team with internal tracking ID - WJM-34759. We will update you when we have an update from the engineering team.

  Regards