refactor(core): BreadcrumbStore added by Reversean · Pull Request #175 · codex-team/hawk.javascript

Reversean · 2026-03-25T20:42:46Z

abstract BreadcrumbStore added
BreadcrumbManager is now BrowserBreadcrumbStore

…n added

Copilot

Pull request overview

This PR refactors breadcrumb handling by introducing a shared BreadcrumbStore contract in @hawk.so/core and updating the JavaScript SDK to use a browser-specific implementation (BrowserBreadcrumbStore) instead of the previous BreadcrumbManager.

Changes:

Added BreadcrumbStore (plus related types) to @hawk.so/core and re-exported them from the core entrypoint.
Refactored the JavaScript SDK breadcrumbs implementation to BrowserBreadcrumbStore and updated Catcher to use the new store API (add/get/clear).
Updated JavaScript tests and types to align with the new names and API surface (and removed the local breadcrumbs-api.ts type).

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
packages/javascript/tests/catcher.user.test.ts	Updates singleton reset/import to `BrowserBreadcrumbStore`.
packages/javascript/tests/catcher.transport.test.ts	Updates singleton reset/import to `BrowserBreadcrumbStore`.
packages/javascript/tests/catcher.test.ts	Updates imports/reset and helper ordering.
packages/javascript/tests/catcher.global-handlers.test.ts	Updates singleton reset/import to `BrowserBreadcrumbStore`.
packages/javascript/tests/catcher.context.test.ts	Updates singleton reset/import to `BrowserBreadcrumbStore`.
packages/javascript/tests/catcher.breadcrumbs.test.ts	Updates singleton reset/import to `BrowserBreadcrumbStore`.
packages/javascript/tests/catcher.addons.test.ts	Updates singleton reset/import to `BrowserBreadcrumbStore`.
packages/javascript/tests/breadcrumbs.test.ts	Renames the suite and updates API calls to `add/get/clear`.
packages/javascript/src/types/index.ts	Re-exports breadcrumb store types from `@hawk.so/core`.
packages/javascript/src/types/breadcrumbs-api.ts	Removes the local `BreadcrumbsAPI` definition in favor of core types.
packages/javascript/src/catcher.ts	Switches from `BreadcrumbManager` to `BrowserBreadcrumbStore` and updates the public `breadcrumbs` getter typing.
packages/javascript/src/addons/breadcrumbs.ts	Introduces `BrowserBreadcrumbStore` implementing the core `BreadcrumbStore` contract and renames browser hint type.
packages/core/src/index.ts	Re-exports breadcrumb store types from the core package entrypoint.
packages/core/src/breadcrumbs/breadcrumb-store.ts	Adds the new shared breadcrumb store contract and associated types.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-31T16:39:35Z

packages/javascript/src/addons/breadcrumbs.ts

@@ -10,9 +11,10 @@ import { buildElementSelector, isValidBreadcrumb, log, Sanitizer } from '@hawk.s
 const DEFAULT_MAX_BREADCRUMBS = 15;

 /**
- * Hint object passed to beforeBreadcrumb callback
+ * Hint object passed to beforeBreadcrumb callback.
+ * Extends generic {@link BreadcrumbHint} with browser-specific data.
 */
-export interface BreadcrumbHint {
+export interface BrowserBreadcrumbHint extends BreadcrumbHint {
  /**


This module previously exported BreadcrumbHint, BreadcrumbInput, and BreadcrumbManager, but the refactor renames/removes them (BrowserBreadcrumbHint, core BreadcrumbInput, BrowserBreadcrumbStore). If any consumers deep-import these symbols, this becomes a breaking change. Consider keeping deprecated compatibility exports (e.g., aliasing the old names to the new ones / re-exporting the core types) to make migration non-breaking.

BreadcrumbHint, BreadcrumbInput, and BreadcrumbManager were never in public index.ts.

All we had exported is BreadcrumbsAPI which persisted, but marked as deprecated, since it's same thing as BreadcrumbStore, which has more convenient naming.

Copilot · 2026-03-31T16:39:36Z

packages/javascript/src/catcher.ts

@@ -123,7 +124,7 @@ export default class Catcher {
  /**
   * Breadcrumb manager instance


The JSDoc says "Breadcrumb manager instance" but the field is now a breadcrumb store (breadcrumbStore). Please update the comment to match the new naming to avoid confusion during maintenance.

Suggested change

* Breadcrumb manager instance

* Breadcrumb store instance

Copilot · 2026-03-31T16:39:36Z

packages/javascript/tests/breadcrumbs.test.ts

 function resetManager(): void {
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  (BreadcrumbManager as any).instance = null;
+  (BrowserBreadcrumbStore as any).instance = null;
 }


Tests reset the singleton by mutating a private static field via (BrowserBreadcrumbStore as any).instance = null. This bypasses destroy() and can leak monkeypatched globals/event listeners between tests (fetch/XHR/history/click handlers). Prefer a public reset helper (or calling destroy() on the existing instance) so globals are reliably restored and tests don't depend on private internals.

Now calling destroy() on instance instead of assigning null to it.

Copilot · 2026-03-31T16:39:36Z

packages/javascript/tests/catcher.user.test.ts

    localStorage.clear();
    mockParse.mockResolvedValue([]);
-    (BreadcrumbManager as any).instance = null;
+    (BrowserBreadcrumbStore as any).instance = null;


Tests reset the singleton by mutating a private static field via (BrowserBreadcrumbStore as any).instance = null. This bypasses destroy() and can leak monkeypatched globals/event listeners between tests. Prefer a public reset helper (or calling destroy() on the existing instance) so globals are reliably restored and tests don't depend on private internals.

Suggested change

(BrowserBreadcrumbStore as any).instance = null;

const instance = (BrowserBreadcrumbStore as any).instance as { destroy?: () => void } | null | undefined;

if (instance && typeof instance.destroy === 'function') {

instance.destroy();

}

Just calling destroy on instance before tests.

Fixed.

Copilot · 2026-03-31T16:39:37Z

packages/javascript/tests/catcher.transport.test.ts

    localStorage.clear();
    mockParse.mockResolvedValue([]);
-    (BreadcrumbManager as any).instance = null;
+    (BrowserBreadcrumbStore as any).instance = null;


Tests reset the singleton by mutating a private static field via (BrowserBreadcrumbStore as any).instance = null. This bypasses destroy() and can leak monkeypatched globals/event listeners between tests. Prefer a public reset helper (or calling destroy() on the existing instance) so globals are reliably restored and tests don't depend on private internals.

Suggested change

(BrowserBreadcrumbStore as any).instance = null;

const instance = (BrowserBreadcrumbStore as any).instance as { destroy?: () => void } | null | undefined;

if (instance && typeof instance.destroy === 'function') {

instance.destroy();

}

Copilot · 2026-03-31T16:39:37Z

packages/javascript/tests/catcher.test.ts

    localStorage.clear();
    mockParse.mockResolvedValue([]);
-    (BreadcrumbManager as any).instance = null;
+    (BrowserBreadcrumbStore as any).instance = null;


Tests reset the singleton by mutating a private static field via (BrowserBreadcrumbStore as any).instance = null. This bypasses destroy() and can leak monkeypatched globals/event listeners between tests. Prefer a public reset helper (or calling destroy() on the existing instance) so globals are reliably restored and tests don't depend on private internals.

Suggested change

(BrowserBreadcrumbStore as any).instance = null;

const currentInstance = (BrowserBreadcrumbStore as any).instance;

if (currentInstance && typeof currentInstance.destroy === 'function') {

currentInstance.destroy();

}

Copilot · 2026-03-31T16:39:37Z

packages/javascript/tests/catcher.global-handlers.test.ts

    localStorage.clear();
    mockParse.mockResolvedValue([]);
-    (BreadcrumbManager as any).instance = null;
+    (BrowserBreadcrumbStore as any).instance = null;


Tests reset the singleton by mutating a private static field via (BrowserBreadcrumbStore as any).instance = null. This bypasses destroy() and can leak monkeypatched globals/event listeners between tests. Prefer a public reset helper (or calling destroy() on the existing instance) so globals are reliably restored and tests don't depend on private internals.

Suggested change

(BrowserBreadcrumbStore as any).instance = null;

const instance = (BrowserBreadcrumbStore as any).instance;

if (instance && typeof instance.destroy === 'function') {

instance.destroy();

}

Copilot · 2026-03-31T16:39:37Z

packages/javascript/tests/catcher.context.test.ts

    localStorage.clear();
    mockParse.mockResolvedValue([]);
-    (BreadcrumbManager as any).instance = null;
+    (BrowserBreadcrumbStore as any).instance = null;


Tests reset the singleton by mutating a private static field via (BrowserBreadcrumbStore as any).instance = null. This bypasses destroy() and can leak monkeypatched globals/event listeners between tests. Prefer a public reset helper (or calling destroy() on the existing instance) so globals are reliably restored and tests don't depend on private internals.

Suggested change

(BrowserBreadcrumbStore as any).instance = null;

const anyStore = BrowserBreadcrumbStore as any;

if (typeof anyStore.getInstance === 'function') {

const instance = anyStore.getInstance();

if (instance && typeof instance.destroy === 'function') {

instance.destroy();

}

}

Copilot · 2026-03-31T16:39:38Z

packages/javascript/tests/catcher.breadcrumbs.test.ts

    localStorage.clear();
    mockParse.mockResolvedValue([]);
-    (BreadcrumbManager as any).instance = null;
+    (BrowserBreadcrumbStore as any).instance = null;


Tests reset the singleton by mutating a private static field via (BrowserBreadcrumbStore as any).instance = null. This bypasses destroy() and can leak monkeypatched globals/event listeners between tests. Prefer a public reset helper (or calling destroy() on the existing instance) so globals are reliably restored and tests don't depend on private internals.

Suggested change

(BrowserBreadcrumbStore as any).instance = null;

const instance = (BrowserBreadcrumbStore as any).instance;

if (instance && typeof instance.destroy === 'function') {

instance.destroy();

}

Copilot · 2026-03-31T16:39:38Z

packages/javascript/tests/catcher.addons.test.ts

    localStorage.clear();
    mockParse.mockResolvedValue([]);
-    (BreadcrumbManager as any).instance = null;
+    (BrowserBreadcrumbStore as any).instance = null;


Tests reset the singleton by mutating a private static field via (BrowserBreadcrumbStore as any).instance = null. This bypasses destroy() and can leak monkeypatched globals/event listeners between tests. Prefer a public reset helper (or calling destroy() on the existing instance) so globals are reliably restored and tests don't depend on private internals.

Suggested change

(BrowserBreadcrumbStore as any).instance = null;

// Reset BrowserBreadcrumbStore via its public API if available, rather than mutating private internals.

(BrowserBreadcrumbStore as any).destroy?.();

Dobrunia · 2026-04-01T18:02:42Z

don't forget to bump version

- fix breadcrumbStore field TSDoc in catcher - destroy BrowserBreadcrumbStore instance before each tests

Reversean · 2026-04-01T18:29:38Z

don't forget to bump version

Thx for reminder.

refactor(core): BreadcrumbStore abstraction and browser implementatio…

8df48ec

…n added

neSpecc requested a review from Copilot March 31, 2026 16:33

Copilot started reviewing on behalf of neSpecc March 31, 2026 16:33 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

neSpecc requested a review from Dobrunia April 1, 2026 17:54

Dobrunia approved these changes Apr 1, 2026

View reviewed changes

fix: fixed review comments

12d6786

- fix breadcrumbStore field TSDoc in catcher - destroy BrowserBreadcrumbStore instance before each tests

chore: bump @hawk.so/javascript to 3.2.19

9890ddb

		@@ -123,7 +124,7 @@ export default class Catcher {
		/**
		* Breadcrumb manager instance

-    (BrowserBreadcrumbStore as any).instance = null;
+    const instance = (BrowserBreadcrumbStore as any).instance as { destroy?: () => void } | null | undefined;
+    if (instance && typeof instance.destroy === 'function') {
+      instance.destroy();
+    }

-    (BrowserBreadcrumbStore as any).instance = null;
+    const currentInstance = (BrowserBreadcrumbStore as any).instance;
+    if (currentInstance && typeof currentInstance.destroy === 'function') {
+      currentInstance.destroy();
+    }

	(BrowserBreadcrumbStore as any).instance = null;
	// Reset BrowserBreadcrumbStore via its public API if available, rather than mutating private internals.
	(BrowserBreadcrumbStore as any).destroy?.();

Conversation

Reversean commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dobrunia commented Apr 1, 2026

Uh oh!

Reversean commented Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants