[TYPING] Added types for session #3025
Conversation
There was a problem hiding this comment.
PR Overview
This pull request introduces improved TypeScript typings for the session management module to clarify the expected types and function signatures.
- Introduces a new SessionData type and refines the Session type
- Updates function signatures for the get and set methods with enhanced type annotations
- Adjusts callback handling in the set method
Reviewed Changes
| File | Description |
|---|---|
| packages/common-db/src/lib/common/session.ts | Added type definitions and updated function signatures to improve type safety for session handling |
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Co-authored-by: Copilot <[email protected]>
GermanBluefox
added
the
type definition update ⌚
|
|
||
| type Session = { | ||
| Store: any; | ||
| }; | ||
|
|
||
| // TODO: in the long term move this file somewhere where we have types access it is nowhere used in controller itself and just exported for adapters so it should go to js-controller-adapter package | ||
| interface AdapterStoreOptions { | ||
| /** The ioBroker adapter */ | ||
| adapter: any; |
There was a problem hiding this comment.
why Any and not Adapter type?
There was a problem hiding this comment.
IDK how to solve it
There was a problem hiding this comment.
The types are not build when DB is build so can not be used. So maybe move AdapterStore out of Common-DB package when it is more Adapter specific?
| * @returns the constructor to create a new AdapterStore | ||
| */ | ||
| export function createAdapterStore(session: Session, defaultTtl = 3600): any { | ||
| const Store = session.Store; | ||
|
|
||
| class AdapterStore extends Store { | ||
| private readonly adapter: any; |
| if (typeof ttl === 'object') { | ||
| fn = sess; | ||
| fn = sess as (err?: Error | null) => void; |
There was a problem hiding this comment.
if think better check if typef sess === 'function' before hardly defining that type here whoich might be wrong
| sess && sess.cookie && sess.cookie.originalMaxAge | ||
| ? Math.round(sess.cookie.originalMaxAge / 1000) | ||
| : defaultTtl; | ||
| ttl = sess?.cookie?.originalMaxAge ? Math.round(sess.cookie.originalMaxAge / 1000) : defaultTtl; |
There was a problem hiding this comment.
is ttl === 0 allowed? could be result of this
There was a problem hiding this comment.
If ttl===0 it will be set to default
| } | ||
| ttl = ttl || defaultTtl; | ||
| this.adapter.setSession(sid, ttl, sess, function () { | ||
| this.adapter.setSession(sid, ttl, sess, function (err?: Error | null): void { |
There was a problem hiding this comment.
maybe use lambda function here?
There was a problem hiding this comment.
No. Because we do not want to give the 'this' context in callback
Better typing for session to understand how it works