Skip to content

fix: Method name is emit_event, not emit #2215

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

Merged

Conversation

alexrecuenco
Copy link
Contributor

@alexrecuenco alexrecuenco commented Mar 14, 2025

Fixes #2214

Description

The function name is emit_event, not emit

Specific Changes

  • Modified the name

Testing

I would need guidance on what test to modify that currently has emit_event in the codebase.

@alexrecuenco alexrecuenco requested a review from a team as a code owner March 14, 2025 22:58
@tracyboehrer tracyboehrer requested a review from axelsrz March 17, 2025 18:02
@axelsrz
Copy link
Member

axelsrz commented Mar 19, 2025

@alexrecuenco were you able to hit this bug in actual usage?

From an initial investigation, it looks like this was code used as preparation for another set of features that are not currently in this SDK

@alexrecuenco
Copy link
Contributor Author

alexrecuenco commented Mar 20, 2025

@alexrecuenco were you able to hit this bug in actual usage?

From an initial investigation, it looks like this was code used as preparation for another set of features that are not currently in this SDK

Yes. I emit events, and then those events get captured by a parent component to do some resource refresh handling

My hotfix is to monkey-patch emit to be equal to emit event. With that monkey-patch, this works perfectly well.

The feature exists in the botbuilder-js, and here it is working perfectly well once you do that.

@alexrecuenco
Copy link
Contributor Author

Yes. I emit events, and then those events get captured by a parent component to do some resource refresh handling

In particular, this is doing some life cycle generic component, that I am hoping to open source soon.

Any resource that is generated through a dialog with a user is encapsulated.

In particular, the main use is something akin to an authorization middleware within a bot. So you have an AuthorizedComponent that generates the sufficient context, and this component encapsulates token authorization.

The internal dialog then has in the turn state the reference to the required resource/authorization, without having to manually handle authorization in each dialog.

And for long running tasks, internal resources can extend that token by calling refresh(dialog_context), that in turns emits an event and is captured by the nearest matching AuthorizedComponent that refreshes the token/resource.

@alexrecuenco alexrecuenco force-pushed the patch-dialogcontext-events branch from 8cf0f53 to 1ce0152 Compare March 21, 2025 19:11
@alexrecuenco
Copy link
Contributor Author

You approved it, but does it need anything else to he merged?

I see it is still not executing the tests, it says that it needs maintainer permissions.

@tracyboehrer tracyboehrer merged commit ec22a89 into microsoft:main Mar 25, 2025
2 checks passed
@alexrecuenco alexrecuenco deleted the patch-dialogcontext-events branch March 25, 2025 17:33
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

Successfully merging this pull request may close these issues.

[BUG] DialogEvent has no method emit when calling component dialog.
3 participants