Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop abusing Composer architecture #1

Closed
IlyaSemenov opened this issue Feb 9, 2022 · 3 comments
Closed

Stop abusing Composer architecture #1

IlyaSemenov opened this issue Feb 9, 2022 · 3 comments

Comments

@IlyaSemenov
Copy link
Owner

So far I've been using grammy-scenes with 2 bots, ~15 scenes, and ~100 "steps", and I wanted to share my observations.

What I like:

  • The whole concept of do/wait/resume. The code base is concise yet flexible. It surely gets things done.
  • Nested scenes. What is especially useful is the ability to return value from a nested scene.
  • Scene-local session storage with strong typings.
  • scene.call / ctx.scene.call (goto, exit, etc.) counterparts, where the former is simply a shortcut for the latter.

What I don't like:

  • Abusing composer architecture, relying on the composer implementation details (on the fact that all composer API methods end up with .use()).
  • Unnamed (numbered) steps. This is easy for the developer but breaks UX for users that were in the middle of a scene when it gets slightly updated.
  • .always() feels counter-intuitive.

So I'm thinking on blending the two approaches, the one that I've used initially with the current one. I'm thinking like this:

const scene = new Scene<Context, LocalSession>("scene_name") // scene is again not a composer

scene.use() // <-- to be called always, such as for injecting ctx.something

scene.step().use(...) // reintroduce "step" as a building block

scene.wait("enter_xx").on(...) // always label wait blocks for reliability

scene.step("mylabel").use(...) // sometimes label steps, for goto

scene.goto("mylabel") // goto, call, exit, etc. work as before
IlyaSemenov added a commit that referenced this issue Apr 30, 2023
BREAKING CHANGES:

- Scene doesn't extend Composer anymore
- remove monkey patches
- add Composer2 and StepComposer with special methods
- remove undocumented SceneManager methods (wait, mustResume)
- prefix private props/methods with _
IlyaSemenov added a commit that referenced this issue Apr 30, 2023
BREAKING CHANGES:

- Scene doesn't extend Composer anymore
- remove monkey patches
- add Composer2 and StepComposer with special methods
- remove undocumented SceneManager methods (wait, mustResume)
- prefix private props/methods with _
@IlyaSemenov
Copy link
Owner Author

After playing around I went with a bit less radical approach. I cleaned the internals and stopped relying on composer internals. However, the API is almost the same and in most cases no changes to user code will be required. The scene still has use/do and wait as main building blocks as before, with all other previously publicly documented methods (label, goto, etc.) available.

The unnumbered steps problem was not addressed in this release, it will be covered separately.

@IlyaSemenov
Copy link
Owner Author

Oops! Seems like semantic release didn't recognize the breaking changes.

semantic-release/commit-analyzer#231

@IlyaSemenov IlyaSemenov reopened this Apr 30, 2023
IlyaSemenov added a commit that referenced this issue Apr 30, 2023
BREAKING CHANGE:

- Scene doesn't extend Composer anymore
- remove monkey patches
- add Composer2 and StepComposer with special methods
- remove undocumented SceneManager methods (wait, mustResume)
- prefix private props/methods with _
@IlyaSemenov
Copy link
Owner Author

Released in v8.0.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant