Update type declaration for `change` function for consistency

[PaulMorris]

See https://github.com/automerge/automerge.github.io/issues/16 for a description of the issue.

Make the change function work like the other type declarations (like applyChanges, applyPatch, etc.) where the user just specifies the plain type T as a type param and the function returns a Doc<T> (which is shorthand for a FreezeObject<T>).

For consistency it looks like the same change could also be applied to getHistory and maybe emptyChange (but I haven't used those yet, so not sure). Then that Proxy<T> type could potentially be removed. I think what it's trying to convey can be conveyed better in other ways.

[ept]

Hi @PaulMorris, thank you for this. I'm not much of a TypeScript user myself so I'm not in a position to review this change. @pvh could you help me out here?

[scotttrinh]

This change seems correct to me, especially since the former generic D had no restrictions at the type level.

[pvh]

I agree with the premise of the patch, and it appears fine to me but I haven't confirmed that it won't break existing code. If it does it seems like it would still mostly just be a minor cleanup and the result would be preferable.

[scotttrinh]

Yeah, I guess it's hard to say if anyone was ever actually setting this generic or relying on type inference to get it from the passed in document. Probably something that can be added to the typescript test suite to test the level of breakage? 🤔

[dan-weaver]

I always assumed the change callback signature used Proxy as the type for the callback argument because proxy actually is slightly different even though it’s current type declaration doesn’t actually currently reflect that.

For instance every array in the Proxy actually has additional methods (insertAt, deleteAt). If the change callback only reflects the POJO type given by the user there’s no future opportunity to type it based on anything the Proxy type would augment that base type with.

Just a small note, otherwise seems good!

[scotttrinh]

@dan-weaver

I think the philosophy so far has been to avoid adding instance methods to T to avoid conflicts with potential user-defined property names. In the case of using List, that usually is used within the T type, right? So something like:

type DirectoryDocument = Automerge.Doc<{ people: Automerge.List<Person> }>;

[dan-weaver]

I think that’s a good point irt the instance methods / proper use of List type. Thanks! Agree.

[ept]

Okay, if there are no concerns then I will merge this patch, and if we find that it breaks something we can reconsider.

[ept]

In ad5104200c5bb0e6525f30a90d100eee26fa927b I also removed the Proxy<T> type from getHistory as it doesn't seem to be necessary (there are TypeScript tests that exercise getHistory and they still pass)