Graph constructors have incorrect typings

[luca-aurelia]

Hi zandaqo!

Thanks for the excellent work you're putting into this library. I'm working on some generative art projects and am looking forward to speeding things up considerably using structurae.

Also, thanks for the typings! I'm using TypeScript, and having that already included was awesome.

I noticed a small bug: in the typings file, the options argument for the Graph constructors is listed as having the type GridOptions. GridOptions only has the properties rows, columns, and pad. But according to the API reference, Graph options have the size, pad, and directed properties.

I've already modified my local copy of index.d.ts. Let me know if you'd like me to submit a PR with the corrections.

Thanks again for you hard work!

Cheers, Thomas

[zandaqo]

Hi, Thomas!

Thank you! I'm sorry for this, I do not use TS myself so I have hard time checking it all. I've fixed this particular issue, but if you ever encounter something like this feel free to submit a PR.

Also, in the latest version I've changed names of existing graph classes from (Un-)WeightedGraph to (Un-)WeightedAdjacencyMatrix since I'm adding AdjacencyList implementations for graphs as well and without specifying it would create more confusion. Sorry again for the hectic nature of this library so far, I hope to realease a stable (1.0) version soon and stop these wild API changes.

Thanks again!

[SupremeTechnopriest]

Hello @zandaqo

Found another instance of this issue. Here is the error output.

node_modules/structurae/index.d.ts:214:5 - error TS2416: Property 'get' in type 'ArrayView' is not assignable to the same property in base type 'ObjectView'.
  Type '(index: number) => ObjectView' is not assignable to type '(field: string) => number | StringView | ArrayView | ObjectView | TypedArrayView'.
    Types of parameters 'index' and 'field' are incompatible.
      Type 'string' is not assignable to type 'number'.

214     get(index: number): ObjectView;
        ~~~

node_modules/structurae/index.d.ts:215:5 - error TS2416: Property 'set' in type 'ArrayView' is not assignable to the same property in base type 'ObjectView'.
  Type '(index: number, value: object) => this' is not assignable to type '(field: string, value: any) => this'.
    Types of parameters 'index' and 'field' are incompatible.
      Type 'string' is not assignable to type 'number'.

215     set(index: number, value: object): this;
        ~~~

node_modules/structurae/index.d.ts:216:5 - error TS2416: Property 'setView' in type 'ArrayView' is not assignable to the same property in base type 'ObjectView'.
  Type '(index: number, value: ObjectView) => this' is not assignable to type '(field: string, value: View) => this'.
    Types of parameters 'index' and 'field' are incompatible.
      Type 'string' is not assignable to type 'number'.

216     setView(index: number, value: ObjectView): this;
        ~~~~~~~

Found 3 errors.

I'm just using a very simple Pool and ObjectView with one string property. Hope this helps!

[zandaqo]

Hi, @SupremeTechnopriest!

Yes, the way ArrayView extended ObjectView was breaking the Liskov Substitution Principle. I thought I'd get away with it till the next refactoring, but I guess TypeScript isn't that forgiving, that's good to know. It should be fixed now, in 1.6.0. Thanks for bringing this up!

[SupremeTechnopriest]

@zandaqo Thank you kindly sir! Great job on this package.