dstotijn / go-notion

Go client for the Notion API.
MIT License
379 stars 40 forks source link

Consider typing interface{} fields? #20

Open abustany opened 2 years ago

abustany commented 2 years ago

First of all, thanks for the work you're putting into this!

There are couple of places in the Go API where the type of a field is only known at runtime (eg. Page.Properties), so the field is typed as interface{} in Go. One approach that's used eg. by the protobuf runtime is to use an interface with a "dummy" method that uniquely identifies the implementors, which helps both IDEs/language servers to do autocompletion and the compiler to ensure that the code is correct.

A simple example

type Page struct {
   Properties IsPageProperties
}

type IsPageProperties interface { isPageProperties() }

type PageProperties struct { /* ... */ }

func (PageProperties) isPageProperties() {}

type DatabasePageProperties struct {  /* ... */  }

func (DatabasePageProperties) isPageProperties() {}

let me know if you think this approach makes sense, and I can try to send a PR.

dstotijn commented 2 years ago

Hey @abustany, I'll have time from next week onwards to look into this. Will follow up then with my thoughts 👍

dstotijn commented 2 years ago

Long overdue update on this: There were some changes in Notion API's version 2022-06-28 that allowed removing the blank interface for Page.Properties (ref). But because Notion reverted that change (ref) I also reverted (ref).

Long story short: This issue is still relevant, and indeed, I agree interface{} should be avoided/removed. I'm not 100% sure yet about the "dummy" method interface though. In the meantime, I'm afraid users of this client will have to do some type assertion/manual labour to figure out what types to use.