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

Refactor onto Fastify #26

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Zegnat
Copy link
Contributor

@Zegnat Zegnat commented Jul 30, 2023

Spoke with @capjamesg about this before. And I had some time to give this a go now 😊

Fastify responds to more than 2× the requests in the same time as Express. Making it a better choice on servers where resources are shared with other applications. (There are even more optimised frameworks, but they will often be a developer experience downgrade, lack other things, or opt to completely replace Node.js’ HTTP code with their own C++ implementation.) For ease and stability, I chose to stick to Fastify rather than go more bleeding edge.

With that said, Fastify has gotten many more updates than Express. The latest releases are from 2023-07-27 and 2022-10-08 respectively. So while you may always be hearing about Express, that is really not the most up-to-date framework to start on anymore.

The other reason why I brought up moving away from Express was that other frameworks, like Fastify, have gotten a lot better at handling promise rejections. Rather than crashing when something throws, it will return an HTTP 500 by default. Much how you might be used to coming from PHP.

This PR does the following (and supersedes #21):

  1. Start using LTS version of Node.js.
  2. Drop unnecessary dependencies, such as node-fetch.
  3. Swap Express with Fastify.
  4. Flatten the Promise callbacks using awaits to ensure the request handlers always return a reply.
  5. Update the test file to actually await the tests finishing.

Because of the code flattening, I recommend looking at the diff ignoring whitespace changes. That will make it easier to spot the actual logic changes vs just flattening.

Because this PR is also for others to learn a little about Fastify, here is what changed coming from Express (I might miss something):

  • Fastify comes with build in logging. This PR switches over to using its logging (at an info level) for everything that previously used the local log() function. The debug_log setting variable is still used and can be set to false to disable all log output coming from Fastify.
  • There is no built-in method to serve a directory as static assets, so this PR pulls in @fastify/static. This ends up almost being a drop-in replacement for express.static().
  • nunjucks has an option where it registers itself onto the Express app instance. This does not work with Fastify. Instead, Fastify has an optional rendering system in @fastify/view. The rendering system now instantiates nunjucks, not the other way around, so all the settings have been moved to be done inside. (This ends up really only being a whitespace change, none of the nun_env.* code needed changing.)
  • The Fastify rendering system uses .view() on the response object, rather than .render() of Express. This ends up being a simple find and replace.
  • The Fastify request handlers are somewhat expected to return a reply context. So every use of .view() has been made to return. This also ensures that there is never an instance where it is tried to render responses twice. (Flattening the promises also helps to make sure that there is never an asynchronous process still running that might try to respond.)
  • Finally the .listen() is changed to follow the new syntax.

A lot of these changes are simple find and replaces. A Codemod could have probably done it all.

Please have a look, and let me know if you would consider merging this, whether there are any questions, or if you would rather continue with Express. (In the case of the latter, #21 is not superseded and could still be worth merging 😉 )

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

Successfully merging this pull request may close these issues.

1 participant