Using type annotation in the project

[ephread]

By contributing, I discovered that inkjs is trying to stick as close as possible to the reference C# implementation. While I don't believe that type annotation is always a silver bullet, this really makes the project a good candidate for type annotation.

Porting statically, strongly typed code to a dynamically, weakly typed language in inherently cumbersome (the reverse holds true as well) and it’s especially true when keeping most of the original language idioms.

I'm neck-deep in porting the changes of Story.cs at the moment and I could really, really use the help of a static type system. I keep checking the method signature by hand, since the code editor can't infer them on dynamically types variables. I really don't feel comfortable, as I'm constantly afraid of making small type mistakes, that would end up being difficult to track. For instance, the ported Javascript methods don't always do argument shifting, which make handling optional arguments fairly brittle. I believe that I may have missed some named arguments at some point, and ended up passing a value to the wrong argument. I'll recheck all my PR, to make sure that wasn't the case.

So, to make inkjs closer to its C# roots, I do feel that we need type annotation. That leaves us with two mains options, Flow and TypeScript. In order to have a fair discussion, I’ll admit that I’m fully biased, since I like TypeScript. But beyond my preferences, TypeScript could provide significant advantages.

Pros

• A number of TypeScript constructs look quite similar to their C# equivalents and the type system would solve issues like interface conformance. Additionally, TypeScript supports optional arguments in functions (ES2015 does as well, to be fair, but it doesn't seem to be used in the project) and function overloading.
• TypeScript does type inference for variable declaration just like C# does.
• TypeScript can transpile to both ES5 and ES2015. It doesn't polyfill, though, but Babel could keep handling the task.
• TypeScript is a strict superset of Javascript, which means that while type annotations would be required on all function signatures and properties, most of the current code would be left untouched.†
• Typescript can be added in the code incrementally.†
• If one knows how to write Javascript and how to read C#, they know how to write TypeScript.

Cons

• Once compiled, concatenated (and possibly minified), the distributed file will necessarily be slightly larger. I don’t consider this a particular issue, as the size isn’t likely to grow significantly, but it may very well be a point of contention.
• TypeScript will add another layer of transpiling, which may increase the difficulty of debugging. My own experience is rather positive, as the source map system works great, but that's something to consider. Besides, we're already using Babel and Rollup, what's one more transpiler after all 🙄.

† I've set up a little example here to show how easy and straightforward it would be to use TypeScript. I've turned Path.js into Path.ts. You can see how small the additions are on the file and how close to the original file the transpiled file is.

Additionally, the typescript compiler warned me about this.components which should have been this._components. (After checking with the C# source, it seems that it was indeed a typo – a few seconds in, and TypeScript proves its usefulness!)

With the change, tests are still passing successfully and Rollup packaged the code with no issues (I also tested the resulting ink.js in the browser and… it works perfectly). It took me all in all 20 minutes to set up the example (and much longer to write the issue 😄), I feel that the experiment is a success.

Thanks for reading, I rest my case! What are your thoughts? If you're up for it after completing v8, I'll happily put the work in.

(Sorry if this whole block of text comes out as brash, that's not the intention. It feels a bit like the newcomer suggesting large changes without necessarily understanding the scope of the project, which ranks fairly high on the scale of Open Source disrespectfulness.)

[y-lohse]

Thanks for the write-up! I definitely empathize with your pain points, and I've experienced them myself. I've never used typescript of flow but I'm clearly not against deploying them here, it looks promising!

A couple of extra questions though:

• Do the type-checks happen at compile time or run time? Or both? And if they happen at run time, what exactly happens when there is a type mismatch?
• How does linting work with typescript? Do you use eslint with a plugin, or a different linter altogether?
• The ts compiler takes an entry file and converts it to plain javascript, correct? So it would be a substitute for rollup, or would we still need both?
• When you say typescript can be added incrementally, you still need to do it on the whole file at once, correct? You can't have untyped definitions in .ts files?

[ephread]

Thanks for the write-up! I definitely empathize with your pain points, and I've experienced them myself.

Thanks for reading my long post and empathizing!

Do the type-checks happen at compile time or run time? Or both? And if they happen at run time, what exactly happens when there is a type mismatch?

Compile time only. It's the standard Javascript behavior that applies at runtime, AFAIK TypeScript generates as little as possible, it mostly checks at compile time and converts the bit of syntactic sugar it offers. For instance, var foo = bar as Foo isn't really a type cast (they call it Type Assertion instead) in TypeScript, thus the type cannot be enforced at runtime without resorting to typeof and instanceof. But that's mostly an issue in polymorphic constructors, and certain types of overloaded functions, where the types will need to be checked the old-fashioned way.

For most of the functions, given that they are not exposed for general use, the static type assertion would be (I believe) sufficient. On the other hand, in all exposed functions, we may need to assert the type of user-provided parameters at runtime.

How does linting work with typescript? Do you use eslint with a plugin, or a different linter altogether?

There is a built-in linter in TypeScript which is more or less combined with the compiler. There is also TSLint from Palantir which mimics ESLint. And it can be enriched with plugins as well.

But I won't lie, although it works fairly well, TSLint is not nearly as advanced as ESLint. Prettier also supports TypeScript; however, it might not be desirable to format code automatically in the project.

The ts compiler takes an entry file and converts it to plain javascript, correct? So it would be a substitute for rollup, or would we still need both?

That's correct, yes. I don't think TypeScript can concatenate and minify files, plus it won't create polyfills for missing features (like Object.Assign), so Rollup would still be needed for the packaging part and Babel would still be required to support older browsers.

When you say typescript can be added incrementally, you still need to do it on the whole file at once, correct? You can't have untyped definitions in .ts files?

Correct! And you can't import untyped definitions either.

So we need to start with files having the lowest number of dependencies and climb up to the files with the highest number of dependencies. When I said incremental, what I had in mind is the fact that we can run tests at each step of the TypeScript integration (file by file, or worse case scenario, couple of files by couple of files) and make sure that we did not break anything.

The tests should definitely remain in plain Javascript by the way, since they test the entire flow.

Adding types is fairly straightforward:

1. turn .js into .ts;
2. open the file side by side with its C# counterpart;
3. watch your file getting clogged with red underlines;
4. add types until all the red is gone;
5. rinse and repeat!

[y-lohse]

Compile time only.

Cool, that was my understanding. But then why would the distributed files become larger? Asking out of curiosity more than anything.

Prettier also supports TypeScript; however, it might not be desirable to format code automatically in the project.

I had forgotten about prettier, but that might actually be a good option combined with the built-in linter of typescript. But cool, there are options, we can discuss that in a separate issue.

I don't think TypeScript can concatenate and minify files

Well the files aren't really concatenated at the moment, Rollup "just" resolves the dependencies and bundles them all together. I thiiink typescript would handle that, so we'd only be left with babel to create a polyfilled version. I'm hopeful we can avoid having 3 transpilers :)

---

The conversion process sounds really easy. Let's leave this open in case someone else wants to chime in, but I think it's at least worth a shot after the v8 port. I'll make sure to contribute as well :)

[ephread]

My bad, I missed the question:

But then why would the distributed files become larger?

Slightly larger because of the syntactic sugar involved. That's a hypothetical thing that I pointed out, in case it would be important. I've never really checked whether it's really the cause; if it is, then it's most likely negligible.

---

Now that v8 is out (kudos 👏!), I've created a new typescript-conversion branch, starting from master's latest commit.

I've also added a TSLint config, matching the ESLint that you created in #165 (a couple of standard rules have been disabled to make it play well with the current style).

The tests are all green, and that makes me very happy. 😃

The diff is mostly useless, since all the files have been moved to src and are transpiled into engine (I should probably have done this in two steps, sorry for this). The files that I've touched in 4c8607a are:

Path.ts, PRNG.ts, PushPop.ts, Stopwatch.ts, StoryException.ts,
StringBuilder.ts, Tag.ts, VariableAssignment.ts

Next on the list are InkList.js, Container.js, Object.js and related dependencies. For now, I'm just converting declarations, adding type annotations / access modifiers, and converting var to let or const when appropriate (you'll let me know if I'm overzealous). In the future, it might be desirable to remove some of the dynamic checks, since in many cases, the static ones will ensure correctness.

Would you be able to create a branch, against which I would open up a PR? We'll then track all the required changes from there.

[y-lohse]

That's exiting! I've created the typescript branch to track the work on that subject.

I was working on linting myself and found a few potential bugs (missing this's, intval instead of intVal, etc), but I guess we'll find them with TSLint too.

converting var to let or const when appropriate

I was planning to write a codemod to change all the var to let, as that is closer to the C# behavior… so go for it!

Anyway, looking forward to this! Let me know if you want some help with anything!

[NQNStudios]

If there's any way another person could help with this, I'm available and I support the move to TypeScript!

[ephread]

Great! Hop in #183!

[y-lohse]

We got this done 💪