refine types: arrays may be readonly

[elmarx]

Hey,

I started using this fine module in a typescript project with lint-rules that enforce the usage of readonly-arrays. Chokidar's types expects mutable arrays, and checking the source-code readonly-arrays should be totally ok, so it's only the types that are too broad.

[paulmillr]

You've replaced mutable arrays with read-only arrays. But while read-only arrays are subset of generic arrays, generic arrays do not inherit from read-only arrays.

Right now we have a general case. You've reducing this to read-only-arrays only. In fact, on two lines you're explicitly casting types using as. This should rarely be done, if ever! I use it only for cases when the compiler is stupid enough to inherit types on its own.

[elmarx]

Just to be sure we're talking about the same thing: "generic arrays" and "mutable arrays" are the same, right?

Since readonly arrays are generic arrays (the simple [] in JS) with some methods removed (thus a subset, as you mention) at type-checking-time, I agree that generic arrays do not inherit from readonly-arrays, because they are the exact same at runtime. And even if they would be implemented differently, as long as they behave the same at runtime (duck-typing) they work the same, and TSC is also fine, given it's structural typing.

The current solution is a general case, but could be even more generic, specifing in the type-definition, that the add/watch methods etc. will just use the subset specified in ReadonlyArray.
In fact, I did not modify your test that use watcher.add() with a generic array, since that of course still matches the required structure of a readonly array.

I agree that using as in TypeScript to "force cast" something is not good. It's just the question how to best write a literal readonly array, I thought this is test-code with very limited scope, just to ensure that the changes to the type-definitions actually do something.
TypeScript 3.4 introduced as const which I would prefer for literal ReadonlyArray, but it seems as if dtslint does not (yet?) understand the syntax. Another option is to introduce an explicitly typed variable.
But I have no strong feelings about code style here, if you maybe decide to accept readonly arrays nonetheless, I'm happy to change the PR as you request.

[paulmillr]

So what i’m trying to understand here is Typescript type hierarchy.

ReadOnlyArray inherits from Array
Array does not inherit from ReadOnlyArray

Which means we will receive type errors when compiling this code: watcher.add([“path”]). Right?

[elmarx]

I see. Well typescript does not use type hierarchy. What is relevant is what I briefly mentioned as "structural typing". TSC sees a type-requirement and checks if the argument fullfills the required interface.
"Inheritance" or "extension" of types in TypeScript is more a tool to reduce code to write, not to define a type hierarchy.

Thus

Which means we will receive type errors when compiling this code: watcher.add([“path”]). Right?

is not right. watche.add(f: ReadonlyArray<string>) requires an argument that has some methods, e.g. find() — ["path"] fulfills this requirement (and has also some more methods, but they are not required), that's enough for TSC to pass the typecheck.

I put together a simple example to further demonstrate that.

I also updated the pull-request to be more verbose and show that really generic arrays still work. :)

[paulmillr]

Weird. I can’t reopen the PR. Could you create a new one?

[elmarx]

Sure, thanks for taking the time to discuss this issue, I really appreciate it :)