-
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
🐝: decouple annotation classes in renderer #269
Conversation
@@ -233,6 +233,22 @@ function getNumberOfRequiredBackticks(text: string) { | |||
}, 1); | |||
} | |||
|
|||
function isBold(annotation?: any) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type predicates may be very helpful here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that's what I was looking for. But now I'm not sure if this is the right approach. If we theoretically had access to document.schema.Bold
then it would be better to rely on that.
Tracking discussion can be found here: https://docs.google.com/document/d/e/2PACX-1vS-KB02JBJjVe_fl1w3w0ONjm8jLWlHH98kTqNIlxfriEcjGvtz1Nm8l2BT5Ea7vjT8Wq9NJh3i2T1D/pub |
I chatted with @bachbui briefly, but it has occurred to me that we could rewrite this for more maximal reuse: function is<T extends AnnotationConstructor>(
AnnotationClass: T,
annotation: Annotation<any>
): annotation is InstanceType<T> {
let OtherClass = annotation.constructor as AnnotationConstructor;
return annotation.type === AnnotationClass.type &&
OtherClass.vendorPrefix === AnnotationClass.vendorPrefix;
} |
This can be closed by the new |
Currently there is logic in the Commonmark Renderer that checks for specific annotation classes in the rendering logic. We should decouple this, as this means this renderer will only work with specific Sources. Instead, the renderer should expose its own types or interfaces for annotations rather than using the annotation classes from a source directly. This fix resolves the immediate issue instead of fleshing out the aforementioned feature, which can be done at a later date.