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

WIP - Migrate backend to Deno #1395

Open
wants to merge 56 commits into
base: master
Choose a base branch
from

Conversation

snowteamer
Copy link
Collaborator

@snowteamer snowteamer commented Aug 28, 2022

Related to #586

Summary of changes:

  • New Deno backend, using my fork of the Pogo server framework.
    An okTurtles-based fork could be used as well.
  • All files in backend/ and shared/ have been converted to TypeScript.
  • Some Flowtype definitions in shared/ were still necessary and have been moved to new types.flow.js files in their respective folders.
  • Grunt tasks backend:launch and backend:relaunch have been replaced with deno:start.
  • New deno:stop and flow:stop tasks, used to avoid dangling process issues.
  • New exec:ts task for typechecking backend and shared files.

Additional info:

Known issues and limitation:

  • Everything but the backend is still based on Nodejs. I think migrating one thing at a time is likely easier.

Remaining tasks:

  • Use Deno's --watch flag to implement npm backend script equivalent functionality without using Nodemon
  • Annotate backend files so they pass typechecking with deno check
  • Fix Flow errors in shared files
  • Replace Mocha by deno test for backend unit tests

@snowteamer snowteamer force-pushed the add-deno-version-without-pogo branch from 3bdba00 to 4a37434 Compare August 29, 2022 16:09
@snowteamer snowteamer self-assigned this Aug 29, 2022
backend/database.ts Outdated Show resolved Hide resolved
@snowteamer snowteamer changed the title Migrate backend to Deno WIP - Migrate backend to Deno Sep 1, 2022
Copy link
Member

@taoeffect taoeffect left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really impressive work @snowteamer! This involved a deep understanding of the differences between Node and Deno.

This is just a preliminary review, nothing serious, two change requests:

  • Use .push instead of .length for streaming APIs, and in general get them to match the original code as much as possible EDIT: oops, seems like you already did this!
  • Go through the logging stuff and try to match the original logging as closely as possible

@snowteamer snowteamer force-pushed the add-deno-version-without-pogo branch 2 times, most recently from 7cb4f8c to 1ea0bb6 Compare October 20, 2022 14:37
@taoeffect
Copy link
Member

Wonder if these updates to Deno are relevant? https://deno.com/blog/v1.35

@taoeffect taoeffect added Note:Research Note:Stale Closed due to inactivity / relevance / memory labels Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Note:Research Note:Stale Closed due to inactivity / relevance / memory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants