paperjs / paper.js

The Swiss Army Knife of Vector Graphics Scripting – Scriptographer ported to JavaScript and the browser, using HTML5 Canvas. Created by @lehni & @puckey
http://paperjs.org
Other
14.51k stars 1.23k forks source link

TypeScript Types 0.12.3 Feedback #1687

Open Domvel opened 5 years ago

Domvel commented 5 years ago

Just a short feedback of a few type issues of the awesome paper.js.

  1. The mouse events of Tool are any type. The Path class mouse methods have the MouseEvent as type.

    const tool = new Tool();
    tool.onMouseDown = event => {
    console.log(event, 'is any type. Should be a defined type (MouseEvent).');
    }

    MouseEvent property point is type Point | null (optional) is this correctly?

  2. The property fullySelected does not exist on type Group. This is also missing in the documentation. I think it's a forgotten type deklaration. It works on runtime.

  3. Group.children should be initialized with an empty array. Currently it's the type Item[] | null. Maybe this is correclty. But I think a Group always have the property childrean and should be initialized with [] (Array). To avoid optional types. Otherwise in TypeScript strict mode, I have to check it for null before accessing. It's a bit annoying. In strict TypeScript you have to set a value on deklaration or in constructor. Otherwise it's option. ...

if (!group.children) {
  // I have to, because children can be null. I expected an empty array if no childs.
  return;
}

group.children.forEach(child => {
  console.log(child);
});
sasensi commented 5 years ago

Hi, thank you for the feedback.

Domvel commented 5 years ago

Thanks for quick reply. :) Here's a suggestion about the optional properties of several objects. e.g. Path.

  1. Path optional properties is type object. I suggest a more specific type which contains all possible attributes. Where can I see all possible attributes in the docs? Currently:
    /** 
    * Creates a new path item from an object description and places it at the
    * top of the active layer.
    * 
    * @param object - an object containing properties to be set on the
    *     path
    */
    constructor(object: object)

The type could be and interface:

enum StrokeCaps {
  Round = 'round'
  // etc ...
}

interface PathOptions {
  segments?: number[][];
  strokeWidth?: number;
  strokeCap?: StrokeCaps;
  selected?: boolean;
  // etc ...
}

constructor(properties?: PathOptions)

Segments maybe accept more types like Point[] and Segment[]. (?) strokeCap enum would be great but a string is also ok.

Maybe other similar classes are affected or it's a common property object which should be abstract. (interface extend)

btw. Item.position should be Point not Point | null. Not optional. E.g. Circle (Path). Is position is required and default 0,0. Or really null?