Empty interfaces on some properties types.

[jhchabran]

While using the lib, I noticed that a bunch of properties have some fields typed with interface{}:

• DateProperty.Date
• PeopleProperty.People
• CheckboxProperty.Checkbox
• ...

While technically, this doesn't prevent to use the API, it still shifts the work on the library user to handle those fields. Is there a particular reason motivating this choice?

From my understanding so far, the API data structures for these fields are rather straightforward, thus it should be simple to handle those cases.

I don't want to meddle with unwanted PRs, so I thought it'd be best to first discuss things here!

[jomei]

Hi, @jhchabran !

The documentation says:

Date database property objects have no additional configuration within the date property.

As I understood, that means DateProperty.Date is just an empty struct. I've decided to use interface{} instead of struct{} here to leave the opportunity to handle cases with documentation errors or some API changes(because the API is still in beta). I understand it is not a good lib design, but couldn't find a better solution.

What do you think?

[jhchabran]

Oh thanks for the prompt response @jomei! Yes, I assumed you knew about the interface{} approach not being practical, I thought I might have missed some context on my side to understand your decision.

I think I start to see from where the confusion is coming. There are two definitions for the date property:

• The property configuration, which is almost empty, as per https://developers.notion.com/reference/database#date-configuration (which is what you quoted).
• https://developers.notion.com/reference/page#date-property-values which has a more fields and is meant to set a column to a given value.

The same pattern applies for other properties. Because the current code includes a SelectOptionProperty, I thought that those two cases were covered by the library, but that appear to not be the case.

I think that from a clarity point of view, it'd be better to separate explicitly the two family of structures: *PropertyConfiguration and *PropertyValue. So Database.Get would return types of the former and Page.Get|Update would use the latter.

WDYT ?

[jhchabran]

Another example of this problem is the code is submitted in #6. Without the fix, it's not possible to perform a Page.Get on a database which has a column whose type is a RelationProperty. But what I missd while mahing the change is that it breaks the property side.

Because both the configuration and value code are using the same struct under the hood, it's not possible to make both work at the same time with the current code.

[jomei]

Oh, I've caught the issue finally...The fix is on the way!

[jhchabran]

@jomei Feel free to ping if you want any help or just to ping pong stuff, I'd be happy to help 💪

[jomei]

@jhchabran could you please check out #9. The code review will be very much appreciated =)