-
Notifications
You must be signed in to change notification settings - Fork 42
docs: add CLAUDE.md for Claude Code guidance #392
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
Conversation
Comprehensive documentation for Claude Code including: **For Application Developers:** - Common development commands (test, lint, build, CI) - Architecture overview and key packages - Import guidelines (@eggjs/tegg, subpaths, internal packages) - Best practices (SingletonProto for performance, AccessLevel scope, lifecycle hooks, BackgroundTaskHelper, dependency injection) - Important constraints (SingletonProto can inject ContextProto via Proxy + AsyncLocalStorage mechanism) **For Framework Contributors:** - Development workflow (lint, ci, tsc:pub before commit) - Package development order (types → decorator → runtime → plugin) - Creating decorator packages and plugins - Wrapping third-party SDKs (core wrapper → Egg subclass → factory) - Plugin package.json config (eggPlugin + eggModule for standalone) - Available lifecycle hooks and testing with egg-mock - Core package internals (core-decorator, metadata, runtime, types, plugin) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds CLAUDE.md documentation file providing comprehensive guidance for the tegg project (Egg.js integration). Covers architecture, monorepo structure, packages, core concepts, dependency injection flow, lifecycle hooks, import guidelines, best practices, code conventions, development workflows, testing, and plugin patterns. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Summary of ChangesHello @killagu, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new CLAUDE.md file, providing extensive documentation tailored for AI assistants to navigate and contribute to the tegg framework. The document covers the framework's architecture, core concepts, development best practices, and detailed guidelines for framework contributions, aiming to enhance AI-assisted understanding and interaction with the codebase. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces a comprehensive guide, CLAUDE.md, intended for the Claude AI to understand and work with the tegg repository. The documentation is exceptionally detailed and well-structured, covering project overview, architecture, best practices, and a deep dive into the framework's internals for contributors. My review focuses on enhancing the clarity, consistency, and completeness of this new documentation file. I've suggested a few minor improvements, such as adding a missing key package to the overview, ensuring type consistency in examples, clarifying a custom command, and fixing minor formatting issues. Overall, this is an excellent addition that will be highly valuable.
| | Package | Description | | ||
| |---------|-------------| | ||
| | `@eggjs/tegg` | Main entry point, re-exports all decorator packages | | ||
| | `@eggjs/tegg-plugin` | Main Egg.js plugin for tegg integration | | ||
| | `@eggjs/core-decorator` | Core decorators: `@ContextProto`, `@SingletonProto`, `@Inject` | | ||
| | `@eggjs/tegg-types` | TypeScript type definitions | | ||
| | `@eggjs/tegg-metadata` | Metadata storage and graph building | | ||
| | `@eggjs/tegg-runtime` | Runtime container and object instantiation | | ||
| | `@eggjs/tegg-lifecycle` | Object lifecycle hooks | |
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.
The @eggjs/tegg-loader package is mentioned in the 'Dependency Injection Flow' section as being responsible for scanning and loading modules, which is a critical function. However, it's missing from this 'Key Packages' list. For completeness and to give a full overview of the core components, please consider adding it to this table.
| @ContextProto() | ||
| export class BadService { | ||
| @Inject() | ||
| ctx: Context; // too broad |
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.
In this 'Avoid' example, ctx is typed as Context. However, in a previous example for RequestTracer on line 175, it is typed as EggContext. To maintain consistency throughout the documentation and avoid potential confusion, consider using EggContext here as well.
| ctx: Context; // too broad | |
| ctx: EggContext; // too broad |
| npm run tsc:pub | ||
| ``` | ||
|
|
||
| **CI Pipeline** (`npm run ci`) executes: |
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.
|
|
||
| export function MyDecorator(params?: MyDecoratorParams) { | ||
| return function(target: EggProtoImplClass) { | ||
| PrototypeUtil.setMetaData(target, MY_DECORATOR_META, params || {}); |
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.
There appears to be a typo in the function name setMetaData. The common camelCase convention is setMetadata. Correcting this will improve the accuracy of the documentation, which is especially important for a guide intended for an AI.
| PrototypeUtil.setMetaData(target, MY_DECORATOR_META, params || {}); | |
| PrototypeUtil.setMetadata(target, MY_DECORATOR_META, params || {}); |
| ├── ModuleHandler.ts # Orchestrates module loading and initialization | ||
| ├── EggModuleLoader.ts # Loads modules using tegg-loader | ||
| ├── EggContextHandler.ts # Manages EggContext lifecycle | ||
| ├── EggContextImpl.ts # EggContext implementation for Egg | ||
| ├── EggCompatibleObject.ts # Wraps egg app/ctx objects as prototypes | ||
| └── EggCompatibleProtoImpl.ts # Makes egg objects injectable |
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.
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: 2
🤖 Fix all issues with AI agents
In @CLAUDE.md:
- Around line 322-328: Several Markdown fenced blocks showing directory
structures (for example the block containing "core/types → Type definitions
(interfaces, enums)" and the other blocks listing core/*-decorator,
core/*-runtime, core/tegg, plugin/*, etc.) are missing language identifiers;
update each of those fence openings to include a language (preferably "text") so
the directory-structure/code-fence blocks use ```text instead of ``` to follow
Markdown best practices. Ensure you update every affected block noted in the
review (all directory/structure listing fences) while preserving the exact block
contents.
- Line 522: Replace the bold emphasis line "**Step 3: Create factory for
instantiation**" with a proper Markdown heading (e.g., "### Step 3: Create
factory for instantiation" or the appropriate heading level used elsewhere) so
the section becomes a real heading instead of bold text; update that single line
to the chosen heading syntax to match the document's heading structure.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CLAUDE.md
🧰 Additional context used
🪛 LanguageTool
CLAUDE.md
[grammar] ~524-~524: Ensure spelling is correct
Context: ...ry for instantiation** Factory injects tegg dependencies and creates the client: `...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~564-~564: Ensure spelling is correct
Context: ...ch: - Keeps core wrapper independent of tegg runtime - Allows Egg-specific extension...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~564-~564: Ensure spelling is correct
Context: ...core wrapper independent of tegg runtime - Allows Egg-specific extensions (context ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md
322-322: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
336-336: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
390-390: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
522-522: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
557-557: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
630-630: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
651-651: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
681-681: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
703-703: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
717-717: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Runner-macos (16)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-macos (18)
🔇 Additional comments (4)
CLAUDE.md (4)
1-95: Well-structured introduction and architecture overview — The project overview, commands, and architecture sections are clear and comprehensive. Good use of tables for key packages and concepts.
96-157: Clear and comprehensive import guidelines — The section properly distinguishes import patterns for application developers, advanced users, and framework internals. Code examples are accurate and follow TypeScript conventions.
158-280: Practical best practices and important constraints — The section provides actionable guidance with clear code examples. The constraint about@SingletonProtoinjecting@ContextProtovia Proxy andAsyncLocalStorageis especially valuable for avoiding runtime errors.
1-732: Comprehensive and well-organized framework guidance — This documentation provides excellent guidance for both application developers and framework contributors. The architecture overview, best practices, constraints, and development guide are clearly written with practical examples.The only issues are minor markdown formatting violations that can be fixed quickly without affecting content quality.
| ``` | ||
| 1. core/types → Type definitions (interfaces, enums) | ||
| 2. core/*-decorator → Decorators and metadata utilities | ||
| 3. core/*-runtime → Runtime logic (if needed) | ||
| 4. core/tegg → Re-export from main package (add to index.ts or create subpath) | ||
| 5. plugin/* → Egg.js plugin integration | ||
| ``` |
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.
Add language identifiers to code fence blocks — Multiple code blocks for directory structures lack language specifications. While these are not strictly executable code, they should follow markdown best practices.
📋 Directory structure blocks need language specification
Change fence opening syntax from:
to:
```
or
```
Affected lines:
- 322-328: Package development order diagram
- 336-349: Decorator package structure
- 390-405: Plugin package structure
- 557-561: Third-party SDK wrapping summary
- 630-645: core-decorator internals structure
- 651-669: tegg-metadata internals structure
- 681-697: tegg-runtime internals structure
- 703-711: tegg-types internals structure
- 717-732: tegg-plugin internals structure
</details>
Also applies to: 336-349, 390-405, 557-561, 630-645, 651-669, 681-697, 703-711, 717-732
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
322-322: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @CLAUDE.md around lines 322 - 328, Several Markdown fenced blocks showing
directory structures (for example the block containing "core/types → Type
definitions (interfaces, enums)" and the other blocks listing core/-decorator,
core/-runtime, core/tegg, plugin/*, etc.) are missing language identifiers;
update each of those fence openings to include a language (preferably "text") so
the directory-structure/code-fence blocks use text instead of to follow
Markdown best practices. Ensure you update every affected block noted in the
review (all directory/structure listing fences) while preserving the exact block
contents.
</details>
<!-- fingerprinting:phantom:triton:mongoose -->
<!-- This is an auto-generated comment by CodeRabbit -->
| } | ||
| ``` | ||
|
|
||
| **Step 3: Create factory for instantiation** |
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.
Convert emphasis to proper heading — Line 522 uses bold text (**...**) as a section heading. Convert to a proper markdown heading for consistency with document structure.
♻️ Proposed fix
-**Step 3: Create factory for instantiation**
+### Step 3: Create factory for instantiation📝 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.
| **Step 3: Create factory for instantiation** | |
| ### Step 3: Create factory for instantiation |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
522-522: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🤖 Prompt for AI Agents
In @CLAUDE.md at line 522, Replace the bold emphasis line "**Step 3: Create
factory for instantiation**" with a proper Markdown heading (e.g., "### Step 3:
Create factory for instantiation" or the appropriate heading level used
elsewhere) so the section becomes a real heading instead of bold text; update
that single line to the chosen heading syntax to match the document's heading
structure.
Comprehensive documentation for Claude Code including:
For Application Developers:
For Framework Contributors:
Checklist
npm testpassesAffected core subsystem(s)
Description of change
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.