Add types file #117
Add types file #117
Conversation
Codecov Report
@@ Coverage Diff @@
## master #117 +/- ##
=======================================
Coverage 99.21% 99.21%
=======================================
Files 12 12
Lines 507 507
Branches 162 162
=======================================
Hits 503 503
Misses 4 4
Continue to review full report at Codecov.
|
ccnixon
force-pushed
the
add-typefile
branch
3 times, most recently
from
5efdb22 to
1b82ab6
Compare
|
Can we re-use docs from the JS implementation? It looks like a lot of it is duplicated - which might cause them to go out of sync. |
index.d.ts
Outdated
| * for. Casing does not matter. | ||
| * @return {Object|undefined} | ||
| */ | ||
| options(integration?: string): object | void |
There was a problem hiding this comment.
why void? can object be undefined?
There was a problem hiding this comment.
Yep I think you're right though honestly it's tough to tell in that function.
| /** | ||
| * Get `groupId` from `context.groupId`. | ||
| */ | ||
| groupId(): unknown |
There was a problem hiding this comment.
Shouldn't this always be a string?
There was a problem hiding this comment.
No it just returns the value of the groupId property. Technically this could be anything the customer wants though it is usually a string.
| /** | ||
| * Get `sessionId / anonymousId`. | ||
| */ | ||
| anonymousId(): unknown |
There was a problem hiding this comment.
Same here, should this always be a string?
There was a problem hiding this comment.
It should be a string but it could be anything. Users have sent numbers here before.
| * Interesting values of `type` are `"ios"` and `"android"`, but other values | ||
| * are possible if the client is doing something unusual with `context.device`. | ||
| */ | ||
| device(): unknown |
There was a problem hiding this comment.
should this be object instead of unknown?
There was a problem hiding this comment.
Yes actually it looks like this is guaranteed to be an object.
| */ | ||
| device(): unknown | ||
| /** Get the User-Agent from `context.userAgent`. */ | ||
| userAgent(): unknown |
f2prateek
left a comment
f2prateek
left a comment
There was a problem hiding this comment.
there are a lot of files marked unknown - is that intentional?
|
Hey @f2prateek - yeah I intentionally put those as |
|
My thinking is that we shouldn't use our spec to define the return types if we can't guarantee it. This will make it painful to use with TS but I think we should solve it with tooling rather than just hope that the return value is what our spec says. |
|
If almost all the return types are unknown, what value does this offer (over the non-typed JS that we currently have)? |
|
Autocomplete methods for one. But most importantly it sets it up to be able
to be used in typescript projects.
On Fri, Apr 12, 2019 at 3:28 PM Prateek Srivastava ***@***.***> wrote:
If almost all the return types are unknown, what value does this offer
(over the non-typed JS that we currently have)?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#117 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AMW7jtGJTbzo53ikyPENOY4-71b52YBhks5vgQhygaJpZM4cjZP0>
.
--
*Christopher Nixon*
Software Engineer
Segment.com
|
Add a types file to make the module compatible with TS programs.