-
Notifications
You must be signed in to change notification settings - Fork 381
docs: update right colored diffs #596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis update introduces inline annotation comments and minor formatting tweaks within a documentation tutorial file. The changes focus exclusively on improving code readability and annotation consistency without modifying any logic, structure, or behavior of the documented code examples. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.update
return.- 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
t
isn’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
decorate
is accurate and consistent with later usage.
253-255
: Path param snippet is fine (TS warning addressed later)Defines
:index
cleanly; 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.Object
is introduced correctly
params: t.Object({ index: t.Number() })
and importingt
are correct and align with later guard usage.
345-347
: 404 viastatus
helper 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
note
plugin 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
,update
on 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
params
validation and removes redundancy in per-route schemas. Annotations accurately show removals.Also applies to: 632-637, 647-652, 662-666
690-695
: LifecycleonTransform
is placed correctlyHook is added before routes; logs are minimal and illustrative.
749-818
: Auth:user
module scaffolding looks good
state
stores 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/verify
work as-is for the example versions used in docs.
859-880
: Reference models are introduced well
signIn
,session
, andoptionalSession
models 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
user
onceuserService
is used.Also applies to: 1109-1142
1189-1217
: MacroisSignIn
is correctly structuredReturning
beforeHandle
via macro is a neat pattern; annotations clearly mark new lines.
1228-1250
: Macro usage reads wellUsing
isSignIn: true
lets you remove imperative checks from the handler. Good example.
1268-1278
: Resolve pattern is demonstrated clearlyDefining
username
via.resolve
after 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
memo
and usingt.Omit
for 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
Follows up #595
Summary by CodeRabbit