From ba1c741867722e21e11a5be4e5a144f65789cbf3 Mon Sep 17 00:00:00 2001 From: Evert van Brussel Date: Sun, 9 Apr 2023 14:40:51 +0200 Subject: [PATCH 1/5] Fix typings in scenes/context.ts --- src/scenes/context.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/scenes/context.ts b/src/scenes/context.ts index 703aa102..2bc1a3f5 100644 --- a/src/scenes/context.ts +++ b/src/scenes/context.ts @@ -62,11 +62,11 @@ export default class SceneContextScene< return session } - get state() { + get state(): D["state"] { return (this.session.state ??= {}) } - set state(value) { + set state(value: D["state"]) { this.session.state = { ...value } } @@ -82,7 +82,7 @@ export default class SceneContextScene< this.ctx.session.__scenes = Object.assign({}, this.options.defaultSession) } - async enter(sceneId: string, initialState: object = {}, silent = false) { + async enter(sceneId: string, initialState: D["state"] = {}, silent = false) { if (!this.scenes.has(sceneId)) { throw new Error(`Can't find scene: ${sceneId}`) } From 78d5e63803c6d73d2722673cc2badd5c672f36b0 Mon Sep 17 00:00:00 2001 From: Evert van Brussel Date: Tue, 18 Apr 2023 14:49:53 +0200 Subject: [PATCH 2/5] Make the type of state even more accurate Even though TypeScript generates errors on some line, which is my I had to use `// @ts-expect-error` a few times, I can assure you that this is more type-correct. --- src/scenes/context.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/scenes/context.ts b/src/scenes/context.ts index 2bc1a3f5..8684bdfe 100644 --- a/src/scenes/context.ts +++ b/src/scenes/context.ts @@ -14,10 +14,10 @@ export interface SceneContext scene: SceneContextScene, D> } -export interface SceneSessionData { +export interface SceneSessionData { current?: string expires?: number - state?: object + state?: Partial } export interface SceneSession { @@ -62,11 +62,12 @@ export default class SceneContextScene< return session } - get state(): D["state"] { + get state(): Exclude { + // @ts-expect-error {} might not be assignable to Exclude return (this.session.state ??= {}) } - set state(value: D["state"]) { + set state(value: Exclude) { this.session.state = { ...value } } @@ -82,7 +83,8 @@ export default class SceneContextScene< this.ctx.session.__scenes = Object.assign({}, this.options.defaultSession) } - async enter(sceneId: string, initialState: D["state"] = {}, silent = false) { + // @ts-expect-error {} might not be assignable to Exclude + async enter(sceneId: string, initialState: Exclude = {}, silent = false) { if (!this.scenes.has(sceneId)) { throw new Error(`Can't find scene: ${sceneId}`) } From 3a108e601434d403675db380217c0ea3a6983232 Mon Sep 17 00:00:00 2001 From: Evert van Brussel Date: Tue, 25 Apr 2023 13:22:23 +0200 Subject: [PATCH 3/5] I finally fixed all the types around scenes and wizards --- package-lock.json | 35 ++++++++++++++++++++------ package.json | 1 + src/scenes/context.ts | 48 ++++++++++++++++++++++-------------- src/scenes/stage.ts | 2 +- src/scenes/utilTypes.ts | 16 ++++++++++++ src/scenes/wizard/context.ts | 27 +++++++++++--------- src/scenes/wizard/index.ts | 38 +++++++++++++++------------- 7 files changed, 112 insertions(+), 55 deletions(-) create mode 100644 src/scenes/utilTypes.ts diff --git a/package-lock.json b/package-lock.json index b9d84bf5..365ac8b4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -37,6 +37,7 @@ "eslint-plugin-prettier": "^4.2.1", "eslint-plugin-promise": "^6.1.1", "prettier": "^2.8.3", + "type-fest": "^3.8.0", "typedoc": "^0.23.24", "typescript": "^4.9.4" }, @@ -3934,6 +3935,18 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/serialize-error/node_modules/type-fest": { + "version": "0.13.1", + "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-0.13.1.tgz", + "integrity": "sha512-34R7HTnG0XIJcBSn5XhDd7nNFPRcXYRZrBB2O2jdKqYODldSzBAqzsWoZYYvduky73toYS/ESqxPvkDf/F0XMg==", + "dev": true, + "engines": { + "node": ">=10" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/shebang-command": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/shebang-command/-/shebang-command-2.0.0.tgz", @@ -4245,12 +4258,12 @@ } }, "node_modules/type-fest": { - "version": "0.13.1", - "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-0.13.1.tgz", - "integrity": "sha512-34R7HTnG0XIJcBSn5XhDd7nNFPRcXYRZrBB2O2jdKqYODldSzBAqzsWoZYYvduky73toYS/ESqxPvkDf/F0XMg==", + "version": "3.8.0", + "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-3.8.0.tgz", + "integrity": "sha512-FVNSzGQz9Th+/9R6Lvv7WIAkstylfHN2/JYxkyhhmKFYh9At2DST8t6L6Lref9eYO8PXFTfG9Sg1Agg0K3vq3Q==", "dev": true, "engines": { - "node": ">=10" + "node": ">=14.16" }, "funding": { "url": "https://github.com/sponsors/sindresorhus" @@ -7376,6 +7389,14 @@ "dev": true, "requires": { "type-fest": "^0.13.1" + }, + "dependencies": { + "type-fest": { + "version": "0.13.1", + "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-0.13.1.tgz", + "integrity": "sha512-34R7HTnG0XIJcBSn5XhDd7nNFPRcXYRZrBB2O2jdKqYODldSzBAqzsWoZYYvduky73toYS/ESqxPvkDf/F0XMg==", + "dev": true + } } }, "shebang-command": { @@ -7610,9 +7631,9 @@ } }, "type-fest": { - "version": "0.13.1", - "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-0.13.1.tgz", - "integrity": "sha512-34R7HTnG0XIJcBSn5XhDd7nNFPRcXYRZrBB2O2jdKqYODldSzBAqzsWoZYYvduky73toYS/ESqxPvkDf/F0XMg==", + "version": "3.8.0", + "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-3.8.0.tgz", + "integrity": "sha512-FVNSzGQz9Th+/9R6Lvv7WIAkstylfHN2/JYxkyhhmKFYh9At2DST8t6L6Lref9eYO8PXFTfG9Sg1Agg0K3vq3Q==", "dev": true }, "typed-array-length": { diff --git a/package.json b/package.json index 61918773..9bddcdf8 100644 --- a/package.json +++ b/package.json @@ -101,6 +101,7 @@ "eslint-plugin-prettier": "^4.2.1", "eslint-plugin-promise": "^6.1.1", "prettier": "^2.8.3", + "type-fest": "^3.8.0", "typedoc": "^0.23.24", "typescript": "^4.9.4" }, diff --git a/src/scenes/context.ts b/src/scenes/context.ts index 8684bdfe..00f292ed 100644 --- a/src/scenes/context.ts +++ b/src/scenes/context.ts @@ -3,28 +3,38 @@ import Composer from '../composer' import Context from '../context' import d from 'debug' import { SessionContext } from '../session' +import type { HasAllOptionalProps } from './utilTypes' const debug = d('telegraf:scenes:context') const noop = () => Promise.resolve() const now = () => Math.floor(Date.now() / 1000) -export interface SceneContext - extends Context { - session: SceneSession - scene: SceneContextScene, D> -} +export type SceneSessionData = + HasAllOptionalProps extends never + ? // This is needed to give the developer some info about what's wrong + 'Error: All state properties must be optional.' + : { + current?: string + expires?: number + state?: S + } -export interface SceneSessionData { - current?: string - expires?: number - state?: Partial +export interface SceneSession< + S extends SceneSessionData & object = SceneSessionData +> { + __scenes?: S } -export interface SceneSession { - __scenes?: S +export interface SceneContext< + D extends SceneSessionData & object = SceneSessionData +> extends Context { + session: SceneSession + scene: SceneContextScene, D> } -export interface SceneContextSceneOptions { +export interface SceneContextSceneOptions< + D extends SceneSessionData & object +> { ttl?: number default?: string defaultSession: D @@ -32,7 +42,7 @@ export interface SceneContextSceneOptions { export default class SceneContextScene< C extends SessionContext>, - D extends SceneSessionData = SceneSessionData + D extends SceneSessionData & object = SceneSessionData > { private readonly options: SceneContextSceneOptions @@ -62,12 +72,11 @@ export default class SceneContextScene< return session } - get state(): Exclude { - // @ts-expect-error {} might not be assignable to Exclude + get state(): NonNullable { return (this.session.state ??= {}) } - set state(value: Exclude) { + set state(value: NonNullable) { this.session.state = { ...value } } @@ -83,8 +92,11 @@ export default class SceneContextScene< this.ctx.session.__scenes = Object.assign({}, this.options.defaultSession) } - // @ts-expect-error {} might not be assignable to Exclude - async enter(sceneId: string, initialState: Exclude = {}, silent = false) { + async enter( + sceneId: string, + initialState: NonNullable = {}, + silent = false + ) { if (!this.scenes.has(sceneId)) { throw new Error(`Can't find scene: ${sceneId}`) } diff --git a/src/scenes/stage.ts b/src/scenes/stage.ts index 02003e1a..23018e1e 100644 --- a/src/scenes/stage.ts +++ b/src/scenes/stage.ts @@ -12,7 +12,7 @@ export class Stage< C extends SessionContext> & { scene: SceneContextScene }, - D extends SceneSessionData = SceneSessionData + D extends SceneSessionData & object = SceneSessionData > extends Composer { options: Partial> scenes: Map> diff --git a/src/scenes/utilTypes.ts b/src/scenes/utilTypes.ts new file mode 100644 index 00000000..b391bef4 --- /dev/null +++ b/src/scenes/utilTypes.ts @@ -0,0 +1,16 @@ +import type { HasRequiredKeys, IsEqual } from 'type-fest' + +// These are the magic utility types + +export type RequireAllOptionalProps = + HasRequiredKeys extends true ? never : T + +// This ensures T extends RequireAllOptionalProps +// AND RequireAllOptionalProps extend T +// so in both directions +export type HasAllOptionalProps = IsEqual< + T, + RequireAllOptionalProps +> extends true + ? T + : never diff --git a/src/scenes/wizard/context.ts b/src/scenes/wizard/context.ts index 8429f5c7..c800f849 100644 --- a/src/scenes/wizard/context.ts +++ b/src/scenes/wizard/context.ts @@ -2,25 +2,28 @@ import SceneContextScene, { SceneSession, SceneSessionData } from '../context' import Context from '../../context' import { Middleware } from '../../middleware' import { SessionContext } from '../../session' +import type { HasAllOptionalProps } from '../utilTypes' -export interface WizardContext - extends Context { +export type WizardSessionData = + SceneSessionData extends string + ? SceneSessionData + : SceneSessionData & { cursor: number } + +export interface WizardContext< + D extends WizardSessionData & object = WizardSessionData +> extends Context { session: WizardSession scene: SceneContextScene, D> - wizard: WizardContextWizard> -} - -export interface WizardSessionData extends SceneSessionData { - cursor: number + wizard: WizardContextWizard, D> } -export interface WizardSession - extends SceneSession {} +export interface WizardSession< + S extends WizardSessionData & object = WizardSessionData +> extends SceneSession {} export default class WizardContextWizard< - C extends SessionContext & { - scene: SceneContextScene - } + C extends WizardContext, + D extends WizardSessionData & object = WizardSessionData > { readonly state: object constructor( diff --git a/src/scenes/wizard/index.ts b/src/scenes/wizard/index.ts index 1243d495..10fd21ca 100644 --- a/src/scenes/wizard/index.ts +++ b/src/scenes/wizard/index.ts @@ -1,34 +1,38 @@ import BaseScene, { SceneOptions } from '../base' import { Middleware, MiddlewareObj } from '../../middleware' -import WizardContextWizard, { WizardSessionData } from './context' +import WizardContextWizard, { + WizardSessionData, + WizardContext, +} from './context' import Composer from '../../composer' import Context from '../../context' import SceneContextScene from '../context' +type WC< + T extends WizardSessionData & object = WizardSessionData +> = WizardContext + export class WizardScene< - C extends Context & { - scene: SceneContextScene - wizard: WizardContextWizard - } + T extends WizardSessionData & object = WizardSessionData > - extends BaseScene - implements MiddlewareObj + extends BaseScene> + implements MiddlewareObj> { - steps: Array> + steps: Array>> - constructor(id: string, ...steps: Array>) + constructor(id: string, ...steps: Array>>) constructor( id: string, - options: SceneOptions, - ...steps: Array> + options: SceneOptions>, + ...steps: Array>> ) constructor( id: string, - options: SceneOptions | Middleware, - ...steps: Array> + options: SceneOptions> | Middleware>, + ...steps: Array>> ) { - let opts: SceneOptions | undefined - let s: Array> + let opts: SceneOptions> | undefined + let s: Array>> if (typeof options === 'function' || 'middleware' in options) { opts = undefined s = [options, ...steps] @@ -41,9 +45,9 @@ export class WizardScene< } middleware() { - return Composer.compose([ + return Composer.compose>([ (ctx, next) => { - ctx.wizard = new WizardContextWizard(ctx, this.steps) + ctx.wizard = new WizardContextWizard, T>(ctx, this.steps) return next() }, super.middleware(), From 1a3edb741cf329b0c56aa4785f3dd1552b51a049 Mon Sep 17 00:00:00 2001 From: Evert van Brussel Date: Tue, 25 Apr 2023 15:04:34 +0200 Subject: [PATCH 4/5] Fixed the mistakes --- src/scenes/context.ts | 57 ++++++++++++++++++++---------------- src/scenes/wizard/context.ts | 21 ++++++++----- src/scenes/wizard/index.ts | 7 ++--- 3 files changed, 48 insertions(+), 37 deletions(-) diff --git a/src/scenes/context.ts b/src/scenes/context.ts index 00f292ed..a817e1fd 100644 --- a/src/scenes/context.ts +++ b/src/scenes/context.ts @@ -9,56 +9,65 @@ const debug = d('telegraf:scenes:context') const noop = () => Promise.resolve() const now = () => Math.floor(Date.now() / 1000) -export type SceneSessionData = - HasAllOptionalProps extends never - ? // This is needed to give the developer some info about what's wrong +export type SceneSessionData< + S extends object, + MustBeValid extends boolean = false +> = HasAllOptionalProps extends never + ? MustBeValid extends true + ? never + : // This is needed to give the developer some info about what's wrong 'Error: All state properties must be optional.' - : { - current?: string - expires?: number - state?: S - } + : { + current?: string + expires?: number + state?: S + } export interface SceneSession< - S extends SceneSessionData & object = SceneSessionData + S extends SceneSessionData = SceneSessionData > { __scenes?: S } export interface SceneContext< - D extends SceneSessionData & object = SceneSessionData + D extends SceneSessionData = SceneSessionData > extends Context { session: SceneSession scene: SceneContextScene, D> } export interface SceneContextSceneOptions< - D extends SceneSessionData & object + D extends SceneSessionData > { ttl?: number default?: string defaultSession: D } +type RS> = NonNullable< + NonNullable>['session']>['__scenes'] +> + +type D> = NonNullable['state']> + export default class SceneContextScene< - C extends SessionContext>, - D extends SceneSessionData & object = SceneSessionData + C extends SessionContext>, + SD extends SceneSessionData = SceneSessionData > { - private readonly options: SceneContextSceneOptions + private readonly options: SceneContextSceneOptions constructor( private readonly ctx: C, private readonly scenes: Map>, - options: Partial> + options: Partial> ) { - // @ts-expect-error {} might not be assignable to D - const fallbackSessionDefault: D = {} + const fallbackSessionDefault = {} as SD this.options = { defaultSession: fallbackSessionDefault, ...options } } - get session(): D { - const defaultSession = Object.assign({}, this.options.defaultSession) + get session(): RS { + const defaultSession = { ...this.options.defaultSession } let session = this.ctx.session?.__scenes ?? defaultSession if (session.expires !== undefined && session.expires < now()) { @@ -72,11 +81,11 @@ export default class SceneContextScene< return session } - get state(): NonNullable { + get state(): D { return (this.session.state ??= {}) } - set state(value: NonNullable) { + set state(value: D) { this.session.state = { ...value } } @@ -92,11 +101,7 @@ export default class SceneContextScene< this.ctx.session.__scenes = Object.assign({}, this.options.defaultSession) } - async enter( - sceneId: string, - initialState: NonNullable = {}, - silent = false - ) { + async enter(sceneId: string, initialState: D = {}, silent = false) { if (!this.scenes.has(sceneId)) { throw new Error(`Can't find scene: ${sceneId}`) } diff --git a/src/scenes/wizard/context.ts b/src/scenes/wizard/context.ts index c800f849..aa139896 100644 --- a/src/scenes/wizard/context.ts +++ b/src/scenes/wizard/context.ts @@ -4,26 +4,33 @@ import { Middleware } from '../../middleware' import { SessionContext } from '../../session' import type { HasAllOptionalProps } from '../utilTypes' -export type WizardSessionData = - SceneSessionData extends string - ? SceneSessionData - : SceneSessionData & { cursor: number } +export type WizardSessionData< + T extends object = object, + MustBeValid extends boolean = false +> = SceneSessionData extends string + ? MustBeValid extends true + ? never + : SceneSessionData + : SceneSessionData & { cursor: number } +// Adding `& Cursor` guarantees that we're not getting the string case export interface WizardContext< - D extends WizardSessionData & object = WizardSessionData + D extends WizardSessionData = WizardSessionData > extends Context { session: WizardSession scene: SceneContextScene, D> wizard: WizardContextWizard, D> } +// Adding `& Cursor` guarantees that we're not getting the string case export interface WizardSession< - S extends WizardSessionData & object = WizardSessionData + S extends WizardSessionData = WizardSessionData > extends SceneSession {} +// Adding `& Cursor` guarantees that we're not getting the string case export default class WizardContextWizard< C extends WizardContext, - D extends WizardSessionData & object = WizardSessionData + D extends WizardSessionData = WizardSessionData > { readonly state: object constructor( diff --git a/src/scenes/wizard/index.ts b/src/scenes/wizard/index.ts index 10fd21ca..b72b4d60 100644 --- a/src/scenes/wizard/index.ts +++ b/src/scenes/wizard/index.ts @@ -8,12 +8,11 @@ import Composer from '../../composer' import Context from '../../context' import SceneContextScene from '../context' -type WC< - T extends WizardSessionData & object = WizardSessionData -> = WizardContext +type WC = WizardSessionData> = + WizardContext export class WizardScene< - T extends WizardSessionData & object = WizardSessionData + T extends WizardSessionData = WizardSessionData > extends BaseScene> implements MiddlewareObj> From 78b5091dc3431894f7466e19f0d21a030b715890 Mon Sep 17 00:00:00 2001 From: Evert van Brussel Date: Tue, 25 Apr 2023 16:52:03 +0200 Subject: [PATCH 5/5] This commit still leaves some TS errors, but it's a WIP --- src/scenes/context.ts | 22 ++++++++++++++++------ src/scenes/stage.ts | 30 +++++++++++++++++------------- src/scenes/utilTypes.ts | 2 ++ src/scenes/wizard/context.ts | 4 ++-- 4 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/scenes/context.ts b/src/scenes/context.ts index a817e1fd..2fc7f885 100644 --- a/src/scenes/context.ts +++ b/src/scenes/context.ts @@ -9,6 +9,12 @@ const debug = d('telegraf:scenes:context') const noop = () => Promise.resolve() const now = () => Math.floor(Date.now() / 1000) +export type DefaultSceneSessionData = { + current?: string + expires?: number + state?: object +} + export type SceneSessionData< S extends object, MustBeValid extends boolean = false @@ -31,9 +37,9 @@ export interface SceneSession< export interface SceneContext< D extends SceneSessionData = SceneSessionData -> extends Context { +> { session: SceneSession - scene: SceneContextScene, D> + scene: SceneContextScene } export interface SceneContextSceneOptions< @@ -50,15 +56,19 @@ type RS> = NonNullable< type D> = NonNullable['state']> +type C> = SessionContext< + SceneSession +> + export default class SceneContextScene< - C extends SessionContext>, - SD extends SceneSessionData = SceneSessionData + SD extends SceneSessionData, + CC extends C = C > { private readonly options: SceneContextSceneOptions constructor( - private readonly ctx: C, - private readonly scenes: Map>, + private readonly ctx: C, + private readonly scenes: Map>>, options: Partial> ) { const fallbackSessionDefault = {} as SD diff --git a/src/scenes/stage.ts b/src/scenes/stage.ts index 23018e1e..ebf33ac9 100644 --- a/src/scenes/stage.ts +++ b/src/scenes/stage.ts @@ -1,18 +1,22 @@ import { isSessionContext, SessionContext } from '../session' import SceneContextScene, { + DefaultSceneSessionData, SceneContextSceneOptions, SceneSession, + SceneContext, SceneSessionData, } from './context' import { BaseScene } from './base' import { Composer } from '../composer' import { Context } from '../context' +import { Modify } from './utilTypes' + +type D = DefaultSceneSessionData export class Stage< C extends SessionContext> & { - scene: SceneContextScene - }, - D extends SceneSessionData & object = SceneSessionData + scene: SceneContextScene + } > extends Composer { options: Partial> scenes: Map> @@ -41,7 +45,7 @@ export class Stage< const handler = Composer.compose([ (ctx, next) => { const scenes: Map> = this.scenes - const scene = new SceneContextScene(ctx, scenes, this.options) + const scene = new SceneContextScene(ctx, scenes, this.options) ctx.scene = scene return next() }, @@ -51,21 +55,21 @@ export class Stage< return Composer.optional(isSessionContext, handler) } - static enter }>( - ...args: Parameters['enter']> + static enter, C>( + ...args: Parameters['scene']['enter']> ) { - return (ctx: C) => ctx.scene.enter(...args) + return (ctx: Modify>) => ctx.scene.enter(...args) } - static reenter }>( - ...args: Parameters['reenter']> + static reenter, C>( + ...args: Parameters['scene']['reenter']> ) { - return (ctx: C) => ctx.scene.reenter(...args) + return (ctx: Modify>) => ctx.scene.reenter(...args) } - static leave }>( - ...args: Parameters['leave']> + static leave, C>( + ...args: Parameters['scene']['leave']> ) { - return (ctx: C) => ctx.scene.leave(...args) + return (ctx: Modify>) => ctx.scene.leave(...args) } } diff --git a/src/scenes/utilTypes.ts b/src/scenes/utilTypes.ts index b391bef4..662bba29 100644 --- a/src/scenes/utilTypes.ts +++ b/src/scenes/utilTypes.ts @@ -14,3 +14,5 @@ export type HasAllOptionalProps = IsEqual< > extends true ? T : never + +export type Modify = Omit & K diff --git a/src/scenes/wizard/context.ts b/src/scenes/wizard/context.ts index aa139896..9af71a1b 100644 --- a/src/scenes/wizard/context.ts +++ b/src/scenes/wizard/context.ts @@ -18,8 +18,8 @@ export interface WizardContext< D extends WizardSessionData = WizardSessionData > extends Context { session: WizardSession - scene: SceneContextScene, D> - wizard: WizardContextWizard, D> + scene: SceneContextScene + wizard: WizardContextWizard } // Adding `& Cursor` guarantees that we're not getting the string case