refactor(core): replace any types with unknown in useStorageState#360
refactor(core): replace any types with unknown in useStorageState#360
Conversation
There was a problem hiding this comment.
Pull request overview
This PR continues the repo’s strict TypeScript goal of eliminating any by updating useStorageState’s internal type utilities and aligning related tests/docs.
Changes:
- Replaced
anywithunknowninSerializableGuard,isPlainObject, andensureSerializabletypings inuseStorageState. - Updated the
useStorageStatecustom serializer/deserializer test to usestringparameters. - Fixed
useMapdocumentation code fences and added a missing import in the example.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/hooks/useStorageState/useStorageState.ts |
Replaces any with unknown in internal helper typings for serializability checks. |
src/hooks/useStorageState/useStorageState.spec.ts |
Updates test serializer/deserializer parameter types to string. |
src/hooks/useMap/useMap.md |
Cleans up markdown code fencing and improves the example snippet. |
Comments suppressed due to low confidence (1)
src/hooks/useStorageState/useStorageState.ts:28
SerializableGuard’s conditional type is effectively a no-op:T[0] extends unknownis always true (includingnever), so the'Received a non-serializable value'branch is unreachable. If the goal is to surface a helpful type-level message whenSerializable<T>collapses tonever, rewrite the conditional to check forneverfirst (or remove the guard entirely if it’s not needed).
type SerializableGuard<T extends readonly unknown[]> = T[0] extends unknown
? T
: T[0] extends never
? 'Received a non-serializable value'
: T;
| const serializer = (value: string) => | ||
| ['string', 'number', 'boolean'].includes(typeof value) ? value : JSON.stringify(value); | ||
| const deserializer = (value: any) => | ||
| const deserializer = (value: string) => | ||
| /^(\d+)|(true|false)|([^[].*)|([^{].*)$/.test(value) ? value : JSON.parse(value); |
There was a problem hiding this comment.
In this test, serializer/deserializer are typed as (value: string) => ..., but their implementations still branch on non-string primitives and call JSON.parse, which returns any and can produce non-string values. This makes parts of the logic unreachable by type and weakens the intent of aligning types; either simplify the functions to be truly string-only, or broaden the types (and the test inputs) to exercise non-string serialization/deserialization without reintroducing any via JSON.parse.
Replace `any` with stricter `unknown` types in `SerializableGuard`, `isPlainObject`, `ensureSerializable`, and test serializer/deserializer to comply with the no-any coding standard.
fba4064 to
f2ee87e
Compare
|
Thank you very much for your contribution. However, since the value parameter is not limited to a string type, narrowing it down to string does not seem appropriate. Additionally, this hook has been deprecated and is no longer planned to be maintained, so we will be closing this issue. That said, we truly appreciate your contribution and effort. |
Summary
anywithunknowninSerializableGuard,isPlainObject, andensureSerializabletype definitionsanywithstringin test serializer/deserializer parameters to match the hook's option typesanycoding standardTest plan
yarn run test:type)