Refactor wasm server and webworker by ImJeremyHe · Pull Request #393 · logisky/LogiSheets

ImJeremyHe · 2026-03-02T01:47:13Z

No description provided.

Copilot

Pull request overview

Refactors the web/wasm integration to use a unified RPC handle(...) entrypoint and a unified style-update payload model, while updating the UI layer to construct Transaction as an interface (via a helper) rather than via constructors/builders.

Changes:

Introduce tx(...) helper to build Transaction objects now that Transaction is an interface.
Replace legacy per-style payload builders with StyleUpdateType-based cellStyleUpdate / lineStyleUpdate payloads.
Switch packages/web wasm API calls to an RPC message handler and add Node RPC tests + Vitest config.

Reviewed changes

Copilot reviewed 50 out of 61 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/core/transaction.ts	Add `tx(...)` Transaction helper
src/components/toolbar/payload.ts	Migrate toolbar style payloads to unified style updates
src/components/toolbar/index.tsx	Replace `new Transaction(...)` with `tx(...)`
src/components/toolbar/border-setting.tsx	Replace Transaction ctor with `tx(...)`
src/components/sheets-tab/index.tsx	Replace Transaction ctor with `tx(...)`
src/components/sheets-tab/contextmenu.tsx	Replace Transaction ctor with `tx(...)`
src/components/format-dialog/num-fmt.tsx	Switch num format payloads + tx helper
src/components/format-dialog/border.tsx	Replace Transaction ctor with `tx(...)`
src/components/engine-canvas/index.tsx	Replace Transaction ctor with `tx(...)`
src/components/content/edit-bar.tsx	Replace Transaction ctor with `tx(...)`
src/components/block-interface/validation-cell.tsx	Replace TransactionBuilder usage with `tx(...)`
src/components/block-interface/menu.tsx	Replace Transaction ctor with `tx(...)`
src/components/block-interface/index.tsx	Replace Transaction ctor with `tx(...)`
src/components/block-interface/image.tsx	Replace Transaction ctor with `tx(...)`
src/components/block-interface/enum-cell.tsx	Replace Transaction ctor with `tx(...)`
src/components/block-interface/datetime-cell.tsx	Replace Transaction ctor with `tx(...)`
src/components/block-interface/bool-cell.tsx	Replace Transaction ctor with `tx(...)`
src/components/block-composer/index.tsx	Replace Transaction ctor with `tx(...)`
packages/web/src/types.ts	Remove Transaction/TransactionBuilder classes
packages/web/src/payloads/set_line_wrap_text.ts	Remove legacy style payload type (deleted)
packages/web/src/payloads/set_line_pattern_fill.ts	Remove legacy style payload type (deleted)
packages/web/src/payloads/set_line_num_fmt.ts	Remove legacy style payload type (deleted)
packages/web/src/payloads/set_line_font.ts	Remove legacy style payload type (deleted)
packages/web/src/payloads/set_line_border.ts	Remove legacy style payload type (deleted)
packages/web/src/payloads/set_line_alignment.ts	Remove legacy style payload type (deleted)
packages/web/src/payloads/set_field_num_fmt.ts	Remove legacy style payload type (deleted)
packages/web/src/payloads/set_cell_wrap_text.ts	Remove legacy style payload type (deleted)
packages/web/src/payloads/set_cell_pattern_fill.ts	Remove legacy style payload type (deleted)
packages/web/src/payloads/set_cell_num_fmt.ts	Remove legacy style payload type (deleted)
packages/web/src/payloads/set_cell_font.ts	Remove legacy style payload type (deleted)
packages/web/src/payloads/set_cell_border.ts	Remove legacy style payload type (deleted)
packages/web/src/payloads/set_cell_alignment.ts	Remove legacy style payload type (deleted)
packages/web/src/payloads/set_block_line_num_fmt.ts	Remove legacy style payload type (deleted)
packages/web/src/payloads/set_block_line_name_field.ts	Remove legacy style payload type (deleted)
packages/web/src/payloads/index.ts	Redefine Payload as EditPayload-only
packages/web/src/client.ts	Client now extends generated WorkbookMethods
packages/web/src/api/worksheet.ts	Replace direct wasm calls with RPC `handle(...)`
packages/web/src/api/workbook.ts	Replace transaction plumbing with RPC `handleTransaction`
packages/node/vitest.config.ts	Add Vitest configuration
packages/node/package.json	Add test script; adjust build pipeline
packages/node/tests/rpc.test.ts	Add RPC integration test suite
packages/engine/dist/types/lib/worker/workbook.worker.d.ts	Update generated worker typings
packages/engine/dist/types/lib/worker/types.d.ts	Update generated worker typings
packages/engine/dist/types/lib/index.d.ts	Update public re-exports for engine types
packages/engine/dist/types/lib/clients/workbook.d.ts	Update generated client typings
packages/engine/dist/types/lib/clients/service.d.ts	Update generated service typings
crates/wasms/server/src/ws.rs	Refactor ws functions to use sheet_idx + direction enum
crates/wasms/server/src/rpc.rs	Add RPC message/types + wasm `handle(...)` dispatcher
crates/wasms/server/src/lib.rs	Export rpc module
crates/wasms/server/src/controller.rs	Remove old per-payload wasm exports; add handle_transaction
crates/wasms/server/Cargo.toml	Add gents deps for TS RPC generation
crates/buildtools/src/generate.rs	Generate RPC TS types via `add_rpc`
crates/buildtools/Cargo.toml	Add wasm server dependency for type gen
crafts/markdown-table-extractor/src/actions.ts	Update to unified style update payloads + transaction interface
Cargo.toml	Bump gents/gents_derives workspace versions
Cargo.lock	Lockfile updates for new deps/versions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-02T01:54:36Z

+        fn(params: GetRowHeightParams, book_id: Option<usize>) -> Result<Value, ErrorMessage>,
+    pub get_col_width:
+        fn(params: GetColWidthParams, book_id: Option<usize>) -> Result<Value, ErrorMessage>,


WorkbookMethods.get_row_height / get_col_width are declared as returning Result<Value, ErrorMessage>, but the underlying RPC handlers return numeric row heights/column widths. This will generate incorrect TypeScript types (and encourage incorrect callers). Update these method signatures to return a numeric type (e.g. Result<f64, ErrorMessage> / Result<usize, ErrorMessage> as appropriate).

Suggested change

fn(params: GetRowHeightParams, book_id: Option<usize>) -> Result<Value, ErrorMessage>,

pub get_col_width:

fn(params: GetColWidthParams, book_id: Option<usize>) -> Result<Value, ErrorMessage>,

fn(params: GetRowHeightParams, book_id: Option<usize>) -> Result<f64, ErrorMessage>,

pub get_col_width:

fn(params: GetColWidthParams, book_id: Option<usize>) -> Result<f64, ErrorMessage>,

Copilot · 2026-03-02T01:54:36Z

+    pub get_display_window: fn(
+        params: GetDisplayWindowWithStartPointParams,
+        book_id: Option<usize>,
+    ) -> Result<DisplayWindowWithStartPoint, ErrorMessage>,
+
+    pub get_display_window_within_cell: fn(
+        params: GetDisplayWindowWithinCellParams,
+        book_id: Option<usize>,
+    )
+        -> Result<DisplayWindowWithStartPoint, ErrorMessage>,
+


WorkbookMethods.get_display_window is declared with GetDisplayWindowWithStartPointParams and returns DisplayWindowWithStartPoint, but the Message::GetDisplayWindow handler expects GetDisplayWindowParams (row/col bounds) and returns a DisplayWindow. This mismatch will generate incorrect TS typings and makes it unclear which method name corresponds to which behavior; align the method signature with the actual RPC message(s) (and add a separate get_display_window_with_start_point method if both are needed).

Copilot · 2026-03-02T01:54:37Z

+        fn(params: GetCellParams, book_id: Option<usize>) -> Result<Value, ErrorMessage>,
+    pub get_style: fn(params: GetCellParams, book_id: Option<usize>) -> Result<Value, ErrorMessage>,


WorkbookMethods.get_formula and get_style are declared as returning Result<Value, ErrorMessage>, but the worksheet/client code expects a formula string and a style object respectively. These types should match the concrete return types produced by ws::get_formula / ws::get_style to avoid incorrect TS generation and runtime misuse.

Suggested change

fn(params: GetCellParams, book_id: Option<usize>) -> Result<Value, ErrorMessage>,

pub get_style: fn(params: GetCellParams, book_id: Option<usize>) -> Result<Value, ErrorMessage>,

fn(params: GetCellParams, book_id: Option<usize>) -> Result<String, ErrorMessage>,

pub get_style: fn(params: GetCellParams, book_id: Option<usize>) -> Result<JsValue, ErrorMessage>,

Copilot · 2026-03-02T01:54:37Z

+    pub undo: fn(book_id: Option<usize>) -> Result<(), ErrorMessage>,
+    pub redo: fn(book_id: Option<usize>) -> Result<(), ErrorMessage>,


WorkbookMethods.undo/redo are declared as returning Result<(), ErrorMessage>, but the RPC implementation returns a boolean (whether an undo/redo occurred) and the JS clients treat it as such. Update the interface signature to return Result<bool, ErrorMessage> (or adjust the handler/client to return void consistently).

Suggested change

pub undo: fn(book_id: Option<usize>) -> Result<(), ErrorMessage>,

pub redo: fn(book_id: Option<usize>) -> Result<(), ErrorMessage>,

pub undo: fn(book_id: Option<usize>) -> Result<bool, ErrorMessage>,

pub redo: fn(book_id: Option<usize>) -> Result<bool, ErrorMessage>,

Copilot · 2026-03-02T01:54:37Z

+        return rpc(
+            'getDisplayWindow',
+            {sheetIdx: this._sheetId, startRow, endRow, startCol, endCol},
+            this._id
        )


Worksheet stores a sheet id (via getSheetId in the ctor) but then passes it as sheetIdx in RPC calls (e.g. getDisplayWindow). Since the server-side RPC params for these methods use sheet_idx and ws::get_sheet_by_idx(...), this will address the wrong sheet once sheet ids and indices diverge. Track both sheetIdx and sheetId (or rename the field) and pass the correct one for each RPC method.

Copilot · 2026-03-02T01:54:37Z

+        Message::GetCellsExceptWindow(params) => ws::get_cell_infos(
+            id,
+            params.sheet_idx,
+            params.start_row,
+            params.start_col,
+            params.end_row,
+            params.end_col,


getCellsExceptWindow is wired to ws::get_cell_infos(...), ignoring the window_* params entirely. This changes semantics from “cells except window” to “all cells in range” and breaks callers relying on the exclusion behavior. Either reintroduce a get_cell_infos_except_window implementation in ws and call it here, or update/remove the RPC method and its clients accordingly.

Suggested change

Message::GetCellsExceptWindow(params) => ws::get_cell_infos(

id,

params.sheet_idx,

params.start_row,

params.start_col,

params.end_row,

params.end_col,

Message::GetCellsExceptWindow(params) => ws::get_cell_infos_except_window(

id,

params.sheet_idx,

params.start_row,

params.start_col,

params.end_row,

params.end_col,

params.window_start_row,

params.window_start_col,

params.window_end_row,

params.window_end_col,

Refactor wasm server and webworker

28e179b

ImJeremyHe requested a review from Copilot March 2, 2026 01:47

Copilot started reviewing on behalf of ImJeremyHe March 2, 2026 01:47 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

Fix

2d2260c

ImJeremyHe force-pushed the jh/wasm-refactor branch from 0a6bcf4 to 2d2260c Compare March 2, 2026 04:00

Fix

62ed2db

ImJeremyHe force-pushed the jh/wasm-refactor branch from e442db3 to 62ed2db Compare March 2, 2026 06:51

ImJeremyHe merged commit 1c2a4ce into main Mar 2, 2026
2 checks passed

ImJeremyHe deleted the jh/wasm-refactor branch March 2, 2026 07:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor wasm server and webworker#393

Refactor wasm server and webworker#393
ImJeremyHe merged 3 commits intomainfrom
jh/wasm-refactor

ImJeremyHe commented Mar 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		fn(params: GetCellParams, book_id: Option<usize>) -> Result<Value, ErrorMessage>,
		pub get_style: fn(params: GetCellParams, book_id: Option<usize>) -> Result<Value, ErrorMessage>,

		pub undo: fn(book_id: Option<usize>) -> Result<(), ErrorMessage>,
		pub redo: fn(book_id: Option<usize>) -> Result<(), ErrorMessage>,

Conversation

ImJeremyHe commented Mar 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants