recogito / text-annotator-js

A JavaScript library for text annotation.
BSD 3-Clause "New" or "Revised" License
20 stars 7 forks source link

Add ability to provide generic for the internal type of the `TextAnnotator` #64

Closed oleksandr-danylchenko closed 1 month ago

oleksandr-danylchenko commented 8 months ago

Current State

Currently, the TextAnnotator allows to provide a generic only for the external type: https://github.com/recogito/text-annotator-js/blob/3f587dd752084f3f6c291dec0577e01afb2d1e4d/packages/text-annotator/src/TextAnnotator.ts#L13 And everywhere in the following code, only the TextAnnotation is hard-binded

Issue

When a user (me 😅) wants to extend the TextAnnotator with some plugin that might mutate the TextAnnotation with additional properties - I'm pretty much out of luck... Because the TextAnnotator, TextAnnotatorState, are strictly bound to the R6O TextAnnotation type.

The one option is to define the .d.ts file and override the TextAnnotation there. But it creates inconvenient indirectness and maintainers will have questions...

However, another better alternative is to make the TextAnnotator look something like this:

export interface TextAnnotator<I extends TextAnnotation = TextAnnotation, E extends unknown = TextAnnotation> extends Annotator<I, E> {

  element: HTMLElement;

  // Returns true if successful (or false if the annotation is not currently rendered)
  scrollIntoView(annotation: I): boolean;

  state: TextAnnotatorState<I>;

}

In that way - lib consumers can safely describe the extended internal types. And then simply use the typed useAnnotator and useAnnotatorStore hooks

oleksandr-danylchenko commented 8 months ago

I also tried following the RecogitoTEIAnnotator example: https://github.com/recogito/text-annotator-js/blob/3a70b9ef5cafec4b54ed629e934e5875d6f06262/packages/extension-tei/src/TEIPlugin.ts#L34-L36 But I think that using the extends Annotator for the plugin that works with the TextAnnotator won't be a precise type depiction. Because the latter adds new props (element, scrollIntoView, etc.) and overrides some of the lifecycle methods of the store (addAnnotation, getAnnotationBounds, etc.). Unfortunately, the extended Annotator won't have any of those... Instead, I would like to be able to use the TextAnnotator type directly with a slightly updated internal model. Which cannot be done at the moment 🤷🏻‍♂️

oleksandr-danylchenko commented 1 month ago

The required changes were implemented in https://github.com/recogito/text-annotator-js/commit/876612f9089b0fdded5cab0624ead266e4c143db & https://github.com/recogito/text-annotator-js/commit/4abc32dc469f19e101e69d13c038690de5584f37