feat(tegg-loader): route module discovery through shared loader fs#5970
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThe PR replaces direct ChangesLoaderFS Injection Through the Tegg Loader Stack
Sequence Diagram(s)sequenceDiagram
participant Test
participant EggApp
participant EggModuleLoader
participant LoaderFactory
participant ManifestLoaderFS
participant CountingFallbackLoaderFS
Note over Test,CountingFallbackLoaderFS: Phase 1 — Local boot to collect manifest
Test->>EggApp: boot(local mode)
EggApp-->>Test: manifest data captured
Note over Test,CountingFallbackLoaderFS: Phase 2 — Bundle mode boot from manifest
Test->>EggApp: boot(ManifestStore.fromBundle + CountingFallbackLoaderFS)
EggApp->>EggModuleLoader: buildAppGraph(loaderFS=ManifestLoaderFS)
EggModuleLoader->>LoaderFactory: loadApp(..., loaderFS=ManifestLoaderFS)
LoaderFactory->>ManifestLoaderFS: glob(pattern, cwd)
ManifestLoaderFS-->>LoaderFactory: manifest-backed file list (no real-fs)
EggModuleLoader->>LoaderFactory: createLoader(module, loaderFS=ManifestLoaderFS)
LoaderFactory->>ManifestLoaderFS: glob(pattern, cwd=modules/)
ManifestLoaderFS->>CountingFallbackLoaderFS: fallback glob inside modules/
CountingFallbackLoaderFS-->>ManifestLoaderFS: real file list
ManifestLoaderFS-->>LoaderFactory: file list
Test->>EggApp: POST /app
EggApp-->>Test: { success: true, traceId }
Test->>EggApp: GET /app
EggApp-->>Test: { app: { name, ... }, traceId }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Deploying egg with
|
| Latest commit: |
39b261c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f66faf87.egg-cci.pages.dev |
| Branch Preview URL: | https://feat-tegg-loader-shared-load.egg-cci.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5970 +/- ##
==========================================
+ Coverage 85.30% 85.53% +0.22%
==========================================
Files 670 669 -1
Lines 19552 19828 +276
Branches 3863 3919 +56
==========================================
+ Hits 16678 16959 +281
+ Misses 2481 2478 -3
+ Partials 393 391 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request integrates a file system abstraction (LoaderFS) into the loading mechanism of tegg to support bundle-mode application booting and cross-module dependency injection without relying on direct disk discovery. It replaces globby with loaderFS.glob in ModuleLoader and propagates loaderFS through LoaderFactory and EggModuleLoader. Additionally, comprehensive integration and unit tests are added to verify behavior in bundled environments. The review feedback suggests two improvements in the test files: using .ts extensions instead of .js for TypeScript imports in BundledDI.test.ts to maintain consistency, and avoiding broad substring checks with path separators in BundledAppBoot.test.ts by matching against exact path segments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
killagu
force-pushed
the
feat/tegg-loader-shared-loader-fs
branch
from
3b11a1a to
ed1e41d
Compare
Deploying egg-v3 with
|
| Latest commit: |
39b261c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f107aedb.egg-v3.pages.dev |
| Branch Preview URL: | https://feat-tegg-loader-shared-load.egg-v3.pages.dev |
|
Dependency limit exceeded — report not shown. This pull request scan exceeded the 10,000-dependency limit applied to this scan, so the results are incomplete and may be inaccurate. To avoid reporting false positives, Socket has not posted a report. Upgrade your plan to raise the dependency limit and get complete reports, or view the partial scan in the dashboard. Socket is always free for open source. If this is a non-commercial open source project, contact us to request a free Team account. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tegg/plugin/tegg/test/BundledAppBoot.test.ts (1)
1-1: ⚡ Quick winRename this test file to lowercase-hyphen format.
BundledAppBoot.test.tsdoes not follow the repo filename convention; please rename it (for example,bundled-app-boot.test.ts).As per coding guidelines,
**/*: Keep file names lowercase with hyphens.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tegg/plugin/tegg/test/BundledAppBoot.test.ts` at line 1, Rename the test file from BundledAppBoot.test.ts to bundled-app-boot.test.ts to follow the repository's naming convention of using lowercase letters with hyphens for file names. This applies to the entire file name and path.Source: Coding guidelines
tegg/core/loader/test/ModuleLoaderLoaderFS.test.ts (1)
1-1: ⚡ Quick winRename this test file to lowercase-hyphen format.
ModuleLoaderLoaderFS.test.tsviolates the repository naming convention; please rename it (for example,module-loader-loader-fs.test.ts) to keep consistency across tooling and package conventions.As per coding guidelines,
**/*: Keep file names lowercase with hyphens.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tegg/core/loader/test/ModuleLoaderLoaderFS.test.ts` at line 1, Rename the test file ModuleLoaderLoaderFS.test.ts to use the lowercase-hyphen naming convention. Convert the PascalCase filename to module-loader-loader-fs.test.ts to align with the repository's file naming guidelines that require lowercase file names with hyphens as separators. Update any import paths or references to this file throughout the codebase to reflect the new filename.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tegg/core/loader/test/ModuleLoaderLoaderFS.test.ts`:
- Line 1: Rename the test file ModuleLoaderLoaderFS.test.ts to use the
lowercase-hyphen naming convention. Convert the PascalCase filename to
module-loader-loader-fs.test.ts to align with the repository's file naming
guidelines that require lowercase file names with hyphens as separators. Update
any import paths or references to this file throughout the codebase to reflect
the new filename.
In `@tegg/plugin/tegg/test/BundledAppBoot.test.ts`:
- Line 1: Rename the test file from BundledAppBoot.test.ts to
bundled-app-boot.test.ts to follow the repository's naming convention of using
lowercase letters with hyphens for file names. This applies to the entire file
name and path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ae84d7ce-019d-4954-a742-3a17d5589fd4
📒 Files selected for processing (10)
tegg/core/loader/package.jsontegg/core/loader/src/LoaderFactory.tstegg/core/loader/src/impl/ModuleLoader.tstegg/core/loader/test/LoaderFactoryManifest.test.tstegg/core/loader/test/ModuleLoaderLoaderFS.test.tstegg/core/runtime/package.jsontegg/core/runtime/test/BundledDI.test.tstegg/plugin/tegg/package.jsontegg/plugin/tegg/src/lib/EggModuleLoader.tstegg/plugin/tegg/test/BundledAppBoot.test.ts
killagu
force-pushed
the
feat/tegg-loader-shared-loader-fs
branch
from
ed1e41d to
3e5cc84
Compare
Wire @eggjs/tegg-loader's module discovery through the shared @eggjs/loader-fs abstraction instead of importing globby directly, so a bundled app's manifest-backed VFS (ManifestLoaderFS) serves tegg discovery too. Normal startup keeps using RealLoaderFS, so behavior is unchanged. - ModuleLoader: discovery uses an injectable LoaderFS.glob (default RealLoaderFS); module loading (LoaderUtil.loadFile + bundle hook) is left untouched. - LoaderFactory: thread an optional loaderFS through createLoader/loadApp. - EggModuleLoader: reuse app.loader.loaderFS in buildAppGraph and loadModule. - drop the direct globby dependency from @eggjs/tegg-loader. Tests: - ModuleLoaderLoaderFS + LoaderFactoryManifest: discovery routes through the injected LoaderFS, precompute skips glob, default RealLoaderFS. - BundledDI (runtime): cross-module DI resolves via manifest decoratedFiles with a glob-throwing LoaderFS, proving zero disk scan. - BundledAppBoot (plugin): full app boots in manifest-consume mode and serves the controller -> service -> cross-module repo DI chain over HTTP; with an injected ManifestLoaderFS, core discovery does zero disk globs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
killagu
force-pushed
the
feat/tegg-loader-shared-loader-fs
branch
from
3e5cc84 to
39b261c
Compare
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Motivation
Part of the bundle startup optimization (Theme F, EGG-130/136/138).
@eggjs/tegg-loaderimportedglobbydirectly for module discovery, so a bundled app's manifest-backed VFS (ManifestLoaderFS) could not serve tegg's discovery — only egg-core's. This routes tegg discovery through the shared@eggjs/loader-fsabstraction, satisfying "no private VFS inside tegg".Scope
ModuleLoader: discovery now uses an injectableLoaderFS.glob(defaultRealLoaderFS, so normal startup is byte-for-byte unchanged). Module loading (LoaderUtil.loadFile+ the__EGG_BUNDLE_MODULE_LOADER__hook + dynamic-import fallback) is intentionally left untouched —RealLoaderFS.loadFileonly returns the default export, which doesn't fit tegg's all-named-exports semantics.LoaderFactory: thread an optionalloaderFSthroughcreateLoader/loadApp(both the manifestdecoratedFilesbranch and the globby branch).EggModuleLoader: reuseapp.loader.loaderFSinbuildAppGraphandloadModule. Normal mode →RealLoaderFS(zero regression); bundle mode →ManifestLoaderFS.globbydependency from@eggjs/tegg-loader.Tests
ModuleLoaderLoaderFS.test.ts(new) +LoaderFactoryManifest.test.ts(extended): discovery routes through the injectedLoaderFS; precomputed files skip glob; defaultRealLoaderFSequivalence.BundledDI.test.ts(runtime, new): cross-module DI (service → public repo → private persistence) resolves via manifestdecoratedFileswith a glob-throwingLoaderFS, proving zero disk scan.BundledAppBoot.test.ts(plugin, new): a full tegg app boots in manifest-consume mode (productionfromBundle→setBundleStorepath) and serves the controller → service → cross-module repo DI chain over HTTP; with an injectedManifestLoaderFS, core/framework discovery does zero disk globs.Test evidence
vitest run tegg/core/loader tegg/core/runtime tegg/plugin/tegg→ 30 passed | 2 skipped (94 tests), 0 failures. typecheck (loader/runtime/plugin), oxlint--type-aware, oxfmt, and--frozen-lockfileall clean.Known follow-ups (out of scope here)
@eggjs/mockhas no first-classloaderFSoption, soBundledAppBootinjects it via the single-mode options passthrough with a cast. TODO: addloaderFStoMockOptionsand drop the cast.EggModuleLoader.loadModulestill globs each module dir from disk (manifest stores teggdecoratedFiles, not module dirs in corefileDiscovery). Pre-existing; a candidate for further bundle-startup optimization.🤖 Generated with Claude Code
Summary by CodeRabbit