-
Notifications
You must be signed in to change notification settings - Fork 13
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
Document.annotations.in(<schema>) #183
Comments
This is wonderful! I'd be interested in what the shape of a schema looks like. Is something like the following reasonable? interface Schema {
type: string;
version: string;
annotations: {
[key: string]: typeof Annotation
];
} Also, extending from this is a series of questions around code organization and how to define converters / |
Some examples of how this would be used in the wild would be lovely too! Is this the correct invocation for something? let doc = new Document({
content: 'Hello, world',
annotations: [{
type: '-commonmark-p',
start: 0,
end: 12
}]
}).in(CommonmarkSchema); |
@tim-evans that schema interface looks chill to me. regarding I'm interested if people think there are any operations that currently live on the document that would make more sense moved to the annotations? |
If you feel comfortable, could you define a general interface for Document and Collection so there's more of a bird's eye view of what would be affected? (This is an example of what I would find most helpful:) interface Schema {
}
export class Document<S extends Schema> {
annotations: Collection<S>;
new(): Document<S>;
in<T extends Schema>(schema: T): Document<T>;
convertTo<T extends Schema>(schema: T): Document<T>;
}
export class Collection<S extends Schema> {
add();
remove();
replace();
} // Equivalent old / new APIs
// -or-
// What old APIs would de-sugar to in terms of new APIs |
I've created a working example of this in the TypeScript Playground |
Ok, so I've worked through this a bit and this requires a lot of moving parts, but there's a few things that we can do in preparation to start this refactor to minimize large changes everywhere:
Unfortunately, the rest of the changes will require some level of coordination or API changes that require different API signatures. |
Also, here's a list of breaking changes that will likely be occurring in this proposal:
|
This feature is a bit... all encompassing. It is a major improvement to the core api, but definitely touches a whole load of stuff. To make these changes reasonably reviewable, we'll need to break it up into several steps. I think it makes the most sense to merge all of these changes into a There's a lot of ways to approach this, but I'm going to give some suggestions on what may be the most logical approach: Change the
|
Right now the Document is the primary typed entity in atjson, which works fairly well in practice but is conceptually a little confusing. When we convert documents between different sources what we're really converting is the collection of annotations associated with that document. I think it would be conceptually simpler if the notion of a type or annotation schema existed primarily at the level of the annotation.
I propose adding a few types to the library:
AnnotationCollection<T extends AnnotationSchema>
with public membersAnnotationCollection<T>.convertTo(schema: <U extends AnnotationSchema>) : AnnotationCollection<U>
andAnnotationCollection<T>.in(schema: <U extends AnnotationSchema>) : AnnotationCollection<U>
convertTo(schema: <U extends AnnotationSchema> : AnnotationCollection<U>
isDocument.convertTo
, just scoped to work on a list of annotations insteadin(schema: <U extends AnnotationSchema>) : AnnotationCollection<U>
returns a new AnnotationCollection where any unknown annotations inschema
from the original collection are made known, and all other annotations are made unknown.AnnotationSchema
is the parent type of schema definitions. This would take over much of the current role occupied byDocument
and its subtypes. Converters would be defined between subtypes of AnnotationSchema rather than subtypes of Document. They would also obviously 'own' the definition of their annotations, and would have a content type string for marking their annotations during de/serialization.This proposal doesn't give the library any additional power (since a document is just a piece of un-annotated media and an AnnotationCollection, and the media portion doesn't at all complicate the associated schema) but I think it would be helpful for explaining the library and would help generally separate independent concerns within the system
The text was updated successfully, but these errors were encountered: