Skip to content

fix: use unique IDs to prevent instance collisions across bundles#126

Open
layershifter wants to merge 1 commit into
microsoft:mainfrom
layershifter:fix/unique-instance-ids
Open

fix: use unique IDs to prevent instance collisions across bundles#126
layershifter wants to merge 1 commit into
microsoft:mainfrom
layershifter:fix/unique-instance-ids

Conversation

@layershifter

@layershifter layershifter commented Mar 4, 2026

Copy link
Copy Markdown
Member

Summary

  • Replaces the module-scoped _lastId counter with crypto.getRandomValues() to generate unique instance IDs
  • When multiple bundles include keyborg on the same page, they each had their own _lastId starting at 0, causing ID collisions in the shared window.__keyborg.refs map
  • With random IDs, each instance is guaranteed unique regardless of how many bundles coexist
  • Uses crypto.getRandomValues() (not crypto.randomUUID()) to support Edge 84+, Firefox 75+, Chrome 84+, Safari 14.1+

Closes #99
Supersedes #75 and #104

Test plan

  • npm run build passes
  • npm run lint passes
  • All existing Playwright tests pass (10/10)

🤖 Generated with Claude Code

@layershifter
layershifter requested review from a team and mshoho as code owners March 4, 2026 11:06
@github-actions

github-actions Bot commented Mar 12, 2026

Copy link
Copy Markdown

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
keyborg
All exports
3.363 kB
1.409 kB
3.495 kB
1.488 kB
132 B
79 B
keyborg
createKeyborg() & disposeKeyborg()
3.195 kB
1.368 kB
3.327 kB
1.445 kB
132 B
77 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
keyborg
KEYBORG_FOCUSIN constant
64 B
80 B
🤖 This report was generated against 103b64b1dec3f593e512b93f845116a22a98a9bd

@mathis-m

Copy link
Copy Markdown

@layershifter could you provide insights on planned release date of this?

Replace the module-scoped _lastId counter with crypto.getRandomValues()
to generate unique IDs. This prevents ID collisions when multiple bundles
include keyborg on the same page, where each bundle had its own _lastId
starting at 0 and produced colliding keys in the shared window.__keyborg.refs map.

Uses crypto.getRandomValues() (not crypto.randomUUID()) to support
Edge 84+, Firefox 75+, Chrome 84+, Safari 14.1+.

Closes microsoft#99

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@layershifter
layershifter force-pushed the fix/unique-instance-ids branch from b261ee9 to 7b4cb62 Compare June 19, 2026 09:35
@layershifter

Copy link
Copy Markdown
Member Author

@mathis-m asked @mshoho to check it

Comment thread src/Keyborg.mts
export function createKeyborg(win: Window): Keyborg {
const kwin = win as WindowWithKeyborg;
const id = "k" + ++_lastId;
const id = generateId();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If I am paranoid, I would use both incremented id + random number, not just one of them. random numbers can repeat (with unlikely, but non-zero probability). incremented part fully guarantees absence of the same instance collisions and strongly reduces slim chances of the multiple instance collision :).

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.

Keyborg instances are overriden when multiple code instances are deployed on the same site

3 participants