fix: remove ContextDelegation #183
Conversation
WalkthroughThe pull request introduces a comprehensive update 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 (
|
|
Removed dependencies detected. Learn more about Socket for GitHub ↗︎ 🚮 Removed packages: npm/tsd@0.31.2, npm/tshy-after@1.4.0, npm/tshy@3.0.2, npm/typescript@5.7.2 |
commit: |
![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 (1)
src/app/middleware/cluster_app_mock.ts (1)
12-14: Avoid castingctx.requesttoany.
Casting toanybypasses type safety. Consider using a typed request body or a narrower type assertion if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
package.json(1 hunks)src/app/extend/application.ts(8 hunks)src/app/middleware/cluster_app_mock.ts(1 hunks)test/fixtures/tegg-app-esm/test/hooks.test.ts(2 hunks)test/fixtures/tegg-app-esm/test/tegg_context.test.ts(1 hunks)test/fixtures/tegg-app/test/hooks.test.ts(2 hunks)test/fixtures/tegg-app/test/tegg_context.test.ts(1 hunks)test/index.test-d.ts(1 hunks)
🔇 Additional comments (30)
src/app/extend/application.ts (11)
9-10: Use consistent import naming.Replacing
LoggerwithEggLoggerandContextDelegationwithContextis consistent with the new implementation. Confirm that all other imports referencingContextDelegationare removed or updated throughout the codebase.
42-42: Interface rename is clear and aligns with Egg’s standard context.Changing
MockContextDelegationtoMockContextand extendingContextensures better type checking and clarity. Keep an eye out for any references to the old interface name.
50-52: Logger type alignment.These changes ensure that the application’s
loggerandcoreLoggerproperties match Egg’s official logger type. This should help avoid confusion and leverage the proper Egg logger APIs.
76-76: Transition fromMockContextDelegationtoMockContextin return type.Returning
MockContexthere aligns with the new extended interface and enhances type consistency.
105-105: Reusing existingcurrentContext.Ensuring the returned context is properly cast to
MockContext. This is acceptable as long as the underlyingcurrentContexttruly conforms toMockContext.
112-112: Directly returning the new context.Casting the newly created context to
MockContextis consistent. Just verify all relevant properties are initialized properly (e.g.,service).
115-115: Method signature update to accept and returnMockContext.Using
MockContextclarifies what context methods or properties can be used within the scope function. The async logic here remains clear.
407-407: Logger parameter usage.The update to
EggLoggerclarifies usage and ensures the parameter can accept a logger instance conforming to Egg’s standard. No further changes needed.
427-427: Synchronized logger checks withEggLogger.Ensures
_mockLogslogic works properly with the updated logger type. This method’s approach to reading logs from memory or file remains valid.
463-463:expectLogmethod aligned withEggLogger.Maintains consistency with the rest of the logger changes in the file. Good job.
473-473:notExpectLogmethod aligned withEggLogger.Ensures consistency with new logging approach. No issues noted.
test/fixtures/tegg-app/test/tegg_context.test.ts (2)
2-2: Context import alignment.This change from
ContextDelegationtoContextis in line with the broader context refactor. Confirm no references to the old type remain.
8-8: Context variable declaration.Declaring
ctxwith the newContexttype ensures type safety. Looks good.test/fixtures/tegg-app-esm/test/tegg_context.test.ts (2)
2-2: Update to import fromegg.Renaming to
Contextis correct and consistent with Egg framework updates.
8-8: Variable type updated.Switching
ctxto theContexttype aligns with the rest of the PR’s context changes.test/fixtures/tegg-app-esm/test/hooks.test.ts (5)
2-2: ImportContextfromegg.Good replacement of the old
ContextDelegationtype. Ensures that the test suite references the correct context type.
9-11: Variables updated toRecord<string, Context>.Switching the type from a custom delegation to the standard Egg
Contextclarifies usage. Keep an eye on any helper methods that might expect the old type.
29-37: Assigningapp.currentContextto typed objects.Casting
app.currentContextasContextensures type safety in tests. No issues found.
43-51: Same pattern forbarsuite.Behavior is identical to the
foosuite, which is consistent and good.
56-63: Array ofContextobjects in multi-it tests.Appending multiple
Contextreferences ensures each test run is self-contained. This approach is valid.test/fixtures/tegg-app/test/hooks.test.ts (5)
2-2: ImportContextfromegg.Updating the import to use
Contextmatches the removal ofContextDelegationfrom the codebase.
9-11: Initialization of context dictionaries withContext.Switches to
Contextconfirm the new type is fully adopted.
29-37: Ensuring typed references to currentContext.Consistently casting to
Contextensures no untyped usage.
43-51:barsuite updated for type consistency.Matches the
foosuite approach.
56-63: Storing multiple contexts in an array.Confirming the array is typed as
Array<Context>. Consistency is maintained here as well.test/index.test-d.ts (2)
2-2: Consider removing unused references toContextDelegation.
Great job replacingContextDelegationwithContext. If there are any remaining references toContextDelegationin other files or test cases, consider removing them for consistency.
8-9: Verify correctness of type assertions.
UsingContext | undefinedensures stricter type safety. Just confirm you have test coverage to validate these scenarios (whenapp.currentContextorapp.ctxStorage.getStore()might beundefined).src/app/middleware/cluster_app_mock.ts (2)
2-2: Context type alignment.
UpdatingContextDelegationtoContextis consistent with the new Egg.js definitions. Good job making this change.
7-7: Preserve functionality after type replacement.
Be sure that noContextDelegation-specific properties or methods are being relied on. Confirm that any references to older context properties are updated accordingly.package.json (1)
32-32: Verify compatibility with@eggjs/core@^6.2.11.
Ensure that other dependencies and your code are fully compatible with the updated@eggjs/coreversion.
[skip ci] ## [6.0.5](v6.0.4...v6.0.5) (2025-01-02) ### Bug Fixes * remove ContextDelegation ([#183](#183)) ([f4051d0](f4051d0))
Summary by CodeRabbit
Dependencies
@eggjs/coredependency from version 6.2.5 to 6.2.11Refactor
ContextDelegationwithContextacross multiple filesLoggertoEggLoggerTests