Adopt typescript for the backend

[ang-zeyu]

Is your request related to a problem?

The readability of the backend (core package) codebase can really benefit from some strict typing.

Describe the solution you'd like

Migrate to typescript

Requirements:

• The migration should be able to be done incrementally
• Adjustments should be made to the dev workflow such that it isn't more cumbersome than before. We may provide npm scripts for:
  • auto recompilation of typescript source files
  • auto restart of some common dev commands (e.g. markbind s -d in the docs/ folder) run after recompilation
  • can look into: https://www.npmjs.com/package/tsc-watch
  • ❔
• Other workflows should be adapted as necessary: release, automated site deployment, ...

Additional context

The frontend may not see as much benefit, since we only develop mostly independent components. Also, typescript support for vue v2 apis isn't known to be strong.

JSDoc typing: #1039 (review)

[le0tan]

Looks like cli is now an independent npm package that depends on core - is there any way to make require('@markbind/core/src/...') depends on the local version?

[ang-zeyu]

@le0tan should be already, we have some updated setup instructions :-)
https://markbind.org/devdocs/devGuide/settingUp.html#setting-up-the-dev-environment

[le0tan]

@ang-zeyu Looks like the dev guide should replace running npm link under packages/cli with running lerna link under the repo root?

[ang-zeyu]

npm link is only for executing markbind xx commands globally for development, lerna link does not do the same

npm run setup executes lerna bootstrap which does installation + also cross-package symlinking

[ang-zeyu]

labelling this as high now that 3.0 is rather stable, we could really use better type safety going forward to prevent silly regressions and make variables / fields a little more readable

- and much better ide suggestions!

[ang-zeyu]

@wxwxwxwx9 @jonahtanjz @raysonkoh @ryoarmanda If anyone wants to tackle this issue in the upcoming semester, do drop a note here. Would advise to start now as well as its a massive change that will conflict with other PRs! (would be much smoother to iterate on it now)

[ryoarmanda]

I can take a crack at it @ang-zeyu ! Not the most ideal time 😅 but I guess doing this now would still be quite okay.

Just to clarify and give a little bit of my plan, the approach would swap most of our backend .js files in the repository with .ts files. The compilation will be done with tsc and compiled in the same directory as the .ts files, as discussed in #1427. The compilation command may be added within npm run setup for first-time compilation (I guess at the end, after lerna finishes). Subsequent compilations can be done manually through npx tsc (which can be set up to npm run compile / npm run build, although those will be more verbose than the actual command 😄), or automatically via tsc-watch. The latter can be written as a script, npm run dev or some sort, that can be run throughout development.

[ang-zeyu]

Just to clarify and give a little bit of my plan, the approach would swap most of our backend .js files in the repository with .ts files.

Yup. Can (and probably should) be done incrementally as well. Converting a select few files should do for a start!

The compilation will be done with tsc and compiled in the same directory as the .ts files, as discussed in #1427. The compilation command may be added within npm run setup for first-time compilation (I guess at the end, after lerna finishes). Subsequent compilations can be done manually through npx tsc (which can be set up to npm run compile / npm run build, although those will be more verbose than the actual command 😄), or automatically via tsc-watch. The latter can be written as a script, npm run dev or some sort, that can be run throughout development.

This was my general idea back then but things might have improved (from tsc side) since then. Feel free to explore alternatives if we can satisfy the requirements even better 😄

One more important note on version control -- generated .js (+ sourcemaps) files should probably not be committed / pr-ed as it generates too much noise in the diffs.
We could gitignore the .js files to avoid this (but not during npmignore) to avoid publishing issues perhaps.
The downside is that what's published isn't reflected in version control, but I think its a small & common pain point we can live with for now.

If you can find any way of gitignoring .js files in between releases (but not during releases) all the better though!

[ryoarmanda]

Yup. Can (and probably should) be done incrementally as well. Converting a select few files should do for a start!

Agree, I will draft up a PR that initially converts some of the smaller .js files in our core package, see if it's all good, then continue working up towards the main files in core.

This was my general idea back then but things might have improved (from tsc side) since then. Feel free to explore alternatives if we can satisfy the requirements even better 😄

It is also what I have came up with so far with my research, I think the approach still holds up for now. For other related aspects, this is what I would likely go for:

• Debugging via IDE: Generate sourcemaps
• Testing: Use ts-jest

I haven't probably covered all aspects for now, will research as I go along. Feel free to let me know for any specifics!

One more important note on version control -- generated .js (+ sourcemaps) files should probably not be committed / pr-ed as it generates too much noise in the diffs.
We could gitignore the .js files to avoid this (but not during npmignore) to avoid publishing issues perhaps.

Also agree. I will continually update the gitginore for the compiled .js files and create an empty npmignore to override the exclusion, so the .js files is still being included into the package.

If you can find any way of gitignoring .js files in between releases (but not during releases) all the better though!

The one I can think off-the-cuff is to temporarily remove the .js exclusion from .gitignore and recompile for release 😄 simple, but admittedly added that extra small manual step of deleting the exclusion rules (albeit it will be just a few lines). Will research for other ways though!

[ryoarmanda]

Just some update that comes along during my work:

I will essentially break the work down in 2 stages (may be in separate PRs, with the se):

1. Set up the necessary environment and swap all of our own .js to .ts (patched libraries will be kept as-is?) without any modifications to the code.
   • There will inevitably be TS "errors", but tsc will still be happy to compile as intended. The errors can be fixed incrementally as we go on.
2. Modify each file to resolve their TS errors and setting the strict mode if need be, which may be an involved process but possibly can be done collaboratively

I already almost completed the first phase, but I haven't run the tests, and unfortunately I just found out that by adding the typescript package, our tests broke, surprisingly because we have diffs in the FontAwesome assets. Looks like the addition of TypeScript indirectly bumps FontAwesome from 5.15.3 to 5.15.4. Therefore, in order for us to use the typescript package, we have to update the tests assets, which brings about huge amount of changes:

(You can verify that typescript is causing this by doing npm install typescript --save-dev then npm run test, which would later freeze at reporting the diff at one of the FontAwesome's CSS)

Also, there is an issue with installing ts-jest as mentioned in the issue above this comment.

I think we should address them first, maybe as a separate PR? Or would you suggest the FontAwesome asset updates are to be done in the same PR as the first stage? (cc @ang-zeyu @jonahtanjz ) (Edit: we can solve #1793 along the way too)

[ang-zeyu]

I think we should address them first, maybe as a separate PR? Or would you suggest the FontAwesome asset updates are to be done in the same PR as the first stage? (cc @ang-zeyu @jonahtanjz ) (Edit: we can solve #1793 along the way too)

👍 Definitely, let's do it separately, to make the PR more reviewable

swap all of our own .js to .ts

If possible, I think this could be done separately as well (i.e. when you attempt step 2), to avoid conflicting with the other PRs as much as possible.