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.

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.

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();

}

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?.();

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(core): BreadcrumbStore added#175

refactor(core): BreadcrumbStore added#175
Reversean wants to merge 1 commit intomasterfrom
refactor/breadcrumb-store

Reversean commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -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!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants