TypeScript definitions file

[JamesSkemp]

Is your feature request related to a problem? Please describe.
No. This is an enhancement request.

Describe the solution you'd like
I recently discovered this library and would love to use it in a project that uses TypeScript. To best enable this it would be great if typings were available.

Describe alternatives you've considered
I plan on working on a TypeScript definition file for my own use, but if there's interest, think it would benefit this project if it were included.

Additional context
My plan is to create the initial TypeScript definitions via dts-gen (already done) and then update them based upon this project's actual documentation. I would then be testing it in my own project.

I understand that TypeScript definitions may not appeal to you, depending upon your own usage and target audience, and since it does require updating if the library changes, so not a big deal if you're not interested. Just want to gauge if there's interest as that will determine how much effort I put into this.

Thanks for the great library!

[JamesSkemp]

Belatedly, saved as a gist to https://gist.github.com/JamesSkemp/d090ba39a53853841b8dc0ecc7d78887

Still in-progress but working pretty good in a private project I'm working on.

More than willing to create a merge request with this if it would be helpful.

[GreenImp]

Hi @JamesSkemp . I've not used Typescript myself yet, but am interested in it.

I had a look at your Gist, seems interesting. Is it a case of listing everything that the library exports and what type it is?
Is it possible to add tests for this, to ensure that it's correct? (I'm not sure how you would, genuinely curious).

I'm happy to accept a pull request into the develop branch. If it's possible to add tests for it, then please do that first, if not, no worries. Once requested I'll get it merged in :)

Thank you very much for the interest and help! Really appreciated.

[JamesSkemp]

Awesome, thanks @GreenImp :)

Yes, since TypeScript is a strongly-typed language, a definitions/declaration file helps with not only what the library provides for functionality, but also what types they accept and return, and then provides some nice type safety while writing code.

I know the Phaser framework has some basic TypeScript tests, so it should be completely possible to do so.

I've started a fork and have the types brought over, but don't have the simple test working yet, probably due to the namespace I'm using. I'll work on it over the next few days and create a merge request when it's in a working state.

(I've been working with TypeScript for a few years, but this is my first time adding support to another library, so time estimate very approximate. :) )

[GreenImp]

Cheers. I would rather have code coverage if possible, but if it proves to be untestable then that's fair enough.
It'll be good to have something like this in the project :)

[JamesSkemp]

Sorry for the delay on this, it's been difficult to find time for any personal projects over the last week and a half. :/

I have a branch - https://github.com/JamesSkemp/rpg-dice-roller/tree/typescript-defs-wip-1 - that I had put some code changes into (with the hope that once I figure things out I would cherry pick or otherwise redo the changes).

npm run test-ts is throwing an error at the moment, which is where I stopped. I've been using https://github.com/photonstorm/phaser as a guideline.

[JamesSkemp]

So the issue I'm running into is having node test the generated JavaScript.

If I generate the JavaScript file and manually change the third line to the following, I'm still getting an import error with node tests.js.

var index_1 = require("../../src/index");

[GreenImp]

Is it the scripts/src/tests.ts file where you're getting the error? on:

import { DiceRoller, DiceRoll } from "../../types/index"

What's the import error? And do you need to use the full filename, types/index.d.ts (Assuming that's the file you're trying to import)?

[JamesSkemp]

Sorry for the delay, it's been a crazy week. :(

In all cases TypeScript works, but running the test with node tests.js fails.

If I have

/// <reference path="../../types/index.d.ts" />
let diceRoller: rpgDiceRoller.DiceRoller = new rpgDiceRoller.DiceRoller();
const roll = diceRoller.roll("3d6");
console.log(roll);

let diceRoll: rpgDiceRoller.DiceRoll = new rpgDiceRoller.DiceRoll("3d6");
console.log(diceRoll.roll());

It generates

/// <reference path="../../types/index.d.ts" />
var diceRoller = new rpgDiceRoller.DiceRoller();
var roll = diceRoller.roll("3d6");
console.log(roll);
var diceRoll = new rpgDiceRoller.DiceRoll("3d6");
console.log(diceRoll.roll());

Which errors with

internal/modules/cjs/loader.js:582
throw err;
^

Error: Cannot find module '../../types/index'
at Function.Module._resolveFilename (internal/modules/cjs/loader.js:580:15)
at Function.Module._load (internal/modules/cjs/loader.js:506:25)
at Module.require (internal/modules/cjs/loader.js:636:17)
at require (internal/modules/cjs/helpers.js:20:18)
at Object. (/Users/james/Projects/rpg-dice-roller/scripts/src/tests.js:12:15)
at Module._compile (internal/modules/cjs/loader.js:688:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:699:10)
at Module.load (internal/modules/cjs/loader.js:598:32)
at tryModuleLoad (internal/modules/cjs/loader.js:537:12)
at Function.Module._load (internal/modules/cjs/loader.js:529:3)

If I have

import { DiceRoller, DiceRoll } from "../../types/index"
let diceRoller: rpgDiceRoller.DiceRoller = new DiceRoller();
const roll = diceRoller.roll("3d6");
console.log(roll);

let diceRoll: rpgDiceRoller.DiceRoll = new DiceRoll("3d6");
console.log(diceRoll.roll());

It generates

"use strict";
exports.__esModule = true;
var index_1 = require("../../types/index");
var diceRoller = new index_1.DiceRoller();
var roll = diceRoller.roll("3d6");
console.log(roll);
var diceRoll = new index_1.DiceRoll("3d6");
console.log(diceRoll.roll());

Which errors with

var diceRoller = new rpgDiceRoller.DiceRoller();
^

ReferenceError: rpgDiceRoller is not defined
at Object. (/Users/james/Projects/rpg-dice-roller/scripts/src/tests.js:2:18)
at Module._compile (internal/modules/cjs/loader.js:688:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:699:10)
at Module.load (internal/modules/cjs/loader.js:598:32)
at tryModuleLoad (internal/modules/cjs/loader.js:537:12)
at Function.Module._load (internal/modules/cjs/loader.js:529:3)
at Function.Module.runMain (internal/modules/cjs/loader.js:741:12)
at startup (internal/bootstrap/node.js:285:19)
at bootstrapNodeJSCore (internal/bootstrap/node.js:739:3)

This makes sense since there's no JS in the types directory. However, if I manually change the tests.js to:

"use strict";
exports.__esModule = true;
var index_1 = require("../../src/index");
var diceRoller = new index_1.DiceRoller();
var roll = diceRoller.roll("3d6");
console.log(roll);
var diceRoll = new index_1.DiceRoll("3d6");
console.log(diceRoll.roll());

(so that it properly points to the index.js file in the src directory) and cd scripts/src && node tests.js I get the following error:

/Users/james/Projects/rpg-dice-roller/src/index.js:1
(function (exports, require, module, __filename, __dirname) { import DiceRoller from './dice-roller.js';
^^^^^^^^^^

SyntaxError: Unexpected identifier
at new Script (vm.js:79:7)
at createScript (vm.js:251:10)
at Object.runInThisContext (vm.js:303:10)
at Module._compile (internal/modules/cjs/loader.js:656:28)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:699:10)
at Module.load (internal/modules/cjs/loader.js:598:32)
at tryModuleLoad (internal/modules/cjs/loader.js:537:12)
at Function.Module._load (internal/modules/cjs/loader.js:529:3)
at Module.require (internal/modules/cjs/loader.js:636:17)
at require (internal/modules/cjs/helpers.js:20:18)

Based upon searches ~two weeks ago, I'm wondering if I need to run/test these with Jasmine, instead of just node, since it sounds like this might be a limitation of node and import.

[GreenImp]

Ah right. Yes, that bit about imports with Node makes sense. I'm not sure if it'll fix your problem, but Node doesn't currently work with ES6 module imports (That's coming soon, or you can use a flag to enable now: https://nodejs.org/api/esm.html#esm_enabling).

So, the library compiles the stuff in the src directory into two different formats, inside the lib directory.

• lib/esm/bundle.js contains the es6 module compatible stuff all bundled into a single file. This can be used like any typical es6 module.
• lib/umd/bundle.js contains a bundled non-es6 module version, specifically for browsers that don't support es6 modules and the current version of Node

Jasmine is different though. I think it uses PhantomJS or similar as a JS engine, so it doesn't have the same issues as Node.

If you're running the library through Node, you'll either need to enable es6 modules, or try running against the umd bundle.
Or you can see if you can get it running through Jasmine, which would be good as all the tests can be run at once then.

[GreenImp]

I was wrong. Jasmine is using the umd compiled version.

[GreenImp]

Hi @JamesSkemp I'm nots ure if you're still looking into this or not - I understand if other things have taken priority - but thought I'd let you know I'm almost done with a re-build of the parsing engine.
The external API hasn't changed much, so the Typescript definitions hopefully shouldn't need many changes.

The benifit though, is that I've replaced Jasmine with Jest. This works nicely with ES6 imports, so there's no longer a need to pre-compile, you can import the src files directly.
Hopefully that will make things a lot easier.

You can take a look at the code in the feature/enhanced-parsing-engine branch. The code is pretty much complete and I'm just writing tests for it now, so should only be minor changes.

[JamesSkemp]

I actually pulled this project back up when I restarted my computer the other day, so definitely still interested in using and writing defs for it.

I'll read up on Jest and take a look at that branch this week.

[GreenImp]

Ah, I also should have mentioned that, although the new rolling engine is on that branch, the replacing of Jasmine JS with Jest is actually on the feature/testing-enhanced-parsing-engine branch. This is in quite active development, but give an idea of how the tests are being done now.

[JamesSkemp]

I've come to the conclusion that I need to ask the community how to integrate these definitions in, since the main suggestions I see about using Jest (and other testing frameworks) relates more to projects that are completely written in TypeScript.

From that, a user (keith) noted a mistake in the repo's summary/description field:

An advanced JS based dice roller that can role various types of dice and modifiers, along with mathematical equations.

[GreenImp]

Ah good spot! That's what happens when I write documentation too late at night.

[ldd]

Quick note that there is an interesting alternative: dice-typescript.

But as another member of the community, I would also be interested in minimal typescript support.

I wouldn't do it entirely by myself, but if more people are interested, I'd love to collaborate.

[JamesSkemp]

Belatedly @ldd, do you have experience creating in-library TS defs? That and actually testing the defs (which I'm not sure is even possible - they should either work or not) is where this is stalled.

[JamesSkemp]

Update: Due to microsoft/dts-gen#42, dts-gen -m rpg-dice-roller can no longer be used to create fresh starting definitions. (Working with Vuejs was hoping to use it as an opportunity to give this another go.)

[GreenImp]

Thanks @JamesSkemp for your work. This has been merged and will go out in v4.2.0

[JamesSkemp]

Woohoo!

[GreenImp]

This has been release now!