Skip to content

Conversation

@ggazzo
Copy link
Member

@ggazzo ggazzo commented Dec 15, 2025

closes https://rocketchat.atlassian.net/browse/FB-138

Summary by CodeRabbit

  • Refactor
    • Simplified event system: all event subscription and emission now go through a single internal dispatcher; external emitter injection removed.
    • Initialization streamlined: SDK init no longer accepts or wires an external emitter.
    • Public event API simplified: subscribe/unsubscribe/emit operations now target only the internal event mechanism.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 15, 2025

Walkthrough

Removes runtime injection and usage of an external emitter from the federation SDK: the emitter parameter and setEmitter wiring were removed from init, and EventEmitterService no longer stores or forwards to an external oldEmitter; event operations now rely solely on the internal AsyncDispatcher.

Changes

Cohort / File(s) Summary
Init signature & wiring
packages/federation-sdk/src/index.ts
Removed emitter parameter from init and deleted code that created EventEmitterService.setEmitter(...); init now only accepts dbConfig and proceeds with DB and listener setup.
EventEmitter service simplification
packages/federation-sdk/src/services/event-emitter.service.ts
Deleted oldEmitter field and setEmitter() method; emit, on, once, and off now operate exclusively against the internal AsyncDispatcher and no longer forward to an external emitter.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify no other modules call the removed setEmitter() or expect an external emitter to be present.
  • Confirm tests (or consumers) relying on dual-emitter behavior are updated or unaffected.
  • Check event delivery behavior in integration points that previously relied on external forwarding.

Possibly related PRs

Suggested reviewers

  • sampaiodiego
  • ricardogarim

Poem

🐰 I nibbled the old emitter right out of the hay,
One dispatcher now hops where the signals all play.
No echoing whispers, no dual-path surprise,
Just tidy, neat hops under clear rabbit skies. 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main changes: removing old emitter handling from EventEmitterService and cleaning up the init function, which aligns with the actual changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/duplication-emit

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ggazzo ggazzo marked this pull request as ready for review December 15, 2025 13:57
@ggazzo ggazzo force-pushed the fix/duplication-emit branch from cffdb17 to efba5b5 Compare December 15, 2025 13:58
@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 53.15%. Comparing base (7346b3a) to head (efba5b5).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #318      +/-   ##
==========================================
- Coverage   53.37%   53.15%   -0.23%     
==========================================
  Files          96       96              
  Lines       12827    12796      -31     
==========================================
- Hits         6847     6802      -45     
- Misses       5980     5994      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ggazzo ggazzo merged commit a3ab397 into main Dec 15, 2025
3 checks passed
@ggazzo ggazzo deleted the fix/duplication-emit branch December 15, 2025 14:21
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.

4 participants