feat: mock property type support symbol and number #3
Conversation
WalkthroughThe changes introduce enhanced flexibility in key handling for a mocking utility by expanding the key type from Changes
Sequence DiagramsequenceDiagram
participant User
participant MockUtility
participant TargetObject
User->>MockUtility: mock(object, key, value)
MockUtility->>TargetObject: Set/Replace property
MockUtility-->>User: Mocked property
User->>MockUtility: isMocked(object, key)
MockUtility-->>User: Boolean (mocked status)
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 ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 81 81
Branches 11 10 -1
=========================================
Hits 81 81 ☔ 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)
test/method.test.ts (1)
140-143: Consider adding Symbol enumeration testWhile the test correctly verifies number property enumeration, consider adding assertions to verify that Symbol properties are not enumerated by Object.keys (which is the expected behavior).
Add this assertion after line 143:
assert.deepEqual(Object.keys(config), [ '1', 'enableCache', 'delay' ]); +// Verify Symbol properties are not enumerated +assert.deepEqual(Object.getOwnPropertySymbols(config), [fooSymbol]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/index.ts(3 hunks)test/method.test.ts(3 hunks)
🔇 Additional comments (6)
src/index.ts (3)
16-16: LGTM! Function signature update aligns with interface change
The mock function's key parameter type has been correctly updated to support all property key types.
78-78: LGTM! Function signature update is consistent
The isMocked function's key parameter type has been correctly updated to match the mock function signature.
4-4: LGTM! Note: Breaking change in MockItem interface
The change from string to PropertyKey correctly enables support for symbol and number property types. However, this is a breaking change that may affect consumers of this interface.
Let's check for any direct implementations of this interface:
✅ Verification successful
LGTM! Breaking change is properly contained within the module
The change from string to PropertyKey in the MockItem interface is safe to proceed with. After verification:
- The interface is only used internally within the same file
- The only usage is in the
mocksarray which is a private variable - All implementations (object literals) are created within the
mockfunction itself - The change aligns with the function signature which already accepts
PropertyKey
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any direct implementations of MockItem interface
rg "implements MockItem"
rg "class \w+\s*:\s*MockItem"
Length of output: 58
Script:
#!/bin/bash
# Let's try a more comprehensive search for MockItem usage
# First, let's find the file containing MockItem definition
fd "MockItem" --type f
# Then search for any usage of MockItem in type declarations or class extensions
rg ":\s*MockItem" -A 2
rg "extends\s+MockItem" -A 2
rg "implements\s+MockItem" -A 2
# Also search for type assertions or casts to MockItem
rg "as\s+MockItem" -A 2
Length of output: 246
Script:
#!/bin/bash
# Let's examine how MockItem is used in the codebase
# First, let's see the content of the file where it's defined
cat src/index.ts
# Also search for any object literal that matches MockItem structure
rg "{\s*key:" -A 2
Length of output: 1900
test/method.test.ts (3)
75-80: LGTM! Good test setup for new property types
The test setup correctly defines both symbol and number properties for testing the new functionality.
95-102: LGTM! Comprehensive test coverage for Symbol properties
The test case thoroughly verifies mocking, checking mocked state, and restoration of Symbol properties.
104-111: LGTM! Well-structured test for number properties
The test case maintains consistency with the Symbol test pattern and provides good coverage for number properties.
[skip ci] ## [1.1.0](v1.0.0...v1.1.0) (2024-12-21) ### Features * mock property type support symbol and number ([#3](#3)) ([79bf8af](79bf8af))
Summary by CodeRabbit
New Features
Bug Fixes
Documentation