fix: export watcher type on eggjs/core namespace #17
Conversation
WalkthroughThe pull request introduces comprehensive updates to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
commit: |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #17 +/- ##
==========================================
+ Coverage 87.05% 93.37% +6.31%
==========================================
Files 9 13 +4
Lines 85 362 +277
Branches 0 45 +45
==========================================
+ Hits 74 338 +264
- Misses 11 24 +13 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
test/index.test.ts (1)
6-11: Excellent coverage of exported entities.
This test effectively checks all exported members inwatcher. Consider adding additional tests to verify the runtime behavior of these exports, ensuring they function as expected.src/config/config.default.ts (1)
17-17: Consider avoiding inline type assertions if possible.Casting the object with
as WatcherConfigworks, but you could refactor to define aconst config: WatcherConfig = { ... }for clarity.src/lib/types.ts (1)
26-34: Module augmentation is well-applied.Augmenting the
@eggjs/coremodule to includewatcherin bothEggCoreandEggAppConfigensures consistent typing across your application. Consider adding a short comment describing the purpose.test/watcher.test.ts (1)
30-30: Potential property mismatch in the test callback.The code references
info.foo, but yourChangeInfointerface no longer explicitly requiresfoo. Ensure these tests align with the flexible design ofChangeInfo.Also applies to: 61-61
src/lib/watcher.ts (2)
15-15: Replacing #config type from WatcherConfig to EggAppConfig.Broadening the type to
EggAppConfigcan simplify usage but may risk type safety ifwatcher-specific fields are missing. Consider asserting the presence of required fields.
27-27: Double type assertion for EventSource.Casting with
unknown as typeof BaseEventSourcecan mask potential type inaccuracies or runtime errors. Revisit usage or confirm that dynamic imports can’t break.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
package.json(3 hunks)src/config/config.default.ts(2 hunks)src/config/config.local.ts(1 hunks)src/config/config.unittest.ts(1 hunks)src/lib/boot.ts(1 hunks)src/lib/event-sources/index.ts(1 hunks)src/lib/types.ts(2 hunks)src/lib/watcher.ts(2 hunks)test/development.test.ts(1 hunks)test/development_cluster.test.ts(1 hunks)test/index.test.ts(1 hunks)test/watcher.test.ts(3 hunks)
🔇 Additional comments (23)
src/config/config.unittest.ts (2)
1-2: Good import usage.
Importing theWatcherConfigtype improves clarity and ensures compile-time checks for proper configuration structure.
6-6: Proper type assertion.
Usingas WatcherConfigon thewatcherobject aligns well with the new type definitions; this is a concise and effective approach to enforce typing.src/config/config.local.ts (2)
1-2: Consistent import across configs.
Mirroring the import ofWatcherConfigin multiple configuration files preserves consistency and clarity.
6-6: Correct type enforcement.
Asserting the default object asWatcherConfigprovides type safety early and clearly communicates which properties are expected.src/lib/event-sources/index.ts (2)
1-3: Explicit imports vs wildcard exports.
Listing each import explicitly improves clarity, making it clear which exports are available and preventing accidental collision or unexpected exports.
5-9: Named exports for clarity.
Using named exports clarifies the available modules and fosters better readability and discoverability.src/config/config.default.ts (1)
3-3: Good addition of the typed import for watcher configuration.The explicit import of
WatcherConfigclarifies the expected shape of the configuration, reinforcing type safety.src/lib/types.ts (2)
5-14: Documentation approach is consistent and thorough.The clarified JSDoc comments for
WatcherConfigeffectively convey how event sources are structured, enhancing maintainability.
17-17: Flexible extension of ChangeInfo is beneficial.Allowing arbitrary fields via
extends Record<string, any>helps handle additional metadata. Verify if you need to enforce more structure to avoid unintentional usage.src/lib/boot.ts (2)
1-1: Confirm that the new type import matches project conventions.Switching from
EggWatcherApplicationCoretoEggApplicationCoremight introduce subtle differences if the original type had custom fields. Ensure this type truly meets your needs.
5-5: Migration away fromEggWatcherApplicationCorelooks good.The constructor and class field now reference
EggApplicationCore. Confirm that custom watcher-specific fields are still properly addressed; otherwise, excellent job consolidating the type usage.Also applies to: 8-8
test/watcher.test.ts (1)
3-3: Switching to @eggjs/mock is consistent with the new namespace.Ensure that
egg-mockis fully removed from dependencies and references to prevent confusion.test/development_cluster.test.ts (1)
4-4: Switched import path to new @eggjs/mock library.This update from
egg-mockto@eggjs/mockappears consistent with the broader transition in the PR. It aligns the test suite with the new library namespace.test/development.test.ts (1)
4-4: Updated import path for mock library.Similar to other test files, this switch ensures the code relies on the modern
@eggjs/mocklibrary instead ofegg-mock. Looks good.src/lib/watcher.ts (3)
5-5: New import from '@eggjs/core'.Using
EggAppConfigindicates deeper integration with Egg's config system. Verify that this import doesn't introduce version conflicts with other dependencies.
8-8: Removed WatcherConfig reference.It’s okay to remove
WatcherConfigin favor of a more unifiedEggAppConfig, but confirm that any watch-specific config fields remain accessible.
18-18: Constructor now accepts EggAppConfig.This aligns with the new approach. Ensure that external calls that instantiate
Watcherare passing the correct config shape.package.json (6)
12-13: Added "typescript" export entry.This addition makes TypeScript source available for tooling. Looks useful for local development, as well as type definitions.
18-18: Extended pretest script to run cleanup first.Automated cleanup helps prevent stale artifacts from affecting test results.
20-20: Adjusted 'preci' workflow.Running
npm run clean && npm run lintensures a consistent environment before CI tasks.
22-24: Post-ci and publishing scripts revised.Replacing or restructuring these steps can impact CI and publishing flows. Confirm that
attw --packandnpm run cleanare still valid in your environment.
41-42: Added '@eggjs/core' dependency.This matches the code changes where
EggAppConfigis imported. Ensure version ^6.2.13 is compatible with other Eggjs packages.
52-54: Added '@eggjs/bin', '@eggjs/mock', and 'rimraf' dependencies.
@eggjs/mockreplacesegg-mockto align with the new library.rimrafsimplifies cleaning artifacts.
Everything looks consistent with the changes.
[skip ci] ## [4.0.3](v4.0.2...v4.0.3) (2025-01-04) ### Bug Fixes * export watcher type on eggjs/core namespace ([#17](#17)) ([12e7fbd](12e7fbd))
Summary by CodeRabbit
Release Notes
Dependencies
@eggjs/utils,@eggjs/core, and replacingegg-binandegg-mockwith@eggjs/binand@eggjs/mockConfiguration
Testing
Chores