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.

Sign up for GitHub

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

Jump to bottom

fix(types): add typings for EventBus #14122

Merged
merged 3 commits into from
Jan 14, 2025
Merged

fix(types): add typings for EventBus #14122

merged 3 commits into from
Jan 14, 2025

Conversation

Antreesy
Copy link
Contributor

@Antreesy Antreesy commented Jan 13, 2025 •
edited
Loading

☑️ Resolves

  • Fix unknown types in TS

🖌️ UI Checklist

🚧 Tasks

  • Check regression from mitt()

🏁 Checklist

  • 🌏 Tested with different browsers / clients:
    • Chromium (Chrome / Edge / Opera / Brave)
    • Firefox
    • Safari
    • Talk Desktop
    • Not risky to browser differences / client

@Antreesy Antreesy self-assigned this Jan 13, 2025
@Antreesy Antreesy added this to the 🌿 Next Beta (31) milestone Jan 13, 2025
@Antreesy Antreesy force-pushed the fix/11130/event-bus-types branch from 88f5bee to a527b0b Compare January 13, 2025 15:42
@Antreesy
Copy link
Contributor Author

Antreesy commented Jan 13, 2025 •
edited
Loading

cc @danxuliu could you confirm signaling-recording-status-changed event works correctly? 4511978

There was probably a regression after migration to a new library: #11996

@Antreesy
Copy link
Contributor Author

/backport to stable30

ShGKme
ShGKme approved these changes Jan 13, 2025
View reviewed changes
Copy link
Contributor

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch

src/types/eventBusTypes.ts Outdated Show resolved Hide resolved
src/types/eventBusTypes.ts Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd keep event buss types in the origin EventBus module. Both modules are not reusable and describe one entity - EventBus, especially GenericEventHandler and ExtendedEmitter.

Events less coupled, but if it is a single type with all the events of the event bus, I'd keep it in the bus. Alternatively, it can be an interface instead of a type, and be extendable in paces new events are created.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep it in one place then. We don't have yet much .ts places using it

@Antreesy Antreesy force-pushed the fix/11130/event-bus-types branch from a527b0b to 2de55d1 Compare January 14, 2025 08:33
@Antreesy Antreesy merged commit fa07be7 into main Jan 14, 2025
50 checks passed
@Antreesy Antreesy deleted the fix/11130/event-bus-types branch January 14, 2025 08:58
@backportbot backportbot bot mentioned this pull request Jan 14, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShGKme ShGKme ShGKme approved these changes

Assignees

@Antreesy Antreesy

Labels
3. to review feature: frontend 🖌️ "Web UI" client technical debt
Projects
None yet
Milestone
v21.0.0-beta.2
Development

Successfully merging this pull request may close these issues.
2 participants
@Antreesy @ShGKme