fix(Synchronizer): actually remove event listener on source removal

[hodossy]

Context

Synchronizer implementation adds the event listener with a .bind() call, effectively creating an anonymus function. When it tries to remove it, it uses a different function, resulting in a stale event listener on the element. We are using the Synchronizer to sync invert state across viewports, and sometimes enable/disable elements in the workflow. When we disable a viewport, we remove it from all synchronizers, and when we enable it, we add it back. This resulted in a state, where after odd number of disable/enable cycles we had an even number of event listeners (with the initial enablement), therefore invert is not working, as target viewports get inverted an even number of times (e.g. after one such cycle on source viewport, target viewport got inverted twice, thus nothing changed).

Changes & Results

• Synchronizer now has an _eventListener property referencing the event listener to add/remove
• Added a new test file with a single test to verify this invalid behavior and the fix itself

Testing

You can reset Synchronizer.ts to main, and run my test to validate that the event listener was actually called twice. Apply my changes again, and see the test pass.

Checklist

PR

• My Pull Request title is descriptive, accurate and follows the
  semantic-release format and guidelines.

Code

• My code has been well-documented (function documentation, inline comments,
  etc.)

Public Documentation Updates

• The documentation page has been updated as necessary for any public API
  additions or removals.

Tested Environment

• "OS: Ubuntu in WSL2 on Windows 11
• "Node version: v24.15.0
• "Browser: Chrome Headless 101.0.4950.0 (Linux 0.0.0)

Summary by CodeRabbit

• Bug Fixes
  • Fixed event listener cleanup and re-registration when a source viewport is removed and re-added, ensuring both primary and auxiliary events fire reliably.
• Tests
  • Added Jest coverage to verify listener behavior across remove/re-add cycles for primary and auxiliary events, confirming callbacks trigger exactly once after re-addition.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0393ed9a-b5a0-4bbc-a780-5454615b86f5

📥 Commits

Reviewing files that changed from the base of the PR and between cb02862 and 2abb886.

📒 Files selected for processing (1)

• packages/tools/src/store/SynchronizerManager/Synchronizer.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/tools/src/store/SynchronizerManager/Synchronizer.ts

---

📝 Walkthrough

Walkthrough

Synchronizer now reuses one bound event listener for source and auxiliary events during add and remove operations. Tests cover removing and re-adding a source viewport for both main and auxiliary events.

Changes

Synchronizer event listener lifecycle

Layer / File(s)	Summary
Stable event listener binding packages/tools/src/store/SynchronizerManager/Synchronizer.ts	Synchronizer stores one bound listener reference and uses it for event registration and removal on source and auxiliary events.
Remove and re-add test packages/tools/test/synchronizer_test.js	Tests set up two viewports, re-add a removed source, dispatch testEvent and auxiliaryEvent, and assert the callback is invoked once in each case.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A bunny hopped with tidy grace,
One listener kept its proper place.
Remove, re-add, then chirp once more—
Just one soft hop at eventful door. 🐇

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise, accurate, and matches the main fix while following the semantic-release format.
Description check	✅ Passed	The description follows the template and includes context, changes, testing steps, checklist, and environment details.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/tools/src/store/SynchronizerManager/Synchronizer.ts`:
- Around line 138-140: The auxiliary event teardown in Synchronizer should
mirror the attach default: addSource treats a missing source as 'element', but
removeSource currently detaches from eventTarget for { name } entries, leaving
listeners behind and causing duplicates. Update the remove logic in Synchronizer
so the same default source resolution is used for both attach and detach,
reusing the existing auxiliary event handling in _auxiliaryEvents and the
addSource/removeSource paths.

In `@packages/tools/test/synchronizer_test.js`:
- Around line 26-30: The afterEach cleanup in synchronizer_test.js is missing
the synchronizer identifier, so the created axialSync instance is not destroyed
and can leak global state between tests. Update each cleanupTestEnvironment call
inside the affected afterEach blocks to pass the same synchronizerId used when
creating the synchronizer, alongside renderingEngineId, and apply the same
change to the other cleanup block mentioned in the test file.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: baa67a06-ed97-41dd-b7e4-2717368f16b9

📥 Commits

Reviewing files that changed from the base of the PR and between fcab92d and ecf58ec.

📒 Files selected for processing (2)

• packages/tools/src/store/SynchronizerManager/Synchronizer.ts
• packages/tools/test/synchronizer_test.js