Closed Wtango closed 4 years ago
Yes, please! :)
I've just written some typings for this. 🙂 See https://github.com/DefinitelyTyped/DefinitelyTyped/pull/42333.
@treybrisbane That's really great, thanks! I will test them as soon as they got merged :-)
This past weekend I rewrote the whole library as Typescript. I've still got to rewrite examples and such so it isn't versioned up yet. Take a look and let me know if you experience any problems.
npm i json-ptr@1.2.0-ts.da6e2a1
It is definitely typed.
@flitbit Wait... Seriously? 😆
Well I'm guessing I should go decline my DefinitelyTyped PR? What's the ETA on the TypeScript release?
In case it's relevant, I did some stuff with conditional types to make some return types dependent on arguments. Maybe take a brief look and see if you can use anything? I tend to be pretty meticulous when typing stuff, so maybe it'll be useful. 🙂
@flitbit I've taken a look at the TS branch, and it's looking pretty solid so far! 🙂
I definitely think we should tighten up the typings though, as they're currently very loose. Some improvements we could make are:
unknown
instead of any
where ever possible
unknown
means "could be any type, but you must explicitly check which type it is before you do anything with it", whereas any
means "could be any type, you can do anything with it, and the compiler won't do any type checking at all". As a result, unknown
is greatly preferred due to its vastly better safety.PointerPair
type into separate JSON pointer and URI fragment types, and choosing which one to return based on the value of the fragmentId
argument
fragmentId
isn't passed (that returns the JSON pointer type), one where fragmentId
is passed but is typed as false
(that also returns the JSON pointer type), and one where fragmentId
is passed and typed as true
(that returns the URI fragment type).Record
type (example) rather than defining your own Dict
interfaceR
types here, here, and here, and the V
type hereisReference
(example)string
everywhere
PathSegments
, JsonStringPointer
, UriFragmentIdentifierPointer
, and Pointer
A couple of misc. other things that could also be improved are:
toString
methods for each of JsonPointer
and JsonReference
inside the respective classes, rather than as assignments to the class prototype
descendingVisit
functionHappy to submit PRs for any of this stuff to help get it over the line! 🙂
I appreciate the commentary. I'm only part way in on typescript so I really like your more seasoned analysis of this attempt.
Of your notes, the Record
type had entered my brain-hole just last week, saw it in some code and went "ah ha!" -- I was trying to remember where I put that Dict
type... now I know!
I would welcome the PR. Seriously dude, I appreciate the pointers (pun). I won't have time in my schedule to look again before next week, but hope to see it!
@treybrisbane I've updated following along most of your typescript suggestions; LMK.
BTW, the "this thread" link goes to nowhere (rejected by the server). I really wanted to check that one out.
Apologies for the delay; I've had a busy week! I'll aim to take a look at your changes tomorrow evening.
As far as the link goes, it was a link to a tweet by Ryan Cavanaugh. Try this: https://twitter.com/SeaRyanC/status/1205170798282960896?s=20 (you'll probably have to then click on the tweet itself to see the various replies in which he elaborates)
Apologies again for the delay!
I've put up this PR with a few improvements. Please take a look at it when you get a few minutes and let me know what you think. 🙂
Supported at v1.3.0
This project is so helpful, it's better to provide the @type for using in typescript.