Skip to content

Conversation

@sawka
Copy link
Member

@sawka sawka commented Nov 11, 2025

new VDomFormData and VDomFileData (and an async path for event handling on the FE)

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 11, 2025

Walkthrough

This pull request implements form and file upload support in the VDOM system by introducing new type definitions and event handling logic across TypeScript and Go codebases. It adds VDomFormData and VDomFileData types, extends VDomEvent to carry form and file metadata, and updates TsunamiModel with async event processing for form submissions and file field changes. A new base64 utility module is added to support file data encoding, with files limited to 5MB and stored as Base64 content within VDOM payloads.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • tsunami/frontend/src/util/base64.ts: Verify base64-js integration and handling of edge cases (null/empty inputs, whitespace stripping in base64ToArrayBuffer)
  • tsunami/vdom/vdom_types.go: Confirm VDomFormData.GetField method properly handles nil maps and empty field lists; review omitempty JSON tags on new VDomEvent fields
  • tsunami/frontend/src/model/tsunami-model.tsx: Examine async event processing logic in asyncAnnotateEvent and verify 5MB file size enforcement in fileToVDomFileData; validate type cast on currentModal atom and deferred batch pushing
  • Cross-file consistency: Ensure TypeScript VDomFileData/VDomFormData definitions align with Go struct definitions, and that Base64 encoding/decoding is handled consistently across boundaries

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding support for onSubmit and onChange event handling for file inputs in the Tsunami framework.
Description check ✅ Passed The description is related to the changeset, mentioning the introduction of VDomFormData, VDomFileData, and an async event handling path for file inputs.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sawka/tsunami-onsubmit

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.5.0)

Error: unknown linters: 'unusedfunc,unusedparams', run 'golangci-lint help linters' to see the list of supported linters
The command is terminated due to an error: unknown linters: 'unusedfunc,unusedparams', run 'golangci-lint help linters' to see the list of supported linters


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f12ae1e and 87f2786.

📒 Files selected for processing (4)
  • tsunami/frontend/src/model/tsunami-model.tsx (6 hunks)
  • tsunami/frontend/src/types/vdom.d.ts (2 hunks)
  • tsunami/frontend/src/util/base64.ts (1 hunks)
  • tsunami/vdom/vdom_types.go (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tsunami/frontend/src/model/tsunami-model.tsx (2)
tsunami/vdom/vdom_types.go (3)
  • VDomFileData (138-145)
  • VDomEvent (65-77)
  • VDomFormData (117-125)
tsunami/frontend/src/util/base64.ts (1)
  • arrayBufferToBase64 (34-37)
tsunami/frontend/src/types/vdom.d.ts (1)
tsunami/vdom/vdom_types.go (2)
  • VDomFileData (138-145)
  • VDomFormData (117-125)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (go)
  • GitHub Check: Build for TestDriver.ai

Comment on lines +720 to +732
const needsAsync =
propName == "onSubmit" ||
(propName == "onChange" && (e.target as HTMLInputElement)?.type === "file");
if (needsAsync) {
asyncAnnotateEvent(vdomEvent, propName, e)
.then(() => {
this.batchedEvents.push(vdomEvent);
this.queueUpdate(true, "event");
})
.catch((err) => {
console.error("Error processing event:", err);
});
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Restore deterministic ordering for async form/file events

Line 724 introduces an async branch that only pushes the event after fileToVDomFileData resolves. Because each handler now resolves independently, a later (smaller) file can finish first, enter the batch, and trigger queueUpdate before an earlier (larger) file has finished reading. The backend will then observe the events out of chronological order, breaking correctness for workflows that depend on deterministic sequencing of changes/submits. Please serialize these asynchronous annotations so they flush to batchedEvents in the exact order the handlers fire.

Apply this diff to chain async work per event:

         if (needsAsync) {
-            asyncAnnotateEvent(vdomEvent, propName, e)
-                .then(() => {
-                    this.batchedEvents.push(vdomEvent);
-                    this.queueUpdate(true, "event");
-                })
-                .catch((err) => {
-                    console.error("Error processing event:", err);
-                });
+            this.pendingAsyncEventChain = this.pendingAsyncEventChain
+                .then(async () => {
+                    await asyncAnnotateEvent(vdomEvent, propName, e);
+                    this.batchedEvents.push(vdomEvent);
+                    this.queueUpdate(true, "event");
+                })
+                .catch((err) => {
+                    console.error("Error processing event:", err);
+                });

And add the supporting field in the class body:

pendingAsyncEventChain: Promise<void> = Promise.resolve();
🤖 Prompt for AI Agents
In tsunami/frontend/src/model/tsunami-model.tsx around lines 720 to 732, async
event annotation for file/onSubmit handlers currently races and can flush
batchedEvents out of chronological order; add a class field
pendingAsyncEventChain: Promise<void> = Promise.resolve() and change the
needsAsync branch to serialize work by chaining onto pendingAsyncEventChain
(e.g. pendingAsyncEventChain = this.pendingAsyncEventChain.then(() =>
asyncAnnotateEvent(vdomEvent, propName, e).then(() => {
this.batchedEvents.push(vdomEvent); this.queueUpdate(true, "event");
}).catch(err => { console.error("Error processing event:", err); }))); this
preserves ordering by ensuring each async annotation+push/queue runs only after
the previous one completes and errors are handled so the chain continues.

@sawka sawka merged commit 7955bf6 into main Nov 11, 2025
7 of 8 checks passed
@sawka sawka deleted the sawka/tsunami-onsubmit branch November 11, 2025 04:00
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.

2 participants