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

Lf 4395 setup typescript on backend #3428

Open
wants to merge 22 commits into
base: integration
Choose a base branch
from

Conversation

navDhammu
Copy link
Collaborator

@navDhammu navDhammu commented Sep 11, 2024

This sets up typescript on the backend. New and existing files in src and tests should work now when converted to .ts. Anything in /db including the migrations still need to use .js but that can be setup with typescript as well if needed. I tested it to make sure its good for development and adjusted the npm scripts for production and integration, but will need some help to ensure nothing is broken in production as I'm not fully aware of how everything is setup with docker, for example there's prod.DockerFile which uses some npm commands that may not work.

Overview

In order to get typescript working on the backend, it has to first be transpiled to js so it can be executed by node as node doesn't support running ts files natively. Although that may change in the near future with this new experimental feature in latest release.

Tsc is the official compiler shipped with typescript that does the transpilation from ts to js, and it's what I started working with initially but then switched to using swc after running into problems with jest. Swc is an alternate to tsc that's written in rust so its much faster and provides a better dev experience, there are others like esbuild which is used by vite on the frontend. I chose swc because they provide a package swc/jest that takes care of the problems I was having initially getting typescript and es modules working with our current test setup with jest. A nice side effect of using this package is that tests are running faster now, I'm seeing api tests in github actions completing in about 3 minutes instead of 6.

For development, theres still this issue of having to first compile all source code to .js before running the node server. So the workflow needs to be: make a change -- run the compiler -- execute the compiled server.js file. To make all this more straightforward there's this popular package ts-node that allows executing .ts files directly, so it makes it possible to directly execute ts-node server.ts. It does this by compiling typescript to javascript on the fly and passing the resulting javascript code to node. I found a similar well maintained package swc-node that does the same thing but uses the swc compiler, so I ended up going with that.

To make things fast during development, this setup doesn't run type-checking on every code change, but instead its done in the pre-commit just like on frontend.

Changes

  • Requires node version >=20.6. This is enforced using the engines option in package.json and engine-strict=true in .npmrc. So when using an npm command like npm install, it will give an error if using the node version doesn't match this.
  • For development - npm run dev replaces npm run nodemon. Nodemon is not needed as node.js comes with watch mode after node v18
  • For production - the source code is compiled to /dist via tsc, and the server can be started by executing node dist/api/src/server.js
  • Babel is replaced with swc
  • There are 2 separate tsconfigs, one for root dir /api and one for /api/src. The root tsconfig is only responsible for type-checking which is run in precommit, and this includes the tests as well. The api/src tsconfig is for building the source code for production.
  • When importing typescript files, you need to import them with the .js extension like import something from file.js - not file.ts. This is because when building the code for production tsc will change all typescript code to javascript including the file extension to .js, but won't change the import statements. So anywhere an import is requiring a file with a .ts extension, it won't exist in the production build which will make the app crash.

Jira link:

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Passes test case
  • UI components visually reviewed on desktop view
  • UI components visually reviewed on mobile view
  • Other (please explain)

Checklist:

  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • The precommit and linting ran successfully
  • I have added or updated language tags for text that's part of the UI
  • I have added "MISSING" for all new language tags to languages I don't speak
  • I have added the GNU General Public License to all new files

@navDhammu navDhammu force-pushed the LF-4395-setup-typescript-on-backend branch from 714f667 to 84f3923 Compare September 29, 2024 15:12
@navDhammu navDhammu marked this pull request as ready for review September 30, 2024 16:41
@navDhammu navDhammu requested review from a team as code owners September 30, 2024 16:41
@navDhammu navDhammu requested review from antsgar and Duncan-Brain and removed request for a team September 30, 2024 16:41
@navDhammu navDhammu added the enhancement New feature or request label Sep 30, 2024
@antsgar
Copy link
Collaborator

antsgar commented Oct 2, 2024

Wow, thank you so much for putting this together! The PR description was super helpful to understand the changes and the decision making process, amazing work 👏 I tested it out and it all seems to be working properly -- for the Dockerfile, I'm assuming we'll need to update the Node version and the last piece to remove installing nodemon and using it to start the server. I'm thinking we could check out this branch on the beta server and test the changes.

Right now we're using the same Node version for all the packages, so I think it might be good to update all of them to 20 which is LTS anyway.

We should also update the Readme instructions on the parts it refers to Nodemon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants