docs: improve connectivity#182
Open
LuLaValva wants to merge 4 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdded a Forms guide covering native submission, server handlers, validation, reactive input binding, and reusable fields. Expanded duplicate-submission guidance with submission keys and redirects. Registered the new route in generated typings and added both guides to site navigation. Updated routing, onboarding, TypeScript, styling, state, serialization, reactivity, and tutorial documentation with clarifications, examples, cross-references, and next-step links. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/explanation/serializable-state.md`:
- Line 25: Clarify the statement near “Fine-Grained Bundling” to distinguish the
HTML response from the serialized client-side state payload: rendered state
remains part of the HTML, while only its unnecessary client-side serialization
can be omitted when no browser logic references it.
In `@docs/guide/duplicate-form-submissions.md`:
- Around line 12-16: Update the native form’s onSubmit handler to return
immediately when submitting is already true, while still setting submitting to
true for the first submission; retain the disabled button as a UI safeguard. In
the fetch example, keep event.preventDefault() before the submitting
early-return check.
In `@docs/guide/forms.md`:
- Line 5: Shorten the “Submitting to the Server” Markdown heading to a concise
1–3 word title, such as “Server Submission,” while preserving its heading level.
- Around line 22-25: Validate the value returned by FormData.get("message") in
POST before calling saveFeedback: accept only non-blank strings, reject null,
File, and other non-string values, and preserve the existing redirect for valid
submissions.
In `@docs/tutorial/fundamentals.md`:
- Around line 121-123: Update the note following the button examples in the
fundamentals tutorial to describe event handlers as the mechanism that can make
buttons respond to clicks, rather than implying the displayed buttons already
include handlers. Preserve the existing links to the event-handlers reference
and Components and Reactivity guide.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c3553eb-27a5-45e8-ade5-891f975d374c
📒 Files selected for processing (13)
.marko-run/routes.d.tsdocs/explanation/immutable-state.mddocs/explanation/nested-reactivity.mddocs/explanation/serializable-state.mddocs/guide/duplicate-form-submissions.mddocs/guide/forms.mddocs/guide/styling.mddocs/marko-run/file-based-routing.mddocs/marko-run/getting-started.mddocs/marko-run/typescript.mddocs/tutorial/components-and-reactivity.mddocs/tutorial/fundamentals.mdsrc/tags/app-menu/app-menu.marko
💤 Files with no reviewable changes (1)
- docs/explanation/nested-reactivity.md
LuLaValva
force-pushed
the
docs-improvements
branch
from
July 15, 2026 16:30
0060d10 to
b464694
Compare
Contributor
PR Preview DeployedYour changes are live at markojs.com/previews/pr-182. commit 39b1e3e |
- Fix the graceful degradation example: a submit button disabled from bound state renders disabled in server HTML, so drop it and warn about the pattern - Add Binding Controls section covering checkedValue for radios and checkbox groups, the enhanced select value attribute, and refining functions for numeric inputs - Add Reusable Fields section showing <id> for label association and forwarding value/valueChange through a custom tag
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/guide/forms.md`:
- Line 106: Update the form-submission explanation around the sentence beginning
“Each control” to state that only successful, named controls submit their values
natively, while preserving the point that bound variables let the rest of the
template react.
- Around line 109-115: Revise the form-control guidance around the value binding
example to explicitly scope the string-conversion behavior to value bindings.
Clarify that checked produces booleans and checkedValue may produce arrays for
checkbox groups, while preserving the warning and parseFloat refining-function
example for value:=quantity.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e6f369e1-472c-4c60-8adc-2c5973ebb90c
📒 Files selected for processing (1)
docs/guide/forms.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/guide/duplicate-form-submissions.md (1)
1-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the required TLDR callout and shorten the heading.
Use a 1-3 word heading and add a
> [!TLDR]summary immediately afterward.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guide/duplicate-form-submissions.md` around lines 1 - 3, Shorten the document heading to 1–3 words and immediately follow it with a > [!TLDR] callout summarizing the guidance that duplicate form submissions must be handled safely by the server.Sources: Coding guidelines, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/guide/duplicate-form-submissions.md`:
- Around line 42-48: Update the saveRsvpOnce guidance to require a
database-level unique index or constraint on submissionId, and use an atomic
insert or upsert rather than a separate existence check and insert. Handle the
duplicate-key outcome by returning the existing RSVP record, preserving the
redirect flow shown in the example.
- Around line 3-14: Update the duplicate-submission guidance around submissionId
to state that per-render keys only deduplicate retries or repeated submissions
of the same rendered form, not submissions from multiple tabs. If cross-tab
deduplication is required, direct readers to use a stable operation-level key.
---
Nitpick comments:
In `@docs/guide/duplicate-form-submissions.md`:
- Around line 1-3: Shorten the document heading to 1–3 words and immediately
follow it with a > [!TLDR] callout summarizing the guidance that duplicate form
submissions must be handled safely by the server.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 885b9993-706a-4208-aa96-8b4adca6a7e1
📒 Files selected for processing (2)
docs/guide/duplicate-form-submissions.mddocs/guide/forms.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/guide/forms.md
Comment on lines
+3
to
+14
| A native form sends an HTTP request and follows the server's response. The browser may send the same request more than once because of repeated submissions, retries, or multiple open tabs, so correctness cannot depend on disabling a button. The server should make repeated submissions safe. | ||
|
|
||
| ## Submission Keys | ||
|
|
||
| Include a unique submission key in the rendered form. Because the page renders on the server, the form works without browser JavaScript. | ||
|
|
||
| ```marko | ||
| /* src/routes/rsvp/+page.marko */ | ||
| <const/submissionId=crypto.randomUUID()> | ||
|
|
||
| <form method="POST" action="/rsvp"> | ||
| <input type="hidden" name="submissionId" value=submissionId> |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not claim that per-render keys protect multiple tabs.
Each rendered tab receives a new submissionId, so two tabs can create two RSVP records. Limit this claim to retries and repeated submissions of the same rendered form, or document a stable operation-level key when cross-tab deduplication is required.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/guide/duplicate-form-submissions.md` around lines 3 - 14, Update the
duplicate-submission guidance around submissionId to state that per-render keys
only deduplicate retries or repeated submissions of the same rendered form, not
submissions from multiple tabs. If cross-tab deduplication is required, direct
readers to use a stable operation-level key.
Comment on lines
+42
to
+48
| const rsvp = await saveRsvpOnce({ submissionId, guests }); | ||
| return context.redirect(`/rsvp/${rsvp.id}`, 303); | ||
| } | ||
| ``` | ||
|
|
||
| > [!WARNING] | ||
| > `saveRsvpOnce` must enforce uniqueness for the submission key in the same database transaction that creates the record. Checking for an existing key and inserting in separate operations leaves a race condition. |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Require database-enforced uniqueness explicitly.
A check-then-insert operation can still race even when both statements share a transaction. Specify a unique index or constraint on submissionId, plus an atomic insert/upsert that handles the duplicate-key result by returning the existing record.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/guide/duplicate-form-submissions.md` around lines 42 - 48, Update the
saveRsvpOnce guidance to require a database-level unique index or constraint on
submissionId, and use an atomic insert or upsert rather than a separate
existence check and insert. Handle the duplicate-key outcome by returning the
existing RSVP record, preserving the redirect flow shown in the example.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I ran a few AI personas and found out where the "sinks" are in our docs. Hopefully the links added here make the full documentation surface more reachable by readers.