Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider typing interface{} fields? #20

Open
abustany opened this issue Nov 15, 2021 · 2 comments
Open

Consider typing interface{} fields? #20

abustany opened this issue Nov 15, 2021 · 2 comments

Comments

@abustany
Copy link

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
Copy link
Owner

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

@dstotijn
Copy link
Owner

dstotijn commented Sep 7, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants