refactor: types from 'any' to 'unknown' #3062
Conversation
Replace 'any' types with 'unknown' for better type safety: - Changed StateTree from Record<PropertyKey, any> to Record<PropertyKey, unknown> - Changed isPlainObject parameter from 'o: any' to 'o: unknown' - Removed eslint-disable comment as it's no longer needed
Replace 'any' types with 'unknown' for better type safety: - Changed isUseStore parameter from 'fn: any' to 'fn: unknown' - Changed patchObject parameters from Record<string, any> to Record<string, unknown> - newState parameter - oldState parameter - return type
✅ Deploy Preview for pinia-official canceled.
|
|
Warning Rate limit exceeded@kazbekaskarov has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 51 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughUpdated TypeScript typings: several Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant HMR as Vite HMR runtime
participant Pinia as pinia.acceptHMRUpdate
participant Store as StoreDefinition
rect rgb(245,250,255)
Dev->>Pinia: import acceptHMRUpdate(initialUseStore, hot)
Note right of Pinia: Type annotations updated to `unknown`\n(runtime unchanged)
end
HMR->>Pinia: hot.accept(newModule)
Pinia->>Store: validate module (isUseStore)
alt module is a store
Pinia->>Store: patch state (patchObject)
Store-->>Pinia: updated
else not a store
Pinia-->>HMR: ignore
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/pinia/src/hmr.ts (1)
87-122: TypeScript error: iterating overunknownwithout narrowing.The
newModuleparameter cannot be used in afor...inloop (line 99) or indexed (line 100) without first narrowing fromunknown. This will cause TypeScript compilation errors.Apply a similar fix as suggested for the
hotparameter. Consider:-return (newModule: unknown) => { +return (newModule: any) => {Or add proper type narrowing:
return (newModule: unknown) => { if (!newModule || typeof newModule !== 'object') return const pinia: Pinia | undefined = /* ... */ // ... rest with type assertions where needed }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/pinia/src/hmr.ts(3 hunks)packages/pinia/src/types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/pinia/src/types.ts (1)
packages/pinia/src/index.ts (1)
StateTree(17-17)
packages/pinia/src/hmr.ts (2)
packages/pinia/src/types.ts (1)
StoreDefinition(493-518)rollup.config.mjs (1)
__DEV__(166-171)
🔇 Additional comments (3)
packages/pinia/src/types.ts (2)
14-14: LGTM: Improved type safety withunknown.Changing
StateTreefromRecord<PropertyKey, any>toRecord<PropertyKey, unknown>properly enforces type narrowing before access, aligning with TypeScript best practices.
16-28: LGTM: Type guard correctly acceptsunknown.The parameter change from
anytounknownis appropriate for type guard functions. The implementation safely performs runtime checks before property access.packages/pinia/src/hmr.ts (1)
32-61: LGTM: Appropriate use ofRecord<string, unknown>.The signature change correctly uses
Record<string, unknown>instead ofany. The implementation safely accesses properties and uses type guards before recursive calls.
| export const isUseStore = (fn: unknown): fn is StoreDefinition | ||
| return typeof fn === 'function' && typeof fn.$id === 'string' | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify TypeScript compilation with property access on unknown.
While changing the parameter to unknown improves type safety, accessing fn.$id after only checking typeof fn === 'function' may cause a TypeScript error. TypeScript doesn't infer that functions can have custom properties without additional narrowing.
Run the following to verify compilation:
🏁 Script executed:
#!/bin/bash
# Check if the project builds successfully with these type changes
cd packages/pinia
npm run build 2>&1 | grep -A 5 -B 5 "error TS"Length of output: 0
🏁 Script executed:
#!/bin/bash
# First, verify the file exists and show its content
echo "=== Checking hmr.ts content ==="
find . -name "hmr.ts" -type f | head -5
echo ""
echo "=== Full content of hmr.ts ==="
cat packages/pinia/src/hmr.ts || echo "File not found at expected path"
echo ""
echo "=== Checking tsconfig.json ==="
cat packages/pinia/tsconfig.json 2>/dev/null || echo "tsconfig.json not found"
echo ""
echo "=== Checking package.json build script ==="
grep -A 2 '"build"' packages/pinia/package.json || echo "Build script not found"Length of output: 3884
🏁 Script executed:
#!/bin/bash
# First, verify the file exists and show its content
echo "=== Checking hmr.ts content ==="
find . -name "hmr.ts" -type f | head -5
echo ""
echo "=== Full content of hmr.ts ==="
cat packages/pinia/src/hmr.ts || echo "File not found at expected path"
echo ""
echo "=== Checking tsconfig.json ==="
cat packages/pinia/tsconfig.json 2>/dev/null || echo "tsconfig.json not found"
echo ""
echo "=== Checking package.json build script ==="
grep -A 2 '"build"' packages/pinia/package.json || echo "Build script not found"Length of output: 3884
Fix syntax error: missing opening brace in isUseStore function signature.
The function at line 19 is missing the opening brace { after the type predicate. The line break splits the signature from the body:
export const isUseStore = (fn: unknown): fn is StoreDefinition {
return typeof fn === 'function' && typeof fn.$id === 'string'
}Additionally, accessing fn.$id on a narrowed Function type creates a type safety issue—functions don't have a $id property by default. Consider either casting to a more specific type or using optional chaining if runtime safety is intended.
🧰 Tools
🪛 Biome (2.1.2)
[error] 20-20: Line terminator not permitted before arrow.
(parse)
[error] 20-21: Expected a statement but instead found '}'.
Expected a statement here.
(parse)
🤖 Prompt for AI Agents
In packages/pinia/src/hmr.ts around lines 19 to 21, the isUseStore function is
missing the opening brace after its type predicate and is unsafely accessing
fn.$id on a plain Function type; add the missing "{" to start the function body
and change the property access to a safe form (for example cast fn to any or a
narrower interface or use optional chaining) so the runtime check becomes
something like typeof (fn as any).$id === 'string' (or typeof (fn as { $id?:
unknown }).$id === 'string') to preserve type safety while fixing the syntax
error.
| >(initialUseStore: StoreDefinition<Id, S, G, A>, hot: unknown) { | ||
| // strip as much as possible from iife.prod | ||
| if (!__DEV__) { | ||
| return () => {} | ||
| } |
There was a problem hiding this comment.
TypeScript error: accessing properties on unknown without narrowing.
Changing hot to unknown without adding type narrowing will cause TypeScript compilation errors at lines 88, 96, and 111 where hot.data and hot.invalidate() are accessed without first narrowing the type.
Consider these options:
Option 1: Keep any for the hot parameter (least disruptive):
->(initialUseStore: StoreDefinition<Id, S, G, A>, hot: unknown) {
+>(initialUseStore: StoreDefinition<Id, S, G, A>, hot: any) {Option 2: Define a proper type for the HMR API:
interface HotModuleAPI {
data: Record<string, any>
invalidate(): void
}
export function acceptHMRUpdate<...>(
initialUseStore: StoreDefinition<Id, S, G, A>,
hot: HotModuleAPI
) {Option 3: Add runtime narrowing (if you want to keep unknown):
>(initialUseStore: StoreDefinition<Id, S, G, A>, hot: unknown) {
if (!hot || typeof hot !== 'object') return () => {}
if (!('data' in hot) || !('invalidate' in hot)) return () => {}
// ... rest of implementation with type assertions🤖 Prompt for AI Agents
In packages/pinia/src/hmr.ts around lines 82 to 86, the function parameter hot
was changed to unknown but the code later (lines ~88, 96, 111) accesses hot.data
and hot.invalidate() without narrowing; update the code to use a proper HMR API
type instead of unknown: declare a HotModuleAPI interface (data: Record<string,
any>; invalidate(): void) and change the function signature to accept hot:
HotModuleAPI, then ensure any remaining runtime checks/assertions use that type;
alternatively, if you prefer minimal change, revert hot to any to avoid
TypeScript errors.
1 participant
Refactor types from 'any' to 'unknown'
Summary by CodeRabbit