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

Start using TypeScript #871

Open
3 of 6 tasks
victorlin opened this issue May 20, 2024 · 9 comments
Open
3 of 6 tasks

Start using TypeScript #871

victorlin opened this issue May 20, 2024 · 9 comments

Comments

@victorlin
Copy link
Member

victorlin commented May 20, 2024

It would be nice to allow usage of TypeScript files in this project similar to how it was introduced in Auspice: nextstrain/auspice#1450

This would affect the way the site is built, but given the benefits of TypeScript, I think we're all fine with the added complexity (mentioned briefly in https://github.com/nextstrain/private/issues/88)

Tasks

Notes:

  1. "configure" means allow the ability to use .ts/.tsx files with curated options in tsconfig.json. Actual usage of TypeScript is expected to happen incrementally over time.
  2. We should create separate issues for the long-term items. Listing them here first for clarity and decision making purposes.
@genehack
Copy link
Contributor

big flaming +1

@jameshadfield
Copy link
Member

One salient difference between this project and Auspice is that Auspice has a bundler/transpiler layer whereas this repo runs directly through nodejs. In other words there's no "build stage" for the server.

Is this issue proposing using ts-node (or similar), or proposing adding a build stage, or something else?

@victorlin
Copy link
Member Author

there's no "build stage" for the server

Previously there wasn't, but now there is with the addition of Next.js, at least based on my understanding in #810 (comment).

I just noticed 2eedca4 already introduced traces of TypeScript: a tsconfig file and 2 TSX files. So it looks like it's already used behind the scenes by Next.js - I think we just need to make our own configurations to use it elsewhere.

@jameshadfield
Copy link
Member

jameshadfield commented May 20, 2024

Previously there wasn't, but now there is with the addition of Next.js

Not quite -- the frontend still has a build stage (edit: in production mode only, in dev mode it's compiled on-demand as pages are requested), but that's directly analogous to the previous set up where gatsby also had a build stage. With nextjs this now uses TS, and since it's no longer siloed into static-site/ quite like it used to be we have the top-level tsconfigs. However the server is still run directly though node -- underneath it all we're just running node server.js. To use TS this would have to change. There are many ways of making this change -- ts-node, tsc ... && node ..., build/transpilation/bundling/chaos etc. (I'm all for moving to TS, but I want to point out we are adding complexity here and should tread carefully.)

(Further complicating the layers involved here, nextjs running from within our server actually runs in its own (nodejs?) process and I suspect it's able to use TS files somehow, but AFAICS that doesn't help us introduce TS into our server. Lots of layers.)

@victorlin
Copy link
Member Author

I see – I forgot that there is still a big distinction between frontend and server even though they share the same package.json now. Yeah it would be worth trying a few options and comparing the complexities here - I haven't used any of them before.

since it's no longer siloed into static-site/ quite like it used to be we have the top-level tsconfigs

It's still under static-site/tsconfig.json and can't be moved up to top-level with the way the site is built currently.

@genehack
Copy link
Contributor

I see – I forgot that there is still a big distinction between frontend and server even though they share the same package.json now.

FWIW, $JOB[-1] had a monorepo with an Express-based backend app / API server and a Svelte-based front-end. They shared a package.json (IIRC anyway) and there wasn't a perceptively painful build step for the backend during normal development, only when we built a specific artifact for deployment (to AWS, as a "lambda-lith").

Happy to mock up a small demo repo with what that looked like if folks would like to kick the tires. (Note: this is not a proposal to convert the front-end to Svelte!)

@jameshadfield
Copy link
Member

This would affect the way the site is built, but given the benefits of TypeScript

Upon re-reading this today I realise there may be two separate conversations going on in this thread (and that may be on me!)

Adding TS to the (nodejs) frontend The frontend (static-site/) already uses TS, albeit only for a few files (e.g.). Type errors cause build failures, and thus CI fails. I think the TS config used is taken directly from the nextjs defaults, but we can totally change this as needed. Gradually moving more frontend files to TS seems a great idea.

Adding TS to the server This is what my comments in previous messages were focused on. It would (probably) introduce an extra layer, but as long as we picked well this wouldn't be too burdensome. Having worked in the server a bunch recently I would strongly support gradual introduction of TS. As part of this we could also move the resource indexer to TS in the same fashion.

The above paints these as separate siloed projects, which is essentially true at the moment. If we moved to SSR and shared information between our server and frontend pages (e.g. as I did in this prototype, and Tom's done elsewhere as well) then it's a little more complicated as we'd want to share type definitions between the silos.

@genehack
Copy link
Contributor

The above paints these as separate siloed projects, which is essentially true at the moment. If we moved to SSR and shared information between our server and frontend pages (e.g. as I did in this prototype, and Tom's done elsewhere as well) then it's a little more complicated as we'd want to share type definitions between the silos.

I'd urge doing that as a deliberate two-step (or three-step?) process: get everything using Typescript, get a good start on adding typing everywhere, and only then worry about refactoring out a set of common shared types.

@victorlin
Copy link
Member Author

I realise there may be two separate conversations going on in this thread (and that may be on me!)

Great to split these out - thanks for making the distinction. I've updated the issue description to reflect this.

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

No branches or pull requests

3 participants