docs: fix tyops, improve grammar in best-practice.md #615
Conversation
WalkthroughThis PR makes editorial updates to docs/essential/best-practice.md, refining grammar, punctuation, hyphenation, and phrasing for clarity and consistency. It standardizes terminology (e.g., request-dependent), clarifies sections on MVC, services, decorators, models, plugin deduplication, and method chaining. No functional or API changes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
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. ✨ Finishing Touches🧪 Generate unit tests
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
docs/essential/best-practice.md (9)
201-210: Broken example: method name mismatch.
.get('/', Controller.hi)references a non‑existent method; should beController.root.-new Elysia() - .get('/', Controller.hi) +new Elysia() + .get('/', Controller.root)
292-318: Macro/handler mismatch in example.
macro.isSignInreturns{ session }, but the route destructures{ Auth: { user } }. Align handler with macro output.-const UserController = new Elysia() - .use(AuthService) - .get('/profile', ({ Auth: { user } }) => user, { - isSignIn: true - }) +const UserController = new Elysia() + .use(AuthService) + .get('/profile', ({ session }) => session, { + isSignIn: true + })
92-109: Auth example: query/select mismatch and inverted credential check.
user.idis used but not selected.verify(...)success should not throw.- const user = await sql` - SELECT password + const user = await sql` + SELECT id, password FROM users WHERE username = ${username} LIMIT 1` @@ - if (await Bun.password.verify(password, user.password)) - // You can throw an HTTP error directly - throw status( - 400, - 'Invalid username or password' satisfies AuthModel.signInInvalid - ) + if (!(await Bun.password.verify(password, user.password))) + // You can throw an HTTP error directly + throw status( + 400, + 'Invalid username or password' satisfies AuthModel.signInInvalid + )
353-356: Sign‑in guard logic inverted.
Should return 401 when there is no session.- if (session.value) - return status(401) + if (!session.value) + return status(401)
388-391: Grammar fixes.-Model or [DTO (Data Transfer Object)](https://en.wikipedia.org/wiki/Data_transfer_object) is handle by [Elysia.t (Validation)](/essential/validation.html#elysia-type). - -Elysia has a validation system built-in which can infers type from your code and validate it at runtime. +Model or [DTO (Data Transfer Object)](https://en.wikipedia.org/wiki/Data_transfer_object) is handled by [Elysia.t (Validation)](/essential/validation.html#elysia-type). + +Elysia has a built‑in validation system that can infer types from your code and validate them at runtime.
490-498: Invalid property access.
AuthModelis a plain object here;AuthModel.modelsdoesn’t exist.export const AuthModel = { sign: t.Object({ username: t.String(), password: t.String() }) } - -const models = AuthModel.models
501-503: Grammar + possessive.-Though this is optional, if you are strictly following MVC pattern, you may want to inject like a service into a controller. We recommended using [Elysia reference model](/essential/validation#reference-model) +Though optional, if you are strictly following the MVC pattern, you may want to inject a model like a service into a controller. We recommend using [Elysia’s reference model](/essential/validation#reference-model).
530-535: Subject–verb agreement and clarity.-This approach provide several benefits: +This approach provides several benefits: 1. Allow us to name a model and provide auto-completion. 2. Modify schema for later usage, or perform a [remap](/essential/handler.html#remap). 3. Show up as "models" in OpenAPI compliance client, eg. OpenAPI. 4. Improve TypeScript inference speed as model type will be cached during registration.
538-542: Grammar tweaks in plugin reuse.-It's ok to reuse plugins multiple time to provide type inference. +It’s okay to reuse plugins multiple times to provide type inference. @@ -Elysia handle plugin deduplication automatically by default, and the performance is negligible. +Elysia handles plugin deduplication automatically by default, and the performance impact is negligible.
🧹 Nitpick comments (5)
docs/essential/best-practice.md (5)
21-24: Tighten intro phrasing for clarity.
Minor grammar/style improvements; avoids “we found it hard” voice and awkward “adapt … with”.-However, there are several concerns when trying to adapt an MVC pattern [(Model-View-Controller)](https://en.wikipedia.org/wiki/Model%E2%80%93view%E2%80%93controller) with Elysia, and we found it hard to decouple and handle types. -This page is a guide on how to follow Elysia structure best practices combined with the MVC pattern, but it can be adapted to any coding pattern you prefer. +However, there are several concerns when applying the MVC pattern [(Model–View–Controller)](https://en.wikipedia.org/wiki/Model%E2%80%93view%E2%80%93controller) in Elysia; decoupling components and managing types can be difficult. +This page guides you through Elysia best‑practice structure with MVC, but you can adapt the guidance to any coding pattern you prefer.
29-50: Polish flow and parallelism.
Small readability nits; preserve meaning.-However, **if you don't have a specific structure in mind**, we recommend a feature-based folder structure where each feature has its own folder containing controllers, services, and models. +However, **if you don't have a specific structure in mind**, we recommend a feature‑based structure where each feature has its own folder containing controllers, services, and models. @@ -This structure allows you to easily find and manage your code and keep related code together. +This structure makes it easy to find and manage your code, keeping related code together.
184-184: Grammar: “recommend to” → “recommend …ing”.-We recommend to <u>**always use method chaining**</u> to provide accurate type inference. +We recommend <u>**always using method chaining**</u> to provide accurate type inference.
321-321: Optional: clarify “singleton” phrasing.
“It will be a singleton” → clarify that Elysia reuses the same plugin by name.-Elysia handles [plugin deduplication](/essential/plugin.html#plugin-deduplication) by default, so you don't have to worry about performance, as it will be a singleton if you specify a **"name"** property. +Elysia handles [plugin deduplication](/essential/plugin.html#plugin-deduplication) by default; if you specify a plugin **name**, Elysia reuses the same instance.
49-49: Optional: reduce duplication.
Combine the two verbs with a gerund for smoother reading.-This structure allows you to easily find and manage your code and keep related code together. +This structure lets you easily find and manage your code, keeping related code together.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/essential/best-practice.md(7 hunks)
🔇 Additional comments (4)
docs/essential/best-practice.md (4)
147-147: LGTM.
Encouraging flexibility reads well.
152-152: LGTM.
Accurately describes the type‑driven API.
321-322: LGTM.
Accurate note on plugin deduplication and named singleton behavior.
384-385: Broken self‑link anchor.
Link text says “Elysia as a service” but anchor points to the controller section. Suggest linking to the “Request‑dependent service as Elysia instance” section or to “Service”.-However we recommend to avoid this if possible, and use [Elysia as a service](#✅-do-use-elysia-as-a-controller) instead. +However, we recommend avoiding this when possible and using [Service](#service) instead.Would you like me to verify the correct slug generated by your docs tool and update to the exact anchor?
| There are 2 types of service in Elysia: | ||
| 1. Non-request dependent service | ||
| 2. Request dependent service | ||
|
|
||
| ### ✅ Do: Abstract away non-request dependent service | ||
|
|
||
| We recommended to abstract a service class/function away from Elysia. | ||
| We recommend abstracting a service class/function away from Elysia. | ||
|
|
||
| If the service or function doesn't tied to HTTP request or access a `Context`. It's recommended to abstract it as a static class or function. | ||
| If the service or function isn't tied to an HTTP request or doesn't access a `Context`, it's recommended to implement it as a static class or function. | ||
|
|
There was a problem hiding this comment.
Hyphenation/plurality for consistency.
Use “request‑dependent” and pluralize “services”.
-There are 2 types of service in Elysia:
-1. Non-request dependent service
-2. Request dependent service
+There are 2 types of services in Elysia:
+1. Non‑request‑dependent services
+2. Request‑dependent services
@@
-We recommend abstracting a service class/function away from Elysia.
+We recommend abstracting a service class/function away from Elysia.
@@
-If the service or function isn't tied to an HTTP request or doesn't access a `Context`, it's recommended to implement it as a static class or function.
+If the service or function isn’t tied to an HTTP request or doesn’t access a `Context`, implement it as a static class or function.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| There are 2 types of service in Elysia: | |
| 1. Non-request dependent service | |
| 2. Request dependent service | |
| ### ✅ Do: Abstract away non-request dependent service | |
| We recommended to abstract a service class/function away from Elysia. | |
| We recommend abstracting a service class/function away from Elysia. | |
| If the service or function doesn't tied to HTTP request or access a `Context`. It's recommended to abstract it as a static class or function. | |
| If the service or function isn't tied to an HTTP request or doesn't access a `Context`, it's recommended to implement it as a static class or function. | |
| There are 2 types of services in Elysia: | |
| 1. Non-request-dependent services | |
| 2. Request-dependent services | |
| ### ✅ Do: Abstract away non-request dependent service | |
| We recommend abstracting a service class/function away from Elysia. | |
| If the service or function isn’t tied to an HTTP request or doesn’t access a `Context`, implement it as a static class or function. |
🤖 Prompt for AI Agents
In docs/essential/best-practice.md around lines 260 to 269, the wording should
use consistent hyphenation and plurality: change “request dependent service” and
“service” instances to “request-dependent services” (and “Non-request dependent
service” to “Non-request-dependent services”) throughout this block; update
headings and the explanatory paragraph to use the hyphenated adjective and
plural form so the terminology is consistent across the section.
| ### ✅ Do: Decorate only request dependent property | ||
|
|
||
| It's recommended to `decorate` only request dependent property, such as `requestIP`, `requestTime`, or `session`. | ||
| It's recommended to `decorate` only request-dependent properties, such as `requestIP`, `requestTime`, or `session`. | ||
|
|
||
| Overusing decorator may tied your code to Elysia making it harder to test, and reuse. | ||
| Overusing decorators may tie your code to Elysia, making it harder to test and reuse. | ||
|
|
There was a problem hiding this comment.
Hyphenation + plurality + scope caution.
Heading and body should use “request‑dependent” and plural “properties”.
-### ✅ Do: Decorate only request dependent property
+### ✅ Do: Decorate only request‑dependent properties
@@
-It's recommended to `decorate` only request-dependent properties, such as `requestIP`, `requestTime`, or `session`.
+Decorate only request‑dependent properties, such as `requestIP`, `requestTime`, or `session`.
@@
-Overusing decorators may tie your code to Elysia, making it harder to test and reuse.
+Overusing decorators can tie your code to Elysia, making it harder to test and reuse.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### ✅ Do: Decorate only request dependent property | |
| It's recommended to `decorate` only request dependent property, such as `requestIP`, `requestTime`, or `session`. | |
| It's recommended to `decorate` only request-dependent properties, such as `requestIP`, `requestTime`, or `session`. | |
| Overusing decorator may tied your code to Elysia making it harder to test, and reuse. | |
| Overusing decorators may tie your code to Elysia, making it harder to test and reuse. | |
| ### ✅ Do: Decorate only request-dependent properties | |
| Decorate only request-dependent properties, such as `requestIP`, `requestTime`, or `session`. | |
| Overusing decorators can tie your code to Elysia, making it harder to test and reuse. |
🤖 Prompt for AI Agents
In docs/essential/best-practice.md around lines 324 to 329, update the heading
and body copy to use the hyphenated adjective "request-dependent" and the plural
"properties" (e.g., "Decorate only request-dependent properties") and ensure the
body text similarly uses "request-dependent properties" to keep terminology
consistent and accurate about scope; make the minimal text edits to replace
non-hyphenated and singular forms with these corrected terms.
2 participants
Summary by CodeRabbit