fix($types): support array index

[andreiglingeanu]

Temporary fix for #84.

Here's a VERY bad way of allowing nested array indexing in queries. Here's a very rough reproduction of the problem for the TypeScript experts that may be able to help us get a more type-safe solution for this problem.

---

There definitely should be a way of not loosing the type data here. I imagine the solution to look like (imaginary syntax that doesn't exist in TypeScript :) ):

type Tree<T> = 
  // If is object
  {[K in keyof T]?: Query<T[K]>}
  |
  // if is array
  {[I in indexesOf T]?: Query<T[I]>}

Is there something like I in indexesOf T in TypeScript? I imagine there's no way of getting such information from just static analysis.

cc. @kolodny @andy-ms

[coveralls]

Coverage remained the same at 100.0% when pulling b4cf9dc on hotfix-for-array-type into 44772d2 on master.

[coveralls]

Coverage remained the same at 100.0% when pulling 508abc4 on hotfix-for-array-type into 44772d2 on master.

[kolodny]

Looks good to me, although I'm still not sure how to go about testing and verifying this. I would also wait to hear back from @sohymg or @mihai-dinculescu that this fixes their issue

[andreiglingeanu]

I managed to actually reproduce this on my end. Very strange that you actually didn't

[sohymg]

This works but seems a bit too loose. How about the using old typing https://github.com/DefinitelyTyped/DefinitelyTyped/pull/22763/files#diff-103618e6d9fa598ca6066092c37ee029L24 or this:

 type ArrayOperators<T> =
   | {$push: T}
   | {$unshift: T}
   | {$splice: Array<[number, number]>}
   | {[customCommand: string]: any}    // <---- new line

[lxcid]

I'm having issue with $unset though…

[andreiglingeanu]

@lxcid Can you please describe the issue a bit more?

[ghost]

I think any in this case could be replaced with { [key: number]: Query<T> }.
However, example from #84 used { $splice: [[1, 1, 13, 14]] } which is currently not a valid query, since the types declare {$splice: Array<[number, number]>} which doesn't allow for 4 element arrays. Either the type is wrong, or the example should use [[1, 1], [13, 14]] instead.

[dblick182]

Additionally, I don't think this change is sufficient to fix all breaking changes.

For example, given the following interfaces, the update below is valid given v2.6.2, but not 2.6.3+, and the proposed change does not fix this case (presuming dataSource is an instance of IFDSource):

Query: update(dataSource, {sources:{0:{definition:{0:{$set: newDefinition}}, foo: {$set: "junk"}}}});

export interface IFDSource {
    sources: Array<IFSource>;
}

export interface IFSource {
    foo: string;
    definition: Array<IFDSourceDefinition>;
}

export interface IFDSourceDefinition {
    x: string;
    source: string;
}

However, the solution proposed by @sohymg does fix this issue.

[ghost]

I asked @sandersn and the following seems to be a design limitation of the type system:

declare function f<T>(obj: T, key: keyof T): void;
f(["a", "b", "c"], 0); // Apparently this is an error

[sandersn]

Yes, see discussion at microsoft/TypeScript#18346. It's likely that we'll revisit this issue eventually.

[andreiglingeanu]

So, what do you think would be a final solution to this problem? I would assume we'll loose a lot of type safety along the way, but it is what it is.

[andreiglingeanu]

That's a game changer for us: microsoft/TypeScript#21316

[andreiglingeanu]

@sohymg @dblick182 could you please test if the current types will help, until a more type safe solution will land?

We can address $splice in a separated PR afterwards