Discontinue the use of TypeScript project references

[ryoarmanda]

What is the purpose of this pull request?

• Documentation update
• Bug fix
• Feature addition or enhancement
• Code maintenance
• Others, please explain:

Resolves #1921 (follow-up to #1921 (comment))

As discontinuing project references would mean we cannot do tsc --build --clean anymore, this PR includes the substitute to the manual clean script and an automatic invocation of it as well.

Overview of changes:

• Manual clean script
  • Added a custom script scripts/clean.js to clean compiled TypeScript files including those that have their TypeScript sources deleted prior cleaning (for brevity, I'm calling this the "rogue" compiled files in the discussions below)
  • Modified npm script clean to run the above script
• Git hooks
  • Added a hook script .githooks/post-checkout that runs npm run clean when triggered
  • Added npm script install:hooks that runs a Git command to reference .githooks as the hooks provider
  • Added npm lifecycle script install that runs install:hooks, which is automatically triggered when npm install/npm ci has successfully installed the packages
• Discontinue the use of project references
  • Modified root tsconfig.json and package core/tsconfig.json accordingly

Anything you'd like to highlight / discuss:

• Feedbacks to approach, organization, and others are appreciated
• In order for ESLint to not throw lint errors about the imports in clean.js, I added walk-sync to the dependency of root package.json. This is the first package that does that, so I want to make sure that we are okay in this.
• Any remnants of tsconfig.tsbuildinfo can be deleted manually. They do not have an impact to the build process now
• There is one case that we have not yet addressed: cleaning before automatic rebuild. To tackle this, we may need to consider two things:
  • Use tsc-watch for the --onCompilationStarted functionality that can run the cleaning script before the build.
  • May need to write a separate cleaning script that only tackles the "rogue" compiled files for performance (no need to clean the proper compiled files, it will be overwritten by the rebuild anyway)

Testing instructions:

Proposed commit message: (wrap lines at 72 characters)
Discontinue the use of TypeScript project references

---

Checklist: ☑️

• Updated the documentation for feature additions and enhancements
• Added tests for bug fixes or features
• Linked all related issues
• No unrelated changes

[ang-zeyu]

Lgtm 👍

Tested: checkout branch / individual commits
Committing / amending dosen't seem to trigger this too which is nice.

There is one case that we have not yet addressed: cleaning before automatic rebuild. To tackle this, we may need to consider two things:

Hmm this sounds quite complicated to tackle for the returns, although definitely nice to have.

I think having the checkout hook at least would be sufficient for now, since the risk comes more so from checking out others' commits. (a developer should be conscious of what files he/she deletes =P)

Relevant: microsoft/TypeScript#16057

There are some third party solutions listed in it but imo they might be a little heavy as additional file watching solutions ("best" fix would still come from the ts team)

Wdyt? @ryoarmanda @jonahtanjz
(tldr I think its not a huge pain point, at least not right now)

[tlylt]

Sorry for the late comment!

It seems like performing a git checkout via UI (in my case, using vscode's git integration) does not trigger the clean-up script from git-hook and will result in an unexpected error like this:

On top of that, it seems like even if running git checkout via the command line will trigger the clean script, the error still persists
if trying to run markbind serve after checking out to another branch. So, on checkout to another branch, devs have to rerun npm run setup (edit: npm run build:backend works too) in order to serve the files. (@ang-zeyu do you experience this as well?)

May I check if this is the intended behavior? It feels pretty unwieldy in terms of development experience... is there any workaround/fixes possible?

[jonahtanjz]

May I check if this is the intended behavior? It feels pretty unwieldy in terms of development experience... is there any workaround/fixes possible?

I think it's possible to add the build script npm run build:backend to the post-checkout githook script so that the typescript files get compiled on checkout?

[ang-zeyu]

It seems like performing a git checkout via UI (in my case, using vscode's git integration) does not trigger the clean-up script from git-hook and will result in an unexpected error like this:

👀😢 No, I only tested these though

1. cli
2. gitkraken
3. sourcetree
4. webstorm

The error seems like another issue ⬇️

On top of that, it seems like even if running git checkout via the command line will trigger the clean script, the error still persists
if trying to run markbind serve after checking out to another branch. So, on checkout to another branch, devs have to rerun npm run setup (edit: npm run build:backend works too) in order to serve the files. (@ang-zeyu do you experience this as well?)

For this PR, the clean script only does exactly that (clean things up), without rebuilding the backend files (which causes your error).

I think it's possible to add the build script npm run build:backend to the post-checkout githook script so that the typescript files get compiled on checkout?

We can certainly add it in. Though, it comes at a slight cost (much longer checkout time).

I'm personally ok with it as I like to "get most things ready" before performing git operations.
I imagine this might be somewhat slow / troublesome if you like to spam git checkout -- ... / ... while developing.

Another option is this: #1877 (comment)

While it dosen't solve the issue, setting up some standard npm commands (instead of npm link + setting up your ide) with development for some of our sites (template / docs) can alleviate the hassle of manually running tsc watch / build post checkout + make it easier to get started with markbind development overall.

But, it also risks "burning" / slowing your system due to running markbind s -d -o everytime you save a file. (@ryoarmanda's comment) Or we can advise to turn off autosave for the project.

I think we pick one or both of these and see how things go first, revert if its too slow. Wdyt? @jonahtanjz @tlylt

[ang-zeyu]

update: just tried vscode's source control tab (checking out another commit in particular), the clean script successfully runs. which operation did you try? @tlylt

[tlylt]

update: just tried vscode's source control tab (checking out another commit in particular), the clean script successfully runs. which operation did you try? @tlylt

I use this to change branches sometimes... If most other interfaces work then perhaps it's not a big issue.

I think it's possible to add the build script npm run build:backend to the post-checkout githook script so that the typescript files get compiled on checkout?

Yup, I think this should work.

I'm personally ok with it as I like to "get most things ready" before performing git operations.
I imagine this might be somewhat slow / troublesome if you like to spam git checkout -- ... / ... while developing.

My "use cases" are that

• at times I may have multiple branches (😓) that I work on somewhat concurrently, and
• sometimes I need to checkout to master to verify some of the issues posted(or some potential issues that I might want to raise if I can confirm locally that the problem is real), or simply check out other dev's PRs

We can certainly add it in. Though, it comes at a slight cost (much longer checkout time).

My personal take is that usually if I want to checkout to a branch, I would typically want to run markbind serve/build. Then in any case I would need to accept the wait time (either done during checkout or invoked manually).

I think the last point that I want to call out is the error (see #1934 (comment)) is not exactly easy to understand/trace, some documentation/caution is perhaps required to help newcomers.

[ang-zeyu]

I use this to change branches sometimes... If most other interfaces work then perhaps it's not a big issue.

Hmm I usually use the UI on the left sidebar. But this works for me too. Just tried checkout > existing branch + create new branch.

Are you checking out to a commit before this PR? (4fd2061) If so it won't work, I think that's by design of the git hooks.
From a development perspective it should be ok as well since we've only recently started transiting to typescript.

My "use cases" are that

Agreed (likewise here)

My personal take is that usually if I want to checkout to a branch, I would typically want to run markbind serve/build. Then in any case I would need to accept the wait time (either done during checkout or invoked manually).

But not so much for just checking-out branches. e.g. This also runs when you do git checkout -- *.png to remove the puml diffs.

But again I don't really run into this sort of situations often. Maybe @jonahtanjz has a different experience.

I think the last point that I want to call out is the error (see #1934 (comment)) is not exactly easy to understand/trace, some documentation/caution is perhaps required to help newcomers.

Yes, I think we can add in a picture of the error + meaning here https://markbind-master.netlify.app/devguide/workflow#editing-backend-features.

[tlylt]

I use this to change branches sometimes... If most other interfaces work then perhaps it's not a big issue.

Hmm I usually use the UI on the left sidebar. But this works for me too. Just tried checkout > existing branch + create new branch.

Sorry, yup it works behind the scene. I didn't see any logs in the terminal and was mistaking it as not running. I guess the "error" is exactly the proof that it works because the clean-up removed the built .js files. Thanks for the clarification!

My personal take is that usually if I want to checkout to a branch, I would typically want to run markbind serve/build. Then in any case I would need to accept the wait time (either done during checkout or invoked manually).

But not so much for just checking-out branches. e.g. This also runs when you do git checkout -- *.png to remove the puml diffs.

Ah TIL... because I only use the UI to discard the changes, I am not aware that we use git checkout to discard files as well.

[jonahtanjz]

But again I don't really run into this sort of situations often. Maybe @jonahtanjz has a different experience.

I mostly use checkout for changing branches/commits and rarely use to remove files. We could add the build script into the git hook for now but it may become slower as we migrate more of our backend to Typescript.

Another alternative we can explore is using ts-node (#1921 (comment)) as mentioned by @ang-zeyu. This should be able to streamline the development experience as there is no need to explicitly build the Typescript files and checkout will not be slow.

[ryoarmanda]

At the moment, I am also inclined to put npm run build:backend to the post-checkout script, as there would be less command sequence the devs need to remember as they move about in the project.

If it happens that the build takes more time than desired (currently it takes some few seconds), or you would like to do some git checkout spam, I can also try to upgrade the post-checkout script such that you can set a local environment variable such that the hooks would be skipped (something like HOOKS=0 git checkout master to skip the post-checkout hook for one-time only).

---

I also found out that the post-checkout script actually receives some parameters (ref, emphasis mine):

... The hook is given three parameters: the ref of the previous HEAD, the ref of the new HEAD (which may or may not have changed), and a flag indicating whether the checkout was a branch checkout (changing branches, flag=1) or a file checkout (retrieving a file from the index, flag=0). ...

This opens up a possibility to do things differently when git checkout <branch> vs git checkout -- <file> is invoked. We can discuss if this is needed for our case.

[ang-zeyu]

I also found out that the post-checkout script actually receives some parameters (ref, emphasis mine):

This looks good, I think minimally we could just look for: previous ref != new ref

Though pointing out it would still fail in some edge cases like git checkout some file from some commit (which I suppose should trigger a rebuild). But shouldn't really matter much since you should be conscious of what files you checkout.

---

So I suppose the frontend dev workflow (or if you just want to do a quick test / edit for the backend) might be something like this:

1. checkout new branch
2. immediately markbind s -d -o

I realised one downside for this added convenience is that it feels a little like "magic" 🎁. As a newcomer, you might wonder why "step 2 just works when I just checkout a branch, yet tsc --watch still exists, etc.". Would have to do some digging to find out about the git hooks. The dev guide should be updated to make this understandable if we want to go with this, as its not quite transparent. That means more things to read and understand too.

Another alternative we can explore is using ts-node (#1921 (comment)) as mentioned by @ang-zeyu. This should be able to streamline the development experience as there is no need to explicitly build the Typescript files and checkout will not be slow.

Another less invasive, slightly more understadable option to this ⏫ (tsc-watch), and also building the backend in git hooks might be simply to combine the above 2 steps into one npm script (no tsc-watch which we can consider later):

// can be less verbose
"buildAndDevDocs": "npm run build:backend && node ...index.js s -d -o thedocs",
"buildAndDevDefaultTemplate": "npm run build:backend && node ...index.js s -d -o defaulttemplate",
"buildAndDevMinimalTemplate": "npm run build:backend && node ...index.js s -d -o minimaltemplate",

+ non build variants to complement them
"devDocs": "node ...index.js s -d -o thedocs",
"devDefaultTemplate": "node ...index.js s -d -o defaulttemplate",
"devMinimalTemplate": "node ...index.js s -d -o minimaltemplate",

I've been thinking about adding these scripts for a while (irregardless of git hooks) as it removes the need to do npm link (which is quite seldom for a development workflow) when getting started.

It feels pretty unwieldy in terms of development experience...

Would this remove the need for building the backend files via git hooks? (though to be clear the current clear.js script is still important as discussed in #1921)

We can also go with adding both in one go, and see how things turn out. Though my personal preference is to add these incrementally as needed if we feel the scripts are sufficient for now.

---

For developing the cli app continuously (e.g. something more substantial like the versioning pr), then one of these 2 options https://markbind-master.netlify.app/devguide/workflow#editing-backend-features.

[ang-zeyu]

Fwiw this is how tsc-watch would fit in in my mind:

watchAndDevDocs: "tsc-watch --onSuccess npm run devDocs
watchAndDevTemplate: "tsc-watch --onSuccess npm run devDefaultTemplate"
watchAndDevMinimalTemplate: "tsc-watch --onSuccess npm run devMinimalTemplate"

If you like to save files every few seconds, and would prefer the process to be separate, then it'd be

• one of these https://markbind-master.netlify.app/devguide/workflow#editing-backend-features
• + npm run devDocs/Template/MinimalTemplate manually, then ctrl+C and rerun when desired