Refactor to TypeScript#61
Conversation
| export = rangeParser; | ||
|
|
||
| namespace rangeParser { | ||
| export interface Range { | ||
| start: number; | ||
| end: number; | ||
| } | ||
|
|
||
| export interface Result extends Array<Range> { | ||
| type: string; | ||
| } | ||
|
|
||
| export interface Options { | ||
| combine?: boolean; | ||
| } | ||
| } |
There was a problem hiding this comment.
Is the namespace necessary here, or is it enough to just export it?
There was a problem hiding this comment.
You can't export anything extra because I left it in CommonJS format with export =.
There was a problem hiding this comment.
I tried not to refactor too much in this PR and left that for the perf one, this one is more of a direct port with types added.
55ff729 to
d94bad8
Compare
d94bad8 to
391481c
Compare
|
Basically, what I found is that we should add #66 and keep the changes we made in #57 to remain strict with the spec. So any changes after that which were merged into |
Alternative to #59. I prefer to author in TypeScript and skip the infrastructure around testing types independently. Uses the same scripts as
path-to-regexpfor automatic formatting and testing.This PR doesn't break backward compatibility in the API (but does in node.js version support). It minimally changes the code and tests to add types and test using
vitest.I plan to add benchmarks (using the built-in
vitestframework) and optimize the code paths (similar to the work done forcookie) in further PRs. I think there's at least a 50% bump available, and that may require minor API changes (e.g. avoid assigning to an object).