Skip to content

Add TypeScript typings#60

Merged
dcodeIO merged 4 commits intodcodeIO:masterfrom
andrezsanchez:ts
May 2, 2018
Merged

Add TypeScript typings#60
dcodeIO merged 4 commits intodcodeIO:masterfrom
andrezsanchez:ts

Conversation

@andrezsanchez
Copy link
Contributor

These come from the @types/long NPM package, originally written by Denis
Cappellin @cappellin, I only made a few modifications when I brought these over.

The dtslint tool lints the index.d.ts file and tests it by compiling (but not running) the test file, compiling using the tsconfig.json file. See the dtslint docs for more info on that.

I modified this because import Long from 'long'; was not working in a different project. I did not include the custom tslint file (didn't take the time to read the modifications; it's already being linted by dtslint). I'm pretty new to TS so there may be something wrong, but having it here should allow for a better feedback loop to correct any issues.

Andre Sanchez added 2 commits April 26, 2018 11:22
These come from the @types/long NPM package, originally written by Denis
Cappellin.
/**
* Returns this Long with bits logically shifted to the right by the given amount.
*/
shru(numBits: number | Long): Long;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one has an additional alias shr_u

/**
* Returns this Long modulo the specified.
*/
mod(other: Long | number | string): Long;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one has an additional rem alias

/**
* Tests if this Long's value is greater than or equal the specified's.
*/
gte(other: Long | number | string): boolean;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also ge

/**
* Tests if this Long's value is less than or equal the specified's.
*/
lte(other: Long | number | string): boolean;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also le

/**
* Tests if this Long's value differs from the specified's.
*/
neq(other: Long | number | string): boolean;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also ne

/**
* Tests if this Long's value equals zero.
*/
isZero(): boolean;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also eqz

types/index.d.ts Outdated
/**
* Converts the specified value to a Long.
*/
static fromValue(val: Long | number | string | {low: number, high: number, unsigned: boolean}): Long;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function accepts a second argument unsigned, see: https://github.com/dcodeIO/long.js/blob/master/src/long.js#L280

@@ -0,0 +1,377 @@
// TypeScript Version: 2.4
export default Long;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be that we have to add something around here to make this work with just the sources under node.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you mean by this. Do you mean when using ts-node?

Copy link
Owner

@dcodeIO dcodeIO Apr 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, the main file used under node is the source file, which just has module.exports = Long. The last time I checked interoperability with a default export requires some additional care there, like defining a property __esModule plus a default property, or something like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah I had to use esModuleInterop: true in my tsconfig.json to make this work. It'll probably be a webpack setting I'm assuming.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue is that the sources are served under node, so no webpack or tsc involved. Should probably manually add what tsc does with esModuleInterop: true to src/index.js.

Copy link
Contributor Author

@andrezsanchez andrezsanchez Apr 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no webpack or tsc involved

In that case I'm confused. How would this TypeScript declarations file be affecting Node?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to use esModuleInterop: true in my tsconfig.json to make this work.

Just to be clear, this was in reference to another TS project, not this one.

Copy link
Owner

@dcodeIO dcodeIO May 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I thought more in a sense of "polyfilling" something into src/long.js that would allow a loader using import Long from "long"; to actually work (it wouldn't currently, I think, erroring with 'there is no default export' or something). I haven't looked this up yet, but I think there might be a workaround like this in loaders if the exported object has an __esModule property plus a default property that indicates the default export - but I might as well be misunderstanding something here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I see. I think that can be addressed separately from this PR and anyone needing to use this can just use the esModuleInterop: true workaround for the time being.

@dcodeIO
Copy link
Owner

dcodeIO commented Apr 27, 2018

Looking good so far, thanks! Just a few comments, and let me also suggest to place typing at ./index.d.ts so that requiring the package by path will automatically provide the typings as well.

@andrezsanchez
Copy link
Contributor Author

let me also suggest to place typing at ./index.d.ts

SGTM. There was other stuff for testing in that folder, but I removed the testing code for the time being because of the Travis peer dependency issue.

@zensh
Copy link

zensh commented May 2, 2018

👍

@dcodeIO dcodeIO merged commit 95ac93e into dcodeIO:master May 2, 2018
@dcodeIO
Copy link
Owner

dcodeIO commented May 2, 2018

Thank you! :)

@zensh
Copy link

zensh commented May 2, 2018

@dcodeIO Could you release a new version please?

@Jameskmonger
Copy link

@dcodeIO Could you publish this please?

@amarzavery
Copy link
Contributor

@dcodeIO Can you please publish this? Current 4.0.0 version does not have the type definitions. What is the use of keeping the type definition in the repo and not publishing it to npm?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants