Map WebIDL object to TypeScript object

[RReverser]

This type on the TS side is relatively new, which is probably why it wasn't used in the first place, but gives more precise definitions and, so, better type checking.

As a side-effect, fixes #290 (although in a different way).

[orta]

@saschanaz - do you have a general opinion about tightening the types like this?

If these are new APIs, I'm OK with tightening it I think

[RReverser]

@orta These are not new APIs, but it is a relatively new type in TypeScript, and I think it makes sense to use it as it's more precise than any and more correct for these cases (that is, it won't break any valid code, but will catch more mistakes).

[saschanaz]

it won't break any valid code

It will, because this changes property types and return types. I'm afraid that this might break codes in scary way,..

I wonder whether we can first convert argument and dictionary field types, as this certainly provides better type check for them.

[RReverser]

It will, because this changes property types and return types. I'm afraid that this might break codes in scary way,..

If someone already used any there (as the definitions were defined before), then it shouldn't break code, because new object will be still backward compatible with that any.

If they assumed a different type (e.g. var x: number = something.toJSON();), then it will just help catch more bugs.

[saschanaz]

object does not allow arbitrary property access like something.toJSON().someExpectedProperty, so any code that currently do so will be broken after this change.

Fortunately there are not many other APIs that return object though, per the changes. (And not sure why anyone would do toJSON().arbitrary)

[RReverser]

Yeah, there are not that many aside from toJSON and new WebAssembly APIs which I just updated / added in the recent PR.

[saschanaz]

The only valid use case I could find is WebAssembly.Instance.exports.(arbitrary). It's a new type, so maybe we could do this change safely.

[RReverser]

@saschanaz I'm actually planning to revamp WebAssembly import/export APIs separately in a follow-up PR to turn them into dictionaries with precise allowed types (since not everything can be imported / exported to Wasm).

[RReverser]

Although I'm starting to think that you're right. Maybe it's worth turning just parameters to object for now - that should be definitely safe, and then we can look at returns more precisely...

[RReverser]

I'm not sure anymore TBH. The .exports case should probably still allow arbitrary access until it's defined more precisely, so it threw me off the initial feeling of safety...

[RReverser]

I'm starting to think that, instead of an object, it would be better to use Record<string, any> all around here, as it allows dot-access while still checking that the value is an object and not any other primitive (string, number, null, etc.).

@saschanaz what do you think?

[sandersn]

I don't like the readability of Record<string, any> compared to object or any. This is important because VS Code's quick info will show up for JS users as well as TS users.

I vote to just change parameter types, not return types.

[sandersn]

I’m going to close this since it’s now kind of stale.

I’m still not convinced of the value versus readability of Record<string, any>, so that’s the first thing we’d need to establish before re-opening.

[RReverser]

@sandersn Oh, sorry for never circling back on this. I can definitely see your point about readability.

I wonder if something like this would help?

interface AnyObject {
    [key: string]: any;
}
// ...
declare function f(x: AnyObject): AnyObject;

This way 1) f shows AnyObject (name subject to change) in parameters and return types, but still 2) accepts only valid objects and not arbitrary values and 3) allows to get properties on the result and so on.