-
Notifications
You must be signed in to change notification settings - Fork 455
docs: show multiple service patterns with tabs #749
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
WalkthroughUpdates documentation to present multiple patterns for non-request-dependent services (Plain Object, Module Exports/Namespace, Abstract Class), refactors the Auth example from a static abstract-class export to an object-literal export, and adds tip admonitions in key concept docs. Includes tabbed UI examples in best-practice docs. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🔇 Additional comments (3)
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 |
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/essential/best-practice.md(4 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/essential/best-practice.md
94-94: Hard tabs
Column: 1
(MD010, no-hard-tabs)
289-289: Hard tabs
Column: 1
(MD010, no-hard-tabs)
290-290: Hard tabs
Column: 1
(MD010, no-hard-tabs)
291-291: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🔇 Additional comments (6)
docs/essential/best-practice.md (6)
92-113: Service pattern change from abstract class to plain object.The Auth service is now a plain object instead of an abstract class with static methods. This is documented clearly with an inline comment referencing the Service section. The change maintains API compatibility (
Auth.signIn()works for both patterns) and introduces beginners to a simpler, more idiomatic JavaScript approach.
288-366: Multi-pattern Service section with Tab component adds value.The new tabbed interface effectively demonstrates three equivalent service organization patterns (Plain Object, Namespace, Abstract Class). Each pattern is:
- Syntactically correct TypeScript
- Clearly explained with its use case
- Functionally equivalent at runtime (as stated on line 368)
The pedagogical value is high—developers can choose based on familiarity rather than being prescribed a single pattern. The plain object example (default) is most approachable for newcomers.
298-314: Plain Object pattern example is clear and idiomatic.The service function definition uses shorthand method syntax with TypeScript type annotations, which is valid and readable. The recursive call to
Service.fibo()demonstrates proper self-referencing within the object literal.
318-340: Namespace pattern correctly demonstrates type co-location.The TypeScript namespace example correctly shows the ability to export both functions and types with the same name, which is a valid TypeScript feature and a legitimate advantage of the namespace pattern over plain objects for type-heavy code.
342-364: Abstract class pattern with static methods is properly motivated.The comment on line 344 appropriately frames this for developers coming from Java/C# backgrounds. The use of
abstract classwith only static methods prevents accidental instantiation, which is correct and idiomatic for that pattern.
17-19: Verify Tab component exists at../components/fern/tab.vue.Ensure the component file is present at the relative path from
docs/essential/best-practice.md. Additionally, fix hard tab characters on lines 94 and 289-291 by replacing them with space indentation to comply with markdown linting standards (MD010).
| static async signIn({ username, password }: AuthModel.signInBody) { | ||
| // Group related functions - see Service section for alternative patterns | ||
| export const Auth = { | ||
| async signIn({ username, password }: AuthModel.signInBody) { |
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.
Fix hard tab indentation to use spaces.
The static analysis tool flagged hard tabs (Tab character) on lines 94, 289-291. Markdown linting standards require spaces for indentation. Replace tabs with spaces to maintain consistency with markdown best practices.
Apply the following diff to fix hard tabs on line 94 (Auth object literal):
~// Group related functions - see Service section for alternative patterns
~export const Auth = {
~ async signIn({ username, password }: AuthModel.signInBody) {
+// Group related functions - see Service section for alternative patterns
+export const Auth = {
+ async signIn({ username, password }: AuthModel.signInBody) {Apply the following diff to fix hard tabs on lines 289-291 (Tab component props):
~<Tab
~ id="service-pattern"
~ :names="['Plain Object', 'Namespace', 'Abstract Class']"
~ :tabs="['object', 'namespace', 'class']"
~>
+<Tab
+ id="service-pattern"
+ :names="['Plain Object', 'Namespace', 'Abstract Class']"
+ :tabs="['object', 'namespace', 'class']"
+>As per static analysis tools, use consistent indentation throughout the file.
Also applies to: 289-291
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
94-94: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🤖 Prompt for AI Agents
In docs/essential/best-practice.md around lines 94 and 289-291, there are hard
tab characters used for indentation; replace the tab(s) on line 94 (inside the
Auth object literal) and the tabs on lines 289-291 (Tab component props) with
spaces to match the repository's markdown indentation style (e.g., 2 spaces per
indent), ensure no other tabs remain in the file, and re-run markdown linting to
confirm the issue is resolved.
Present all four approaches for grouping service functions (plain object, module exports, namespace, abstract class) using tabs, letting developers choose based on their background and preferences. Add tip callouts in other locations pointing to this central reference. Changes: - Add Tab component to best-practice.md with 4 pattern options - Add "Module Exports" pattern (import * as) for ES module-native approach - Reorder patterns: Plain Object → Module Exports → Namespace → Abstract Class - Add tip callout after Controller "Do" example linking to Service patterns - Add tip callout in key-concept.md linking to Service patterns - Update main service.ts example to use plain object (simplest approach)
0cb95a3 to
d78eb51
Compare
The Best Practice page currently shows
abstract classwith static methods for services andnamespacefor models. Rather than prescribing a single pattern, this change presents all four approaches for grouping functions using tabs, letting developers choose based on their background.The Service section now shows four options in order of simplicity: plain objects (simplest, idiomatic JavaScript), module exports with
import * as(ES module-native, tree-shakeable), namespaces (TypeScript feature for co-locating types with values), and abstract classes with static methods (familiar to Java/C# developers). All four produce identical runtime behavior; the choice is purely stylistic.Other locations that show abstract class examples (Controller section, key-concept.md) now include a brief tip callout pointing to the Service patterns section, making it clear that alternatives exist without cluttering the examples. The anti-pattern examples that use abstract class are left unchanged since the grouping pattern isn't the point there.
This aligns with Elysia's stated philosophy of being "pattern-agnostic" and leaving coding pattern decisions to the developer and their team.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.