fix: mock property support symbol #62
Conversation
WalkthroughThe pull request introduces modifications to a mocking library, focusing on enhancing type flexibility and removing the dedicated TypeScript declaration file. The changes primarily involve updating function signatures to support Changes
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (
|
commit: |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #62 +/- ##
=======================================
Coverage 95.52% 95.52%
=======================================
Files 1 1
Lines 648 648
Branches 123 123
=======================================
Hits 619 619
Misses 29 29 ☔ 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 (1)
src/index.ts (1)
248-248: Line 248 not covered by tests.
No existing test calls 'spy' on a non-function property, leaving this line untested. Consider adding a test to improve coverage.Below is a diff snippet that adds a new test block to cover this scenario:
+describe('spy non-function property coverage', () => { + it('should throw error when property is not a function', () => { + const obj = { prop: 123 }; + (function() { + mm.spy(obj, 'prop'); + }).should.throw(Error, { message: 'spy target prop is not a function' }); + }); +});🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 248-248: src/index.ts#L248
Added line #L248 was not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
package.json(1 hunks)src/index.d.ts(0 hunks)src/index.ts(14 hunks)test/mm.test.ts(2 hunks)
💤 Files with no reviewable changes (1)
- src/index.d.ts
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/index.ts
[warning] 248-248: src/index.ts#L248
Added line #L248 was not covered by tests
🔇 Additional comments (21)
test/mm.test.ts (4)
766-782: Tests for environment variable mocking are robust.
The revised tests successfully validate that environment variables can be mocked and restored. This ensures consistency in environment-dependent behaviors.
784-790: Symbol property mocking validated successfully.
The test case effectively ensures that symbol property mocking functions as expected, reflecting the newly added support for PropertyKey.
792-796: Number property mocking validated successfully.
This test scenario confirms that numeric property mocking is supported, aligned with the expanded PropertyKey usage.
847-847: Comprehensive coverage for complex property mocking.
This test case demonstrates mocking functions that themselves contain additional properties, extending coverage to more intricate mocking scenarios. Implementation details seem correct.
package.json (1)
22-22: Dependency version upgraded to ^1.1.0.
The updated version may include critical fixes or improvements. Confirm there are no breaking changes that could affect existing functionality.
✅ Verification successful
Version upgrade from ^1.0.0 to ^1.1.0 is safe to proceed
The verification shows:
- Node.js version requirement (>= 18.19.0) matches in both package.json and the dependency
- The package is used for basic mocking functionality (muk, isMocked, restore) which are core features
- The minor version bump (1.0.0 to 1.1.0) follows semver, indicating only backward-compatible changes
- The dependency has minimal usage in the codebase (only in src/index.ts)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify if there are known issues with @cnpmjs/muk-prop@^1.1.0
npm view @cnpmjs/muk-prop@^1.1.0 dependencies
npm view @cnpmjs/muk-prop@^1.1.0 engines
Length of output: 117
Script:
#!/bin/bash
# Check current Node.js version requirement in package.json
rg '"node":\s*"[^"]*"' package.json
# Find usage of muk-prop in the codebase
rg -l "@cnpmjs/muk-prop"
# Check the previous version of muk-prop in package.json from git history
git show HEAD~1:package.json | rg "muk-prop"
Length of output: 192
Script:
#!/bin/bash
# Check how muk-prop is used in the codebase
rg -A 5 "@cnpmjs/muk-prop" src/index.ts
Length of output: 235
src/index.ts (16)
13-17: Enhanced type safety for 'mock()'.
Switching the property parameter to PropertyKey allows both string and symbol-based property mocking. This seamlessly accommodates the new test cases in “mm.test.ts”.
Line range hint 18-41: Refined 'spyFunction' for robust function tracking.
Changing the function signature to accept PropertyKey broadens the spy to symbol-based method names. The proxy usage is well-structured.
35-35: Proper error handling for mocking async functions.
Throwing an error for mismatched async usage is prudent, preventing unexpected behavior.
42-42: Broadening 'isAsyncLikeFunction' to PropertyKey.
This supports a consistent approach to symbol-based property checks. Implementation remains straightforward.
Line range hint 93-128: '_mockError' expansion to handle symbol-based methods.
Allowing symbols for module method names aligns with the broader shift to PropertyKey. Timeout logic and callback invocation appear correct.
Line range hint 143-147: 'mockError' updated to accept string | symbol.
This change continues the consistent pattern across function signatures. Implementation has no apparent gaps.
Line range hint 157-161: 'errorOnce' alignment with symbol-based property keys.
The new parameter type is consistent. No functional regressions noted.
Line range hint 171-207: 'mockDatas' for multi-value returns, improved type coverage.
Broadening “method” to PropertyKey is consistent. The logic for asynchronous generations remains intact.
Line range hint 211-218: 'mockData' reusing 'mockDatas' internally is well-structured.
This reduces duplication and keeps the code DRY. Good approach.
Line range hint 220-229: 'dataWithAsyncDispose' supporting symbol-based mocking.
Implementation is concise and consistent with the rest of the code changes.
Line range hint 237-244: 'mockEmpty' to return null values with symbol-based method.
Retains the original intention for mocking empty results, with minimal overhead.
Line range hint 246-256: 'spy' can now handle symbols while verifying target is a function.
Implementation ensures function references are strictly validated.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 248-248: src/index.ts#L248
Added line #L248 was not covered by tests
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 23)
[warning] 243-243:
Expected @param names to be "mod, method". Got "mod,, method,"
[warning] 243-243:
Expected @param names to be "mod, method". Got "mod,, method,"
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 22)
[warning] 243-243:
Expected @param names to be "mod, method". Got "mod,, method,"
[warning] 243-243:
Expected @param names to be "mod, method". Got "mod,, method,"
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 20)
[warning] 243-243:
Expected @param names to be "mod, method". Got "mod,, method,"
[warning] 243-243:
Expected @param names to be "mod, method". Got "mod,, method,"
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 18.19.0)
[warning] 243-243:
Expected @param names to be "mod, method". Got "mod,, method,"
[warning] 243-243:
Expected @param names to be "mod, method". Got "mod,, method,"
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 18)
[warning] 243-243:
Expected @param names to be "mod, method". Got "mod,, method,"
[warning] 243-243:
Expected @param names to be "mod, method". Got "mod,, method,"
🪛 GitHub Check: Node.js / Test (macos-latest, 23)
[warning] 243-243:
Expected @param names to be "mod, method". Got "mod,, method,"
[warning] 243-243:
Expected @param names to be "mod, method". Got "mod,, method,"
🪛 GitHub Check: Node.js / Test (macos-latest, 22)
[warning] 243-243:
Expected @param names to be "mod, method". Got "mod,, method,"
[warning] 243-243:
Expected @param names to be "mod, method". Got "mod,, method,"
🪛 GitHub Check: Node.js / Test (macos-latest, 18.19.0)
[warning] 243-243:
Expected @param names to be "mod, method". Got "mod,, method,"
[warning] 243-243:
Expected @param names to be "mod, method". Got "mod,, method,"
Line range hint 265-274: 'syncError' also supports symbol-based methods.
No issues found—this approach parallels existing logic.
Line range hint 279-288: 'syncData' includes symbol property coverage.
Implementation continues the same pattern of broadening property keys.
Line range hint 291-299: 'syncEmpty' consistently extends 'syncData'.
No functional discrepancies noted.
🧰 Tools
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 23)
[warning] 288-288:
Expected @param names to be "mod, method". Got "mod,, method,"
[warning] 288-288:
Expected @param names to be "mod, method". Got "mod,, method,"
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 22)
[warning] 288-288:
Expected @param names to be "mod, method". Got "mod,, method,"
[warning] 288-288:
Expected @param names to be "mod, method". Got "mod,, method,"
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 20)
[warning] 288-288:
Expected @param names to be "mod, method". Got "mod,, method,"
[warning] 288-288:
Expected @param names to be "mod, method". Got "mod,, method,"
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 18.19.0)
[warning] 288-288:
Expected @param names to be "mod, method". Got "mod,, method,"
[warning] 288-288:
Expected @param names to be "mod, method". Got "mod,, method,"
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 18)
[warning] 288-288:
Expected @param names to be "mod, method". Got "mod,, method,"
[warning] 288-288:
Expected @param names to be "mod, method". Got "mod,, method,"
🪛 GitHub Check: Node.js / Test (macos-latest, 23)
[warning] 288-288:
Expected @param names to be "mod, method". Got "mod,, method,"
[warning] 288-288:
Expected @param names to be "mod, method". Got "mod,, method,"
🪛 GitHub Check: Node.js / Test (macos-latest, 22)
[warning] 288-288:
Expected @param names to be "mod, method". Got "mod,, method,"
[warning] 288-288:
Expected @param names to be "mod, method". Got "mod,, method,"
🪛 GitHub Check: Node.js / Test (macos-latest, 18.19.0)
[warning] 288-288:
Expected @param names to be "mod, method". Got "mod,, method,"
[warning] 288-288:
Expected @param names to be "mod, method". Got "mod,, method,"
584-584: 'classMethod' supporting class prototypes with PropertyKey.
Implementation is straightforward, ensuring consistent mocking for symbol-based or numeric properties in class prototypes.
[skip ci] ## [4.0.1](v4.0.0...v4.0.1) (2024-12-21) ### Bug Fixes * mock property support symbol ([#62](#62)) ([a7950f3](a7950f3))
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor