docs: update right colored diffs #596
Conversation
WalkthroughUpdated a documentation tutorial file with inline annotation comments and minor formatting edits; also removed the unused Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
docs/tutorial.md (1)
1494-1496: Fix syntax and diff consistency in PATCH exampleThere’s a syntax error and an unannotated addition:
- Extra closing parenthesis at the end of the
note.updatereturn.- The new
body: 'memo'line isn’t marked as an addition, which breaks “right-colored” diff consistency in this block.Apply this patch:
- ({ note, params: { index }, body: { data }, status }) => { // [!code --] - if (index in note.data) return note.update(index, data) // [!code --] - ({ note, params: { index }, body: { data }, status, username }) => { // [!code ++] - if (index in note.data) // [!code ++] - return note.update(index, { data, author: username })) // [!code ++] + ({ note, params: { index }, body: { data }, status }) => { // [!code --] + if (index in note.data) return note.update(index, data) // [!code --] + ({ note, params: { index }, body: { data }, status, username }) => { // [!code ++] + if (index in note.data) // [!code ++] + return note.update(index, { data, author: username }) // [!code ++] @@ - body: 'memo' + body: 'memo' // [!code ++]Also, consider removing the old handler entirely in this snippet to avoid confusion, relying solely on the [!code --]/[!code ++] annotations to communicate the change.
Also applies to: 1500-1504
🧹 Nitpick comments (2)
docs/tutorial.md (2)
1703-1712: Nit: unused import in observability example
tisn’t used in this block.-import { Elysia, t } from 'elysia' +import { Elysia } from 'elysia'
24-24: Minor grammar/wording fixes to polish the doc
- Line 24: “Elysia feature” → “Elysia features”
- Line 30: “From other framework?” → “From other frameworks?”
- Line 77: “support Web Standard API” → “support the Web Standard API”
- Line 108: “barebone project” → “bare-bones project”
- Line 118: “Elysia use Bun” → “Elysia uses Bun”
- Line 157: “both value and function” → “both values and functions”
- Line 159: “use function to access” → “use a function to access”
- Line 1311: “encapsulate lifecycle” → “encapsulates lifecycle”
- Line 1612: “We use use” → “We use”
- Line 2370: “And- that’s it” → “And that’s it”
- Line 2374: “You could to take a step further” → “You could take it a step further”
Happy to open a quick follow-up PR with these copyedits if desired.
Also applies to: 30-30, 77-77, 108-108, 118-118, 157-157, 159-159, 1311-1311, 1612-1612, 2370-2370, 2374-2374
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/tutorial.md(15 hunks)
🔇 Additional comments (24)
docs/tutorial.md (24)
131-133: Route example enhancement is correctAdds a second GET route to demonstrate simple value responses. Looks good.
150-155: Diff annotations for method and handler changes are consistent
- GET → POST swap is properly marked.
- Handler updated to use Context.path is correctly highlighted.
Also applies to: 162-169
190-194: Swagger plugin usage is correctly introducedApplying
.use(swagger())before routes matches best practices and later references.
216-224: Decorator example reads cleanlyIntroducing Note and injecting via
decorateis accurate and consistent with later usage.
253-255: Path param snippet is fine (TS warning addressed later)Defines
:indexcleanly; later “Validation” section resolves the typing warning.Ensure the TS playground blocks (twoslash) show the expected hint for untyped params, so readers see the value of the follow-up validation step.
274-296: Validation witht.Objectis introduced correctly
params: t.Object({ index: t.Number() })and importingtare correct and align with later guard usage.
345-347: 404 viastatushelper is demonstrated properlyClear, minimal example of returning a 404 when the index is missing.
375-377: Custom 404 message example is correctGood follow-up to show a custom message variant.
427-450: Plugin extraction in index.ts is coherent
- Importing
noteplugin and removing the inline Note routes are consistently annotated.- Ensures the main app is decluttered.
472-485: CRUD additions are technically sound and easy to follow
- Methods
add,remove,updateon Note are idiomatic for the tutorial.- Route validations for body/params match their handlers.
Also applies to: 490-494, 506-518, 519-534
550-553: Prefix grouping is a good simplificationSwitching to
new Elysia({ prefix: '/note' })and updating paths to relative is consistent.
620-626: Guard consolidation is correctCentralizes
paramsvalidation and removes redundancy in per-route schemas. Annotations accurately show removals.Also applies to: 632-637, 647-652, 662-666
690-695: LifecycleonTransformis placed correctlyHook is added before routes; logs are minimal and illustrative.
749-818: Auth:usermodule scaffolding looks good
statestores and cookie schema are reasonable for a tutorial.- Sign-up/sign-in flows are clear; diff annotations consistent.
For readers running Bun: confirm
Bun.password.hash/verifywork as-is for the example versions used in docs.
859-880: Reference models are introduced well
signIn,session, andoptionalSessionmodels reduce duplication effectively.
1023-1056: Sign-out and profile routes are consistent with earlier models
- Uses modeled cookies correctly.
token.remove()is a good demonstration.
1074-1107: Service/plugin deduplication is accurate
- Naming the service plugin prevents duplicate registration.
- Properly removes duplicate state/model from
useronceuserServiceis used.Also applies to: 1109-1142
1189-1217: MacroisSignInis correctly structuredReturning
beforeHandlevia macro is a neat pattern; annotations clearly mark new lines.
1228-1250: Macro usage reads wellUsing
isSignIn: truelets you remove imperative checks from the handler. Good example.
1268-1278: Resolve pattern is demonstrated clearlyDefining
usernamevia.resolveafter a guard is coherent and matches the narrative.
1328-1334: Scoped lifecycle usage is correctBoth inline
as: 'scoped'and chained.as('scoped')are shown; nice to include both.Also applies to: 1364-1364
1410-1416: Authorization refactor is solid (Memo schema + typed updates)
- Introducing
memoand usingt.Omitfor body is great.- Updating Note methods to
Memo/Partial<Memo>is consistent.- PUT now attaches author via
username; good.Also applies to: 1419-1444, 1450-1451, 1464-1469
1547-1567: Final authorized note controller is coherent
.use(userService)then.use(getUserId)before protected routes is correct.- Validations and macro usage align with previous sections.
Also applies to: 1569-1601
1626-1631: onError examples are well placed and accurateShowing both “ignore NOT_FOUND” and “custom 404 body” variations is helpful.
Also applies to: 1660-1663
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
docs/tutorial.md (1)
1478-1481: Syntax error: extra closing parenthesis in patch handlerThere’s an extra “)” after note.update(...), which will break the example.
- ({ note, params: { index }, body: { data }, status, username }) => { // [!code ++] - if (index in note.data) // [!code ++] - return note.update(index, { data, author: username })) // [!code ++] + ({ note, params: { index }, body: { data }, status, username }) => { // [!code ++] + if (index in note.data) // [!code ++] + return note.update(index, { data, author: username }) // [!code ++]
🧹 Nitpick comments (4)
docs/tutorial.md (4)
690-695: Be careful logging request bodies and paramsLogging body and params verbatim can leak sensitive info in real apps. Consider redacting or filtering fields (e.g., passwords) before logging.
- .onTransform(function log({ body, params, path, request: { method } }) { - console.log(`${method} ${path}`, { - body, - params - }) - }) + .onTransform(function log({ body, params, path, request: { method } }) { + const redactedBody = body && typeof body === 'object' + ? { ...body, password: body.password ? '<redacted>' : body.password } + : body + console.log(`${method} ${path}`, { + body: redactedBody, + params + }) + })
749-820: Auth examples are clear; minor security note on cookie optionsExamples use hashing and cookies correctly for a tutorial. For production, ensure cookies are set httpOnly, secure, and with SameSite to mitigate CSRF; these are set when issuing cookies, not in the t.Cookie model schema.
796-803: Session token generation is weak for productionUsing a 32-bit random number as a session token is guessable at scale. Prefer a cryptographically strong, high-entropy identifier (e.g., UUIDv4 or 128-bit random bytes). This will require changing the cookie and session map to string keys.
- const key = crypto.getRandomValues(new Uint32Array(1))[0] - session[key] = username - token.value = key + const key = crypto.randomUUID() + session[key] = username + token.value = keyAnd update models/store accordingly:
- session: {} as Record<number, string> + session: {} as Record<string, string>- token: t.Number() + token: t.String()
1023-1036: Use POST for sign-out to avoid unsafe GET side-effectsGET should be safe/idempotent; logging out mutates state and is susceptible to CSRF. Prefer POST here.
- .get( + .post( '/sign-out', ({ cookie: { token } }) => { token.remove() return { success: true, message: 'Signed out' } }, { cookie: 'optionalSession' } )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/tutorial.md(23 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/tutorial.md
1453-1453: Hard tabs
Column: 7
(MD010, no-hard-tabs)
🔇 Additional comments (13)
docs/tutorial.md (13)
131-133: Right-diff callouts moved inline — looks goodPlacing [!code ++] at EOL inside TS comments is correct for VitePress callouts and improves readability.
189-194: Swagger plugin callout placement — goodInline callout reads clearly; no functional change to the example.
216-224: Decorate example reads cleanlyInline callouts on class and decorator lines are consistent and readable.
253-256: Path parameter example — callouts OKEOL callouts are consistent. Example compiles conceptually as-is.
290-296: Validation block callouts — OKGood placement on opening/closing braces and schema fields.
345-347: Status handling callouts — OKAdding status() call is clearly marked and correct in context.
472-485: CRUD additions — examples look correctCallout placement is consistent across put/delete/patch handlers and schemas.
Also applies to: 490-495, 506-518, 519-534
550-553: Grouping with prefix — goodSwitching to a prefixed instance and updating routes improves clarity.
620-626: Guard extraction — nice reduction of duplicationMoving params validation into guard and removing per-route schemas is the right call.
Also applies to: 631-637, 647-653, 662-666
1395-1401: Fix hard tabs flagged by markdownlint (MD010) in this blockReplace hard tabs with spaces for consistency and to satisfy MD010. Example:
-const memo = t.Object({ - data: t.String(), // [!code ++] - author: t.String() // [!code ++] -}) // [!code ++] +const memo = t.Object({ + data: t.String(), // [!code ++] + author: t.String() // [!code ++] +}) // [!code ++]
1451-1454: Fix hard tab indentation (MD010) here as wellThe linter reports a tab at Line 1453, replace tabs with spaces in this small block.
- { // [!code ++] - body: 'memo' // [!code ++] - } // [!code ++] + { // [!code ++] + body: 'memo' // [!code ++] + } // [!code ++]
1488-1489: Body schema alias usage — looks goodSwitching to the named 'memo' model keeps runtime/type-level in sync; nice touch.
1686-1695: OpenTelemetry example — callouts and order look correctPlugin order (opentelemetry before swagger) and EOL callouts are consistent.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/tutorial.md (1)
1451-1454: Replace hard tabs with spacesThe code blocks at these lines contain hard tabs, tripping markdown-lint rule MD010. Switch to spaces for consistency with the rest of the doc and to satisfy lint.
Also applies to: 1480-1483
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/tutorial.md(24 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/tutorial.md
1453-1453: Hard tabs
Column: 7
(MD010, no-hard-tabs)
1480-1480: Hard tabs
Column: 10
(MD010, no-hard-tabs)
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/tutorial.md (1)
569-575: Fix route path after adding prefix: should drop '/note'With
prefix: '/note', the added [!code ++] line should beget('/', ...), notget('/note', ...). The current diff inversion will confuse readers.Apply this diff:
- .get('/note', ({ note }) => note.data) // [!code ++] + .get('/', ({ note }) => note.data) // [!code ++]Also applies to: 580-582
🧹 Nitpick comments (4)
docs/tutorial.md (4)
525-544: Prefer explicit bounds check overindex in note.dataUsing the
inoperator on arrays is surprising and can yield unintended truthiness. For clarity and correctness, check numeric bounds instead.- ({ note, params: { index }, status }) => { // [!code ++] - if (index in note.data) return note.remove(index) // [!code ++] + ({ note, params: { index }, status }) => { // [!code ++] + if (Number.isInteger(index) && index >= 0 && index < note.data.length) + return note.remove(index) // [!code ++] // [!code ++] return status(422) // [!code ++] }, // [!code ++]Apply the same pattern to the PATCH handler below in this block.
592-599: Repeat the safer index bounds checkThe same
index in note.datapattern appears here. Recommend the same explicit bounds check for DELETE and PATCH examples to keep the tutorial consistent.- ({ note, params: { index }, status }) => { - if (index in note.data) return note.remove(index) + ({ note, params: { index }, status }) => { + if (Number.isInteger(index) && index >= 0 && index < note.data.length) + return note.remove(index) return status(422) },Also applies to: 606-613
1352-1356: Replace hard tabs with spaces (markdownlint MD010)Tabs at these lines trigger MD010 and can misalign diff color blocks. Use spaces.
- { // [!code ++] - body: 'memo' // [!code ++] - } // [!code ++] + { // [!code ++] + body: 'memo' // [!code ++] + } // [!code ++]
1379-1382: Replace hard tabs with spaces (markdownlint MD010)Another tab is present here; convert to spaces to satisfy linting and keep formatting consistent.
- return note.update(index, { data, author: username }) // [!code ++] + return note.update(index, { data, author: username }) // [!code ++]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/tutorial.md(11 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/tutorial.md
1354-1354: Hard tabs
Column: 7
(MD010, no-hard-tabs)
1381-1381: Hard tabs
Column: 10
(MD010, no-hard-tabs)
🔇 Additional comments (5)
docs/tutorial.md (5)
491-504: CRUD example additions look goodThe Note methods and corresponding CRUD routes/read models are coherent and demonstrate Elysia patterns well.
Also applies to: 509-553
294-317: Validation example correctly reintroducestand applies schemaImporting
tand validatingparamsis accurate and aligns with the explanation.
446-469: Plugin extraction and application LGTMGood move to extract the note routes into a plugin and apply with
.use(note), removing duplicated handlers inindex.ts.
1445-1455: Macro-based auth usage is correctUsing
guard({ auth: true })and passing{ auth: true, body: 'memo' }at route-level cleanly demonstrates macro application.Also applies to: 1472-1484
1584-1602: Import cleanup and OpenTelemetry integration LGTMRemoving the unused
timport fromindex.ts, addingopentelemetry, and ordering plugins before routes follows best practices.
Follows up #595
Summary by CodeRabbit