github-starred/actual
2
0
Fork 0
mirror of https://github.com/actualbudget/actual.git synced 2026-07-15 22:54:03 -05:00
Code Issues 3.0k Packages Projects Releases 58 Wiki Activity

[PR #4887] [MERGED] Enable Typescript in sync-server #112461

New Issue
Closed
opened 2026-06-05 18:03:56 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-06-05 18:03:56 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/actualbudget/actual/pull/4887
Author: @rgoldfinger
Created: 4/25/2025
Status: ✅ Merged
Merged: 5/6/2025
Merged by: @jfdoming

Base: master ← Head: rg-sync-server-ts

📝 Commits (10+)

📊 Changes

23 files changed (+71 additions, -66 deletions)

View changed files

📝 .github/workflows/build.yml (+2 -2)
📝 .github/workflows/docker-edge.yml (+1 -1)
📝 .github/workflows/docker-release.yml (+1 -1)
📝 .github/workflows/netlify-release.yml (+1 -1)
📝 .github/workflows/publish-npm-packages.yml (+1 -1)
📝 bin/package-electron (+1 -0)
📝 package.json (+1 -1)
📝 packages/desktop-client/src/components/settings/AuthSettings.tsx (+0 -2)
📝 packages/desktop-electron/index.ts (+1 -0)
📝 packages/sync-server/app.ts (+0 -0)
📝 packages/sync-server/docker/alpine.Dockerfile (+2 -3)
📝 packages/sync-server/docker/ubuntu.Dockerfile (+2 -3)
📝 packages/sync-server/package.json (+11 -14)
📝 packages/sync-server/src/app-sync.test.ts (+1 -0)
📝 packages/sync-server/src/app-sync.ts (+6 -3)
📝 packages/sync-server/src/app.ts (+9 -7)
📝 packages/sync-server/src/load-config.js (+3 -1)
📝 packages/sync-server/src/migrations.js (+1 -1)
📝 packages/sync-server/src/scripts/run-migrations.js (+1 -1)
📝 packages/sync-server/tsconfig.json (+12 -15)

...and 3 more files

📄 Description

Enable TS usage in sync server. Based on the work in https://github.com/actualbudget/actual/pull/4690 by alecbakholdin.

This PR focuses on enabling TS in sync-server and all of the tooling changes required to do so. Actually converting the files to TS can then be done in followup PR's.

Use cases:

  • packages/sync-server/package.json build/watch
    Continuing to use tsc to build and nodeman to watch

  • tests
    Confirmed vitest is working with ts files by converting both a test file, and the file it's testing, to typescript.

  • sync-server.Dockerfile, packages/sync-server/docker/ubuntu.Dockerfile, and alpine.Dockerfile
    Added sync-server build to bin/package-browser and adjusted the Dockerfile to copy in the build directory and run the app from there.

  • migrate db scripts in packages/sync-server/package.json and everything else in the scripts directory
    Moved run-migrations.js into scripts since it's a script.
    I went back and forth between requiring a build first or installing tsx and using it via yarn tsx. I landed on requiring build before running any scripts because it appears as though they assume there's a server running anyways. Feedback on this is appreciated. If we use tsx, we could use that everywhere in sync-server which would simplify things somewhat.

  • npm @actual-app/sync-server package
    Added sync-server build to bin/package-browser, and then include the build directory in the package.

  • electron build
    Added the sync server build to the electron build script, and pointed startSyncServer to the right file.
    Screenshot 2025-05-03 at 4 11 14 PM

  • Should we enable sourcemaps?

Early feedback, and feedback on any thing else that needs to be change that I've missed, is appreciated.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/actualbudget/actual/pull/4887 **Author:** [@rgoldfinger](https://github.com/rgoldfinger) **Created:** 4/25/2025 **Status:** ✅ Merged **Merged:** 5/6/2025 **Merged by:** [@jfdoming](https://github.com/jfdoming) **Base:** `master` ← **Head:** `rg-sync-server-ts` --- ### 📝 Commits (10+) - [`2b7e67b`](https://github.com/actualbudget/actual/commit/2b7e67b46108949db46d64b2fb71c8ec3955be57) attempt at running with typescript - [`ef09e5e`](https://github.com/actualbudget/actual/commit/ef09e5ed2a0234d802292f28be7807f57f3af493) release notes - [`b681254`](https://github.com/actualbudget/actual/commit/b6812541568ee425621f82b90c0a78c1601fb5f4) working jest tests for TS files - [`51548d0`](https://github.com/actualbudget/actual/commit/51548d0ad000bc58f19d0e3eb352d3f3afd5a3e7) working docker image build - [`66ba628`](https://github.com/actualbudget/actual/commit/66ba628bb01dfd64500e001f7b3c640dc3b53631) remaining docker images - [`3a19c4d`](https://github.com/actualbudget/actual/commit/3a19c4d847e5fbbe08747ce5c4cd564d375fc1c3) cleanup - [`03de202`](https://github.com/actualbudget/actual/commit/03de202f1e2da3ff74a72d6eebcd42242174aa7a) ensure vitest is working - [`2a53676`](https://github.com/actualbudget/actual/commit/2a53676765bf06a77c8691bd91e0812ad1199d5c) get tests passing in ci - [`c913ece`](https://github.com/actualbudget/actual/commit/c913ece8f808bb47edcdef175525a8e62605b0b7) less strict - [`7b9d4b4`](https://github.com/actualbudget/actual/commit/7b9d4b4b831c6da504735bb10a2b9cd1cd15c517) update release notes ### 📊 Changes **23 files changed** (+71 additions, -66 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/build.yml` (+2 -2) 📝 `.github/workflows/docker-edge.yml` (+1 -1) 📝 `.github/workflows/docker-release.yml` (+1 -1) 📝 `.github/workflows/netlify-release.yml` (+1 -1) 📝 `.github/workflows/publish-npm-packages.yml` (+1 -1) 📝 `bin/package-electron` (+1 -0) 📝 `package.json` (+1 -1) 📝 `packages/desktop-client/src/components/settings/AuthSettings.tsx` (+0 -2) 📝 `packages/desktop-electron/index.ts` (+1 -0) 📝 `packages/sync-server/app.ts` (+0 -0) 📝 `packages/sync-server/docker/alpine.Dockerfile` (+2 -3) 📝 `packages/sync-server/docker/ubuntu.Dockerfile` (+2 -3) 📝 `packages/sync-server/package.json` (+11 -14) 📝 `packages/sync-server/src/app-sync.test.ts` (+1 -0) 📝 `packages/sync-server/src/app-sync.ts` (+6 -3) 📝 `packages/sync-server/src/app.ts` (+9 -7) 📝 `packages/sync-server/src/load-config.js` (+3 -1) 📝 `packages/sync-server/src/migrations.js` (+1 -1) 📝 `packages/sync-server/src/scripts/run-migrations.js` (+1 -1) 📝 `packages/sync-server/tsconfig.json` (+12 -15) _...and 3 more files_ </details> ### 📄 Description Enable TS usage in sync server. Based on the work in https://github.com/actualbudget/actual/pull/4690 by [alecbakholdin](https://github.com/alecbakholdin). This PR focuses on enabling TS in sync-server and all of the tooling changes required to do so. Actually converting the files to TS can then be done in followup PR's. Use cases: - [x] packages/sync-server/package.json build/watch Continuing to use `tsc` to build and `nodeman` to watch - [x] tests Confirmed `vitest` is working with ts files by converting both a test file, and the file it's testing, to typescript. - [x] `sync-server.Dockerfile`, `packages/sync-server/docker/ubuntu.Dockerfile`, and `alpine.Dockerfile` Added sync-server build to `bin/package-browser` and adjusted the Dockerfile to copy in the `build` directory and run the app from there. - [x] migrate db scripts in packages/sync-server/package.json and everything else in the scripts directory Moved `run-migrations.js` into `scripts` since it's a script. I went back and forth between requiring a `build` first or installing `tsx` and using it via `yarn tsx`. I landed on requiring `build` before running any scripts because it appears as though they assume there's a server running anyways. Feedback on this is appreciated. If we use `tsx`, we could use that everywhere in sync-server which would simplify things somewhat. - [x] npm `@actual-app/sync-server` [package](https://github.com/actualbudget/actual/pull/4798/files#diff-c578f685cc90b866f33ea25fd2cb0dc92306a5d416baa5f5a51072326cee8e78) Added sync-server build to `bin/package-browser`, and then include the `build` directory in the package. - [x] electron build Added the sync server build to the electron build script, and pointed `startSyncServer` to the right file. <img width="635" alt="Screenshot 2025-05-03 at 4 11 14 PM" src="https://github.com/user-attachments/assets/6ee1e947-e93c-4d9e-aad0-a88c51ebfc93" /> - [ ] Should we enable sourcemaps? Early feedback, and feedback on any thing else that needs to be change that I've missed, is appreciated. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-06-05 18:03:57 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
AI generated
API
bank sync
budgeting
bug
can’t replicate
dependencies
docker
documentation
electron
experimental feature
feature
feedback
goal templates
good first issue
help wanted
importers
maintenance
needs info
needs testing
needs triage
needs votes
openid
payees
pull-request
Mirrored from GitHub Pull Request
 regression
reports
responsive
rules
schedules
server
✨ merged
split transactions
tech debt
tech-support
theme
transaction import
transaction reconciliation
transactions
translations
upstream
user interface
✅ approved
wontfix
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/actual#112461
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?