[Ready for review] Rewrite to TypeScript

[cshaa]

In this PR, I rewrite the code from make-things-less-complicated to TypeScript. Right now there are many type errors and some tests don't work. What does work, however, is compilation: npm run build will produce essentially the same result as before. You can test the compiled code using npm run test:compiled.

This PR will fix #36, and close #37

[ericman314]

It's looking great! I'm excited to see how this turns out.

Thanks for your work so far. I am actually migrating another of my projects to TypeScript right now, so I know how frustrating it can be, but I think the end result will be worth the effort.

[cshaa]

The types are all over the place, which is partly caused by me not knowing what I'm doing, but mostly by the fact that the library was written very “dynamically” – many objects can have three or more different interfaces that I had to reverse-engineer from the code. However, I hope that now that it's TypeScript, it will be much easier to refactor the code.

There are some types (eg. this one) that I had no idea how to name. If you have better naming ideas, I'll happily use them :)

The main test (ie. test:src) doesn't work yet, but compilation works A-OK and test:compiled also works.

[cshaa]

I had to add an Unit<V> interface, since TypeScript couldn't export the types from an internal non-exported class Unit.

[ericman314]

I just pulled down your PR to try it out, and I'm getting an error with npm test. Are you familiar with what's going on here? Did I forget something?

FAIL  test/Unit.test.ts
  ● Test suite failed to run

    SyntaxError: /home/eric/Documents/unitmath/src/Unit.ts: Missing semicolon. (8:4)

       6 | // TODO: Make things behave nicely when performing operations between units that exist in different namespaces (ahhhhh!)
       7 |
    >  8 | type n = number
         |     ^
       9 | const IS_DEFAULT_FUN = '_IS_UNITMATH_DEFAULT_FUNCTION' as any
      10 |
      11 | export interface TypeArithmetics<T = any> {

      at Parser._raise (node_modules/@babel/parser/src/parser/error.js:134:45)
      at Parser.raiseWithData (node_modules/@babel/parser/src/parser/error.js:129:17)
      at Parser.raise (node_modules/@babel/parser/src/parser/error.js:78:17)
      at Parser.semicolon (node_modules/@babel/parser/src/parser/util.js:131:10)
      at Parser.parseExpressionStatement (node_modules/@babel/parser/src/parser/statement.js:835:10)
      at Parser.parseStatementContent (node_modules/@babel/parser/src/parser/statement.js:321:19)
      at Parser.parseStatement (node_modules/@babel/parser/src/parser/statement.js:176:17)
      at Parser.parseBlockOrModuleBlockBody (node_modules/@babel/parser/src/parser/statement.js:910:25)
      at Parser.parseBlockBody (node_modules/@babel/parser/src/parser/statement.js:886:10)
      at Parser.parseProgram (node_modules/@babel/parser/src/parser/statement.js:74:10)

[cshaa]

Hey Eric, glad to hear from you! Try to test the project with npm run test:compiled.

Since I left npm run test unchanged, it can't possibly work, as it tries to run uncompiled TypeScript in Node. I added the test:compiled script which compiles TypeScript to JS and then runs the tests on the result. If you're okay with having to recompile before every testing, we can just use the test:compiled script as default. However, it should be possible to run the tests directly on the source using ts-node. Do you want me to look into that option?

[ericman314]

Ah, I see. I think the default test should definitely include compiling with typescript so that type errors show up immediately.

ts-node looks to be widely used so it will probably work. Alternatively, what about something like this? https://jestjs.io/docs/getting-started#using-typescript Will that accomplish what we need?

[ericman314]

Never mind, looks like @babel/preset-typescript doesn't perform type checking.

It looks like you tried ts-jest. I uncommented preset: 'ts-jest' in jest.config.js, and then when running npm test I got of lot of type errors from the Unit.test.js. So maybe it's already set up the right way and we just need to fix the type errors in the tests themselves now?

[cshaa]

It looks like you tried ts-jest. I uncommented preset: 'ts-jest' in jest.config.js, and then when running npm test I got of lot of type errors from the Unit.test.js. So maybe it's already set up the right way and we just need to fix the type errors in the tests themselves now?

Oh, cool, I completely forgot I tried that 😁️ Yes, it seems like adding proper types to the tests would solve the problem. If you want to take on this, go for it! :) Otherwise, I'll look into it in a few days.

[ericman314]

Cool, looks like my test commit worked. I'll look at adding types to Unit.test.js.

[ericman314]

Just to give an update: it's been a while since I've worked on this (due to a variety of circumstances). But last time I tried, I was having a hard time fixing the typescript errors after renaming Unit.test.js to Unit.test.ts. I'm wondering now if this is the result of sloppy design of the initial library on my part, so that now we're having to jump through a bunch of hoops to get the typescript to compile without errors. It seems to me like a simpler design would have made the conversion to typescript more intuitive. Or perhaps the difficulty instead lies in the flexibility that is required by the API, which allows people to add their custom definitions and even use custom numeric types.

If there are any refactorings we can make that would make the type definitions more intuitive and simple, I am 100% in favor of making those changes. Also I'm aware we need to change some of the variables' names to protect the innocent prevent confusion. Things like unit.unit.value and so forth. I really want to make this a simple library, to make it easier to understand and less error-prone.

[ericman314]

I've finally gotten almost all the tests to pass, but I had to make some simplifications in order to do so--otherwise it would have been such a monumental task I would never have gotten it done.

• I removed the concept of unit "systems", as it was giving me a lot of headaches. But I don't think it will be too difficult to add that functionality back in.
• Sadly, I have removed the support for custom types and all the generic type identifiers that go with it. I absolutely do want to add these back in as soon as possible, however.

I still need to do some refactoring to make variable and type names more clear, although, it will be hard to improve on UnitDefinitionsButCooler ;-)

[ericman314]

Thanks @m93a! I still need to add back in support for custom types but this is a great start and we are getting close to v1.0.

[josdejong]

🎉