Migrate Markdown to TypeScript #3618
Conversation
| @@ -23,7 +22,7 @@ const MarkdownRender = (props: ReactMarkdown.ReactMarkdownProps) => { | |||
| ...props.renderers, | |||
| math, | |||
| inlineMath | |||
| } | |||
| } as any | |||
There was a problem hiding this comment.
I added this any here to get around some type errors that were being thrown up in the ReactMarkdown component. As it turns out, the keys in the renderers object can only be one of the 16 or so node types defined in ReactMarkdown.NodeType. math and inlineMath are not in that set so it throws an error. I trued to expand the declaration of the react-markdown module but had trouble getting it working. I decided to just force it into an any type and call it a day here. Open to any advice on how we can work around this.
| export function blockPlugin(opts: Object) { | ||
| function blockTokenizer(eat, value, silent) { | ||
| export function blockPlugin(this: any, opts: Object) { | ||
| function blockTokenizer(eat: any, value: string, silent: boolean) { |
There was a problem hiding this comment.
I didn't know how to type eat here.
There was a problem hiding this comment.
Yeah I'm not sure either, this was borrowed and refactored code. @tlyon3
| @@ -10,8 +9,8 @@ const MIN_FENCE_COUNT = 2; | |||
| const CODE_INDENT_COUNT = 4; | |||
|
|
|||
| // eslint-disable-next-line no-unused-vars | |||
| export function blockPlugin(opts: Object) { | |||
| function blockTokenizer(eat, value, silent) { | |||
| export function blockPlugin(this: any, opts: Object) { | |||
There was a problem hiding this comment.
I added the type on this here to get around an implicit any that it was throwing inside the function.
| @@ -216,7 +215,7 @@ export function blockPlugin(opts: Object) { | |||
| // Stringify for math block | |||
| if (Compiler != null) { | |||
| const visitors = Compiler.prototype.visitors; | |||
| visitors.math = function(node) { | |||
| visitors.math = function(node: any) { | |||
There was a problem hiding this comment.
Once again, not sure what type node should be here.
There was a problem hiding this comment.
That's ok for now. We can have a de-any party someday.
| @@ -0,0 +1,3 @@ | |||
| declare module "trim-trailing-lines" { | |||
| export default function trim(value: string): string; | |||
There was a problem hiding this comment.
I added type definitions for the trim-trailing-lines library since there were none.
| <MathJax.Node>{props.value}</MathJax.Node> | ||
| ); | ||
|
|
||
| const inlineMath = (props: { value: string }) => ( | ||
| const inlineMath = (props: { value: string }): React.ReactNode => ( |
There was a problem hiding this comment.
I seem to recall from an earlier review that we don't have to declare the React.ReactNode return type here. Perhaps that was only for render() and not for functional components.
| export function blockPlugin(opts: Object) { | ||
| function blockTokenizer(eat, value, silent) { | ||
| export function blockPlugin(this: any, opts: Object) { | ||
| function blockTokenizer(eat: any, value: string, silent: boolean) { |
There was a problem hiding this comment.
Yeah I'm not sure either, this was borrowed and refactored code. @tlyon3
| @@ -216,7 +215,7 @@ export function blockPlugin(opts: Object) { | |||
| // Stringify for math block | |||
| if (Compiler != null) { | |||
| const visitors = Compiler.prototype.visitors; | |||
| visitors.math = function(node) { | |||
| visitors.math = function(node: any) { | |||
There was a problem hiding this comment.
That's ok for now. We can have a de-any party someday.
Closes #3572.
I left some comments in-line about the code.