Modernizing code #8874
Replies: 3 comments 1 reply
-
I'd recommend that item 4 above be split into multiple parts, since async/await changes and validated queries tend to cause a lot of file changes (in particular with the reduction of indentation requirements) that end up losing history due to renaming. This is not as big a deal in pages, but it's a bigger concern in libs. So:
|
Beta Was this translation helpful? Give feedback.
-
A few more things we could add: Use the updated query functionsThe prairielearn/postgres package has been updated with new functions for making validated queries:
These new functions should be used in preference to the
Note that the Move code out of SQL sprocs and into TypeScriptPrairieLearn historically made heavy use of PostgreSQL stored procedures ("sprocs"). These allowed up to use a strongly-typed language for some of the more critical pieces of code (e.g., authentication, grading, syncing) when the rest of the code base was in JavaScript. Now that we are converting to TypeScript we want to rewrite our sprocs in TypeScript as well. This makes the code easier to read, test, and debug. As a general rule, a sproc should be converted to a single TypeScript function where all the logic is shifted into TypeScript and only the basic SQL queries are kept in SQL. To preserve the transactional nature of sprocs it is common to wrap the contents of the replacement TypeScript function in a Our eventual goal is to have zero sprocs in PL. This will also help us to manage DB connections using connection pooling between servers, which is problematic when using sprocs. Use "DB types" whenever possibleThe file We use a mixed casing convention where the DB types are in CamelCase but objects of these basic types are in In general we don't make derived types of the DB types to store extra data. Instead make a new container type (and schema) to store the extra data. For example, if we have a query that returns a list of courses and their enrollment counts then we would make a type like:
Move helper code into
|
Beta Was this translation helpful? Give feedback.
-
Additions to the recommended sequenceAfter step 6 (convert EJS to
|
Beta Was this translation helpful? Give feedback.
-
We've been working on modernizing the PrairieLearn codebase for a while now, but it's not always clear what "modernization" entails. This note tries to summarize exactly what modernization means, and provides a suggested sequence for modernizing code.
What does modern code look like?
In no particular order:
Replacing callbacks with
async
/await
"callback hell" is a very real problem. Libraries like
async
can make it more easy to deal with, but nothing beats native async code for its conciseness, reliable error handling, and ergonomics.TypeScript
Types are great; they help catch mistakes as we make them and allow us to more easily and confidently refactor code. Type checking can be enabled by adding
// @ts-check
to the top of*.js
files, or (preferably) by renaming*.js
files to*.ts
.Parsed/validated data
This goes hand-in-hand with TypeScript: we want to make sure that data entering our system from outside sources has the expected properties, shape, types, etc. Our library of choice for this is
zod
.The most common use case for this is SQL queries. We have a number of functions that will automatically parse query results with a Zod schema and ensure that the results are property typed:
Replacing CJS with ESM syntax
CommonJS has served us well, but we're starting to prepare for an ESM-first future. To that end, we're replacing CJS with ESM syntax where we can. At the moment, we're still transpiling ESM back to CJS at runtime, but converting everything to at least be authored in ESM will eventually make the transition to native ESM at runtime relatively straightforward. This transpilation step means there are still some rough edges, so constructs like
import foo = require('foo')
are sometimes required.Generating HTML in TypeScript
We've historically used EJS as our templating language for responses. However, it's impossible to use type checking with EJS, so we've started using our own homegrown
@prairielearn/html
library to generate HTML directly in TypeScript.Avoiding the use of
res.locals
res.locals
is difficult to type check and use. Where possible, prefer passing around well-typed objects instead ofres.locals
as a whole. And in route handlers, prefer querying directly instead of attaching data tores.locals
in a middleware and reading it in the route handler.We're not particularly dogmatic about this. While we want to avoid
res.locals
in brand-new code, we acknowledge that it will be a long journey to remove it from existing code. So, don't feel like you have to jump through hoops to avoid it, and if you have to use it in new code to work with existing code, that's fine too.Recommended sequence
// @ts-check
and making the minimum number of changes to get the TypeScript compiler happy with them.async
/await
ones..ts
file extension and replace JSDoc types with native types).html
tagged template literals. You may also do this as a part of the above step if you wish.Beta Was this translation helpful? Give feedback.
All reactions