-
Notifications
You must be signed in to change notification settings - Fork 4
refactor(core): BreadcrumbStore added #175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Reversean
wants to merge
2
commits into
master
Choose a base branch
from
refactor/breadcrumb-store
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| import type { Breadcrumb } from '@hawk.so/types'; | ||
|
|
||
| /** | ||
| * Hint passed to beforeBreadcrumb callback. | ||
| */ | ||
| export interface BreadcrumbHint { | ||
| [key: string]: unknown; | ||
| } | ||
|
|
||
| /** | ||
| * Breadcrumb input type - breadcrumb data with optional timestamp. | ||
| */ | ||
| export type BreadcrumbInput = Omit<Breadcrumb, 'timestamp'> & { timestamp?: number }; | ||
|
|
||
| /** | ||
| * Contract for breadcrumb storage. Also serves as public breadcrumbs API. | ||
| */ | ||
| export interface BreadcrumbStore { | ||
| add(breadcrumb: BreadcrumbInput, hint?: BreadcrumbHint): void; | ||
| get(): Breadcrumb[]; | ||
| clear(): void; | ||
| } | ||
|
|
||
| /** | ||
| * @deprecated Use {@link BreadcrumbStore} instead. | ||
| */ | ||
| export type BreadcrumbsAPI = BreadcrumbStore; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
| * @file Breadcrumbs module - captures chronological trail of events before an error | ||
| */ | ||
| import type { Breadcrumb, BreadcrumbLevel, BreadcrumbType, Json, JsonNode } from '@hawk.so/types'; | ||
| import type { BreadcrumbHint, BreadcrumbInput, BreadcrumbStore } from '@hawk.so/core'; | ||
| import { buildElementSelector, isValidBreadcrumb, log, Sanitizer } from '@hawk.so/core'; | ||
|
|
||
| /** | ||
|
|
@@ -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 { | ||
| /** | ||
| * Original event that triggered the breadcrumb (if any) | ||
| */ | ||
|
|
@@ -51,7 +53,7 @@ export interface BreadcrumbsOptions { | |
| * - Return `false` — the breadcrumb will be discarded. | ||
| * - Any other value is invalid — the original breadcrumb is stored as-is (a warning is logged). | ||
| */ | ||
| beforeBreadcrumb?: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => Breadcrumb | false | void; | ||
| beforeBreadcrumb?: (breadcrumb: Breadcrumb, hint?: BrowserBreadcrumbHint) => Breadcrumb | false | void; | ||
|
|
||
| /** | ||
| * Enable automatic fetch/XHR breadcrumbs | ||
|
|
@@ -75,12 +77,6 @@ export interface BreadcrumbsOptions { | |
| trackClicks?: boolean; | ||
| } | ||
|
|
||
| /** | ||
| * Breadcrumb input type - breadcrumb data with optional timestamp | ||
| * (timestamp will be auto-generated if not provided) | ||
| */ | ||
| export type BreadcrumbInput = Omit<Breadcrumb, 'timestamp'> & { timestamp?: Breadcrumb['timestamp'] }; | ||
|
|
||
| /** | ||
| * Internal breadcrumbs options - all fields except 'beforeBreadcrumb' are required | ||
| * (they have default values and are always set during init) | ||
|
|
@@ -90,17 +86,18 @@ interface InternalBreadcrumbsOptions { | |
| trackFetch: boolean; | ||
| trackNavigation: boolean; | ||
| trackClicks: boolean; | ||
| beforeBreadcrumb?: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => Breadcrumb | false | void; | ||
| beforeBreadcrumb?: (breadcrumb: Breadcrumb, hint?: BrowserBreadcrumbHint) => Breadcrumb | false | void; | ||
| } | ||
|
|
||
| /** | ||
| * BreadcrumbManager - singleton that manages breadcrumb collection and storage | ||
| * Browser implementation of BreadcrumbStore. | ||
| * Singleton that manages breadcrumb collection and storage. | ||
| */ | ||
| export class BreadcrumbManager { | ||
| export class BrowserBreadcrumbStore implements BreadcrumbStore { | ||
| /** | ||
| * Singleton instance | ||
| */ | ||
| private static instance: BreadcrumbManager | null = null; | ||
| private static instance: BrowserBreadcrumbStore | null = null; | ||
|
|
||
| /** | ||
| * Breadcrumbs buffer (FIFO) | ||
|
|
@@ -167,10 +164,10 @@ export class BreadcrumbManager { | |
| /** | ||
| * Get singleton instance | ||
| */ | ||
| public static getInstance(): BreadcrumbManager { | ||
| BreadcrumbManager.instance ??= new BreadcrumbManager(); | ||
| public static getInstance(): BrowserBreadcrumbStore { | ||
| BrowserBreadcrumbStore.instance ??= new BrowserBreadcrumbStore(); | ||
|
|
||
| return BreadcrumbManager.instance; | ||
| return BrowserBreadcrumbStore.instance; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -180,7 +177,7 @@ export class BreadcrumbManager { | |
| */ | ||
| public init(options: BreadcrumbsOptions = {}): void { | ||
| if (this.isInitialized) { | ||
| log('[BreadcrumbManager] init has already been called; breadcrumb configuration is global and subsequent init options are ignored.', 'warn'); | ||
| log('[BrowserBreadcrumbStore] init has already been called; breadcrumb configuration is global and subsequent init options are ignored.', 'warn'); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think users should see such a log. It's better to remove it. |
||
|
|
||
| return; | ||
| } | ||
|
|
@@ -219,7 +216,7 @@ export class BreadcrumbManager { | |
| * @param hint - Optional hint object with original event data (Event, Response, XMLHttpRequest, etc.) | ||
| * Used by beforeBreadcrumb callback to access original event context | ||
| */ | ||
| public addBreadcrumb(breadcrumb: BreadcrumbInput, hint?: BreadcrumbHint): void { | ||
| public add(breadcrumb: BreadcrumbInput, hint?: BrowserBreadcrumbHint): void { | ||
| /** | ||
| * Ensure timestamp | ||
| */ | ||
|
|
@@ -293,14 +290,14 @@ export class BreadcrumbManager { | |
| /** | ||
| * Get current breadcrumbs snapshot (oldest to newest) | ||
| */ | ||
| public getBreadcrumbs(): Breadcrumb[] { | ||
| public get(): Breadcrumb[] { | ||
| return [ ...this.breadcrumbs ]; | ||
| } | ||
|
|
||
| /** | ||
| * Clear all breadcrumbs | ||
| */ | ||
| public clearBreadcrumbs(): void { | ||
| public clear(): void { | ||
| this.breadcrumbs.length = 0; | ||
| } | ||
|
|
||
|
|
@@ -358,9 +355,9 @@ export class BreadcrumbManager { | |
| this.popstateHandler = null; | ||
| } | ||
|
|
||
| this.clearBreadcrumbs(); | ||
| this.clear(); | ||
| this.isInitialized = false; | ||
| BreadcrumbManager.instance = null; | ||
| BrowserBreadcrumbStore.instance = null; | ||
| } | ||
|
|
||
|
|
||
|
|
@@ -399,7 +396,7 @@ export class BreadcrumbManager { | |
|
|
||
| const duration = Date.now() - startTime; | ||
|
|
||
| manager.addBreadcrumb({ | ||
| manager.add({ | ||
| type: 'request', | ||
| category: 'fetch', | ||
| message: `${response.status} ${method} ${url}`, | ||
|
|
@@ -419,7 +416,7 @@ export class BreadcrumbManager { | |
| } catch (error) { | ||
| const duration = Date.now() - startTime; | ||
|
|
||
| manager.addBreadcrumb({ | ||
| manager.add({ | ||
| type: 'request', | ||
| category: 'fetch', | ||
| message: `[FAIL] ${method} ${url}`, | ||
|
|
@@ -483,7 +480,7 @@ export class BreadcrumbManager { | |
| const url = this.hawkUrl || ''; | ||
| const status = this.status; | ||
|
|
||
| manager.addBreadcrumb({ | ||
| manager.add({ | ||
| type: 'request', | ||
| category: 'xhr', | ||
| message: `${status} ${method} ${url}`, | ||
|
|
@@ -529,7 +526,7 @@ export class BreadcrumbManager { | |
|
|
||
| lastUrl = to; | ||
|
|
||
| manager.addBreadcrumb({ | ||
| manager.add({ | ||
| type: 'navigation', | ||
| category: 'navigation', | ||
| message: `Navigated to ${to}`, | ||
|
|
@@ -599,7 +596,7 @@ export class BreadcrumbManager { | |
| */ | ||
| const text = (target.textContent || target.innerText || '').trim().substring(0, 50); | ||
|
|
||
| manager.addBreadcrumb({ | ||
| manager.add({ | ||
| type: 'ui', | ||
| category: 'ui.click', | ||
| message: `Click on ${selector}`, | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This module previously exported
BreadcrumbHint,BreadcrumbInput, andBreadcrumbManager, but the refactor renames/removes them (BrowserBreadcrumbHint, coreBreadcrumbInput,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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BreadcrumbHint,BreadcrumbInput, andBreadcrumbManagerwere never in publicindex.ts.All we had exported is
BreadcrumbsAPIwhich persisted, but marked as deprecated, since it's same thing asBreadcrumbStore, which has more convenient naming.