Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Typescript compiler. #233

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aquach
Copy link

@aquach aquach commented Nov 6, 2021

Hey there, I have a Typescript compiler I whipped up yesterday that I'd like to share. Rather than generating Typescript code directly, it generates .d.ts files, which are declaration files like .h files for C, that add types to untyped JS code.

Here's an example of generated code: https://github.com/aquach/test-ksy-typescript/blob/main/src/generated/Sqlite3.d.ts which is generated from https://github.com/aquach/test-ksy-typescript/blob/main/sqlite3.ksy. The goal of the .d.ts is to match the shape of the runtime, which is in https://github.com/aquach/test-ksy-typescript/blob/main/src/generated/Sqlite3.js. The net result is that Typescript code can get autocomplete and type inference for Kaitai-generated structs, which you can see if you check out the project and look at the test example https://github.com/aquach/test-ksy-typescript/blob/main/src/test.ts. Rather than being unchecked code, Typescript is able to complain if you misspell a field, or access something without checking nullity/undefinedness.

The implementation is actually not too bad because most of the runtime stuff can be entirely ignored. You really just need the class shape and the fields. I'm relatively new to Kaitai so please let me know what I'm missing. I tried to exercise a few different .ksy files from the site which exercised a fair amount of the feature set, but there's a lot I could be missing.

The only runtime change I needed to make was to the JS compiler to add a new __type field. This is a common pattern for discriminating unions in Typescript, and makes the resulting code much easier and safer to use. For example, with this .ksy snippet:

seq:
  - id: key_name
    type: cstring
  - id: body
    type:
      switch-on: key_name.value
      cases:
        '"ssh-rsa"': key_rsa
        '"ssh-dss"': key_dsa

Without __type:

const s = /* struct */;
switch (s.key_name) {
  case 'ssh-rsa':
    const body = s.body as KeyRsa;
    // operate on body
    break;
  case 'ssh-dss':
    const body = s.body as KeyDsa;
    // operate on body
    break;
}

With __type:

const s = /* struct */;
const body = s.body
switch (body?.__type) {
  case 'KeyRsa':
    // operate on body, which is automatically narrowed to KeyRsa
    break;
  case 'ssh-dss':
    // operate on body, which is automatically narrowed to KeyDsa
    break;
  case undefined:
    // handle undefined case
    break;
}

I'd also like to know how to go about writing tests for this, since a lot of the testing seems to be in the translation layer and not the compilation layer.

@naclomi
Copy link

naclomi commented Nov 6, 2021

thank you so much for this! i'm using it successfully for a typescript-based project already 😄

@GreyCat
Copy link
Member

GreyCat commented Nov 8, 2021

Hey @aquach , thanks for this contribution!

Just for sake of visibility - there was a previous attempt to add TypeScript support - #165 - which seems to have largely stalled and never finished.

I'll try to get to review this one soon.

@aquach
Copy link
Author

aquach commented Nov 8, 2021

Thanks! I did see that one. It looks like it was trying to generate native Typescript code instead of just the definition file. I believe generating the definition file is significantly easier (as evidenced by having something usable with only a few hundred lines of added code) so I decided to go down that path. Thanks for the review!

@aquach
Copy link
Author

aquach commented Nov 8, 2021

Looks there are some test failures, but at first glance they don't seem related to my change 🤔

@naclomi
Copy link

naclomi commented Dec 17, 2021

Bump
It'd be really nice to have this upstreamed

@naclomi
Copy link

naclomi commented Oct 18, 2022

Any updates on the status of this?

@laura-a-n-n
Copy link

This still works for me, it's great!! Thank you so much for this.

I was able to pull it and merge in the latest main branch. There was a conflict and some errors in the Scala code, but I was able to resolve it. Not sure what the best way to share my changes would be.

Would love to see this merged!

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.

4 participants