Conversation
kriszyp
left a comment
There was a problem hiding this comment.
Lots of great stuff, thank you for working on this!
resources/TableInterface.ts
Outdated
| import { Context, Id, ResourceInterface, type ResourceStaticInterface } from './ResourceInterface.ts'; | ||
| import { DBI } from './DatabaseInterface.ts'; | ||
|
|
||
| export interface Table<Record extends object = any> extends TableInterface<Record> { |
There was a problem hiding this comment.
Are these all things we are exposing in our public interface, or things that we have declare internally for consistency? We certainly don't document these nor ever want users accessing most of these.
There was a problem hiding this comment.
We should only put stuff in that we want to expose, IMO
There was a problem hiding this comment.
I'm going to backtrack on my prior comment -- these interfaces are actually a mixture of the internal interface, and the external interface. I haven't been able to fully wrap my head around how we'd want to differentiate the two yet. Arguably there are actually three separate interfaces that show up in our code:
- The fully internal table interface (aka
typeof TableResource) which is used inside of table.ts mostly - The inside-our-harper-codebase interface
- The external interface that consumers can call from their applications and extensions
🤔
cb1kenobi
left a comment
There was a problem hiding this comment.
Nice! Wish we had these types a long time ago! Couple of small nits that you can accept/ignore/punt.
7d7872b to
caebdc4
Compare
kriszyp
left a comment
There was a problem hiding this comment.
Is this kinda still in draft? Do you want me to put together a list of desired exposed properties/methods on tables?
| import type { Context, Id, ResourceInterface, ResourceStaticInterface } from './ResourceInterface.ts'; | ||
| import type { DBI } from './DatabaseInterface.ts'; | ||
|
|
||
| export interface TableInterface<Record extends object = any> extends ResourceInterface<Record> { |
There was a problem hiding this comment.
I don't think any of these are table instance methods or properties except getExpiresAt() and getUpdatedTime().
| remove: (valueToRemove, id: Id, options?: unknown) => void; | ||
| } | ||
|
|
||
| export interface TableStaticInterface<Record extends object = any> extends ResourceStaticInterface<Record> { |
There was a problem hiding this comment.
I think we probably want to expose these properties:
- attributes
- createdTimeProperty
- databaseName
- expirationMS
- getResidencyById
- primaryKey
- replicate
- schemaDefined
- sealed
- tableName
- updatedTimeProperty
And methods:
- addAttributes
- coerceId
- getRecordCount
- getSize
- getNewId
- getStorageStats
- removeAttributes
- setTTLExpiration
- sourcedFrom
f0a6ad5 to
b68e814
Compare
I did a few things here:
shardin the config-root.schema.jsonWhy? Well, it will more easily describe our tables and resources for consumers. I shifted the
tablesanddatabasesto point people at these interfaces instead of at the return value of makeTable. These are a lot easier for IDEs, agents, and humans to read as a result. It will also let people guard their types when using static methods themselves.Here's the question, though: what should be described in the interfaces for statics? What shouldn't be described, because its an internal implementation detail?