Rewrite in typescript #3
Conversation
There was a problem hiding this comment.
We don't have to worry about invalid values as long as we are using typescript. So I commented out some tests against them.
Should we....
- Keep the previous functionality(checking a parameter) and test in Javascript?
- Discard the useless code(because typescript ensure them) and test in Typescript?
I prefer 2. although it would be a breaking change.
|
|
||
| function stringify( | ||
| value: Unist.Node | Unist.Position | Unist.Point, | ||
| ): string | null { |
There was a problem hiding this comment.
If we choose 2., it can just return string.
| value: Unist.Node | Unist.Position | Unist.Point, | ||
| ): string | null { | ||
| /* Nothing. */ | ||
| if (!value || typeof value !== 'object') { |
There was a problem hiding this comment.
We don't need this checking.
| } | ||
|
|
||
| /* Point. */ | ||
| if (isPoint(value)) { |
There was a problem hiding this comment.
We don't need to check here neither because we've already check it is not a node or a position.
| } | ||
|
|
||
| /* ? */ | ||
| return null |
There was a problem hiding this comment.
We don't need this neither
| return null | ||
| } | ||
|
|
||
| function stringifyPoint(point: Unist.Point) { |
There was a problem hiding this comment.
I renamed function name because tslint blames me that the argument name and the previous function name were same.
| } | ||
|
|
||
| function stringifyPoint(point: Unist.Point) { | ||
| if (!point || typeof point !== 'object') { |
There was a problem hiding this comment.
We don't need to check this.
| } | ||
|
|
||
| return ( | ||
| convertIndexToNumber(point.line) + ':' + convertIndexToNumber(point.column) |
There was a problem hiding this comment.
It would be nicer to write with template string like
return `${point.line}:${point.column}`
| return stringifyPoint(pos.start) + '-' + stringifyPoint(pos.end) | ||
| } | ||
|
|
||
| function convertIndexToNumber(value: any): number { |
There was a problem hiding this comment.
We don't need this.
Many adopters are still using JavaScript. https://github.com/syntax-tree/unist-util-stringify-position/network/dependents |
|
Hmm.. Sounds reasonable. Actually all of adopters still using js because there is no type definitions for ts. I think we need more time. Then, I'll keep adding type definitions to DefinitelyTyped so lots of people could start using typescript. After lots of adopters using typescript, we might be able to discard runtime check. |
|
I'll close this because it won't be merged soon! I'll be back with type definitions. |
I tried to rewrite in typescript. This is not done yet because we need to deal with some issues. And I don't mind if it is not merged immediately.
FYI, you can run the test by: