Add TypeScript typings.

[jchv]

Took a few tries but I think I got it right. It should work when used as an ambient module or through a module system.

I think it would be nice to go further and convert the whole dang thing to TypeScript. It would even catch some errors, including an operator precedence one in the signed integer handling (I'll create another PR for it.) So I think it's definitely worth the trouble to have another build step to build the usual ES5 UMD module. There's a couple issues though:

• Moving everything to TypeScript would add a dependency (at minimum we need the Node.JS typings for Buffer) and a build step. Today, this module is really simple. It has no dependencies and no build steps at all. I think that is really nice, and therefore getting rid of that property requires more consideration.
• In ES6 classes, you can extend the Error class normally, but if you compile this down to ES5 prototype inheritance, it gets a little broken. There's hacks to work around it (like Object.setPrototypeOf) but I definitely think this sucks. Compiling to ES6 classes would be doable, but it would break compatibility with old browsers. The setPrototypeOf hack would break compatibility with IE <10. The existing approach checks most boxes but it can't be expressed in TypeScript. 🙁

[GreyCat]

My only concern right now is that this is not being tested in any way. Can we add some sort of CI step to ensure that these .d.ts files at least compile?

[koczkatamas]

I will be able to review this tonight hopefully.

[jchv]

@GreyCat Yeah, it would be nice to test it. However, at that point I'd have to add a dev dependency on at least the TypeScript compiler. Is it worth just porting the whole thing over at that point? The caveat regarding extending errors above is the only other thing that stopped me from proposing that.

[GreyCat]

I totally won't mind moving to TypeScript for everything, as long as we still support older (ES3?) targets.

That said, though, currently, we don't have any tests for in-browser JS engines, and this is probably something to consider here. Mocha, which we use, technically can be used to launch an in-browser test, and we can even automate that to some extent by running this in headless chrome or something similar, but it will be a major hurdle to jump over to make it run in CI with really old browsers (IE6?).

So, on that trrack, probably we should start at deciding on that minimal baseline and adding relevant tests. Until then, I'm a huge proponent of "does not break tests = valid".

[jchv]

SGTM. In that light, maybe the right direction is incremental steps. Adding browser testing before considering porting to TypeScript will save us some pain.

I do believe that TypeScript's ES5 output will be compatible with any browser that has native typed arrays (IE10+) and then some. So as far as compatibility goes, I think tsc transpilation is sufficient, if we establish our baseline as native typed array support. It seems reasonable, since performance of emulated typed arrays is pretty bad.

[GreyCat]

LTGM now.

[GreyCat]

native typed array support

Probably a solid idea: https://caniuse.com/#feat=typedarrays

That actually means that we'll be officially supporting IE11+, Edge, FF 4+, Chrome 7+, Opera 12+. Everything else is probably more or less safe to ignore.

Now I actually wonder if there are any ready-made browser testing farms that can be used to export test unit results from something similar to Mocha, so we could leverage it to run in-browser tests?

[GreyCat]

Added main project issue: kaitai-io/kaitai_struct#540 to track in-browser CI idea.

[k2on]

Is this getting merged?

[jchv]

This has been superceded, since a proper TypeScript port was merged instead.

[generalmimon]

@jchv:

This has been superceded, since a proper TypeScript port was merged instead.

You're right, thanks - I forgot to close this after merging #25.

According to my understanding, the [email protected] version at npm should already contain TypeScript definitions. I did my best to ensure that the published TS definitions are correct, but admittedly I don't know that much about TypeScript. So if you have some time, I'd be grateful if you could check that it looks as expected.

I can only tell with some degree of confidence that the migration to TypeScript didn't break the KaitaiStream.js file that is now generated from KaitaiStream.ts, because CI already uses it (we run npm install -g kaitai-struct@next when building the kaitai-javascript-nodejs{4,8,10,12,20}-linux-x86_64 Docker images in kaitai_struct_docker_images / src/javascript/_common/prepare:59-60) and passed for all Node.js versions we're testing on. But I have no idea if the *.d.ts files work.

[jchv]

From a cursory glance the TypeScript definitions in the package look fine to me, but I'll have to try updating it in one of my TypeScript projects and find out.