feat: reset api with options#2445
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a typed ResetOptions and extends the SDK reset API to accept an options object (with boolean fallback). Implements per-key reset logic in UserSessionManager, introduces DEFAULT_RESET_OPTIONS and deepFreeze, updates tests, and adjusts a few build/config defaults. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant RA as RudderAnalytics
participant Core as Analytics
participant USM as UserSessionManager
participant Utils as getFinalResetOptions
App->>RA: reset(options?: ResetOptions | boolean)
RA->>Core: reset(options)
alt SDK not loaded (buffered)
Core->>Core: push ['reset', options]
else SDK loaded
Core->>USM: reset(options)
USM->>Utils: getFinalResetOptions(options)
Utils-->>USM: computed ResetOptions
USM->>USM: iterate opts.entries and apply per-key resets
alt entries.anonymousId true
USM->>USM: generate/set new anonymousId
end
alt entries.sessionInfo true
USM->>USM: resetAndStartNewSession() (auto/manual start logic)
end
USM-->>Core: reset complete
end
Core-->>RA: return
RA-->>App: void
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (13)
packages/analytics-js/.size-limit.mjs (1)
50-50: Avoid bumping size limits without CODEOWNERS’ sign-offHeader explicitly says “DO NOT EDIT.” Please either:
- Revert the bump to 28.5 KiB and keep the budget at 28 KiB, or
- Add justification in the PR description and secure CODEOWNERS approval.
If the increase is unavoidable due to the new reset flow, please paste size-limit output before/after and enumerate what pushed the delta (e.g., new utilities or deep-merge helper).
packages/analytics-js/src/components/userSessionManager/types.ts (1)
23-23: Document boolean as deprecated to steer users to options-based APIThe union keeps backward compatibility, but callers won’t see deprecation guidance. Consider a short JSDoc to nudge migration.
export interface IUserSessionManager { @@ - reset(options?: ResetOptions | boolean): void; + /** + * Clear user session information. + * Passing a boolean toggles only `entries.anonymousId` (legacy behavior). + * @deprecated Use ResetOptions for fine-grained control. + */ + reset(options?: ResetOptions | boolean): void;packages/analytics-js-common/src/types/EventApi.ts (1)
49-53: Add succinct docs and prefer Partial<Record<…>> for readability (optional)The type is correct. A tiny doc helps discoverability; Partial<Record<…>> is equivalent and a bit clearer.
-export type ResetOptions = { - entries: { - [key in UserSessionKey]?: boolean; - }; -}; +/** + * Options to control which user-session entries are cleared. + * Omitted keys are left untouched. + */ +export type ResetOptions = { + entries: Partial<Record<UserSessionKey, boolean>>; +};packages/analytics-js-common/src/types/IRudderAnalytics.ts (2)
79-83: Annotate overloads; mark boolean overload as deprecatedOverloads are sound. Add brief docs so editors surface intended usage and deprecation.
-export type AnalyticsResetMethod = { - (options?: ResetOptions): void; - (resetAnonymousId?: boolean): void; -}; +export type AnalyticsResetMethod = { + /** + * Clear user session information using fine-grained options. + * Omitted keys are left untouched. + */ + (options?: ResetOptions): void; + /** + * @deprecated Use the options-based overload instead. + * Passing a boolean toggles only `entries.anonymousId` (legacy behavior). + */ + (resetAnonymousId?: boolean): void; +};
140-145: Docs Updated & Boolean-Overload Usage AuditI’ve applied the proposed docs tweak to clarify that
reset(true|false)is deprecated and only togglesentries.anonymousId. I also ran the suggested ripgrep search and located all existing boolean-style calls—note that they’re exclusively in test files, not in production code:• packages/analytics-js/tests/browser.test.ts:185
• packages/analytics-js/tests/components/userSessionManager/UserSessionManager.test.ts:1807, 1815
• packages/analytics-js/tests/components/core/Analytics.test.ts:630, 640
• packages/analytics-js/tests/app/RudderAnalytics.test.ts:375, 389You can leave these tests as-is (relying on the deprecated boolean overload) or update them to the new options-based form for consistency and future clarity. Let me know if you’d like help refactoring those test cases!
packages/analytics-js/src/components/core/IAnalytics.ts (1)
170-173: Document boolean backward-compat and default behavior in the JSDoc.Clarify that the first param can be ResetOptions or a boolean (deprecated) and what the default entries are. This reduces API ambiguity for integrators.
Apply this doc tweak:
- /** - * Clear user information - * @param options options for reset - */ + /** + * Clear user information. + * @param options Reset options map or a deprecated boolean flag. + * - By default, the SDK resets: userId, userTraits, groupId, groupTraits, sessionInfo, authToken. + * - By default, the SDK does NOT reset: anonymousId, initialReferrer, initialReferringDomain. + * - Passing a boolean (deprecated): + * - true => also reset anonymousId + * - false/undefined => default behavior above + */packages/analytics-js/src/components/core/Analytics.ts (2)
73-73: Remove unused import: stringifyData.This import is unused and the file disables the no-unused-vars rule globally. Trim the import to keep bundle size leaner and avoid masking future misses.
-import { stringifyData } from '@rudderstack/analytics-js-common/utilities/json';
655-668: Reset flow now correctly accepts options and defers to UserSessionManager.
- Buffering logic correctly stores
[type, options]and remains backward compatible because the implementation acceptsboolean | ResetOptions.- Breadcrumb is updated; consider logging a minimal, sanitized summary of which entries are toggled (optional).
Looks good as-is.
Optional tiny enhancement to breadcrumb without risking PII:
- this.errorHandler.leaveBreadcrumb(`New ${type} invocation`); + this.errorHandler.leaveBreadcrumb( + `New ${type} invocation${options ? ' (options provided)' : ''}`, + );packages/analytics-js/src/app/RudderAnalytics.ts (2)
30-30: Remove unused import: isBoolean.
isBooleanis not used in this module.-import { isBoolean, isString } from '@rudderstack/analytics-js-common/utilities/checks'; +import { isString } from '@rudderstack/analytics-js-common/utilities/checks';
532-569: Great API docs and overloads; add a deprecation warning for boolean usage.Runtime warning will help partners migrate off the boolean overload without reading docs.
- reset(options?: ResetOptions | boolean): void { + reset(options?: ResetOptions | boolean): void { try { - this.getAnalyticsInstance()?.reset(getSanitizedValue(options)); + if (typeof options === 'boolean') { + this.logger.warn( + 'reset(boolean) is deprecated; use reset(options) with entries instead.', + ); + } + this.getAnalyticsInstance()?.reset(getSanitizedValue(options)); } catch (error: any) { dispatchErrorEvent(error); } }packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts (2)
9-9: Remove unused import: isBoolean.Not referenced in this file.
-import { - isBoolean, - isDefinedAndNotNull, - isDefinedNotNullAndNotEmptyString, - isNullOrUndefined, - isString, -} from '@rudderstack/analytics-js-common/utilities/checks'; +import { + isDefinedAndNotNull, + isDefinedNotNullAndNotEmptyString, + isNullOrUndefined, + isString, +} from '@rudderstack/analytics-js-common/utilities/checks';
768-793: Selective reset via getFinalResetOptions looks solid; minor robustness suggestion.Looping over DEFAULT_USER_SESSION_VALUES is fine. Alternatively, iterating over
Object.keys(opts.entries)prevents surprises if either set changes independently. Not a blocker.Alternative iteration:
- Object.keys(DEFAULT_USER_SESSION_VALUES).forEach(key => { + Object.keys(opts.entries).forEach(key => { if (opts.entries[key as UserSessionKey] !== true) { return; } switch (key) { case 'anonymousId': this.setAnonymousId(); break; case 'sessionInfo': this.resetAndStartNewSession(); break; default: session[key as UserSessionKey].value = DEFAULT_USER_SESSION_VALUES[key as UserSessionKey]; break; } });packages/analytics-js/src/components/userSessionManager/constants.ts (1)
6-16: Consider stronger typing for DEFAULT_USER_SESSION_VALUES.Using
Record<UserSessionKey, any>works but weakens compiler checks. If feasible, tighten to a concrete mapped type and mark it readonly to prevent accidental mutation.Example:
type DefaultUserSession = { userId: string; userTraits: ApiObject; anonymousId: string; groupId: string; groupTraits: ApiObject; initialReferrer: string; initialReferringDomain: string; sessionInfo: SessionInfo; authToken: string | null; }; const DEFAULT_USER_SESSION_VALUES: Readonly<DefaultUserSession> = { ... };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
packages/analytics-js-common/src/types/EventApi.ts(2 hunks)packages/analytics-js-common/src/types/IRudderAnalytics.ts(3 hunks)packages/analytics-js/.size-limit.mjs(1 hunks)packages/analytics-js/src/app/RudderAnalytics.ts(2 hunks)packages/analytics-js/src/components/core/Analytics.ts(3 hunks)packages/analytics-js/src/components/core/IAnalytics.ts(2 hunks)packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts(4 hunks)packages/analytics-js/src/components/userSessionManager/constants.ts(2 hunks)packages/analytics-js/src/components/userSessionManager/types.ts(2 hunks)packages/analytics-js/src/components/userSessionManager/utils.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/002-javascript-typescript.mdc)
Code must work in browsers AND service workers (no Node.js APIs)
Files:
packages/analytics-js/src/components/core/IAnalytics.tspackages/analytics-js/src/components/userSessionManager/types.tspackages/analytics-js-common/src/types/EventApi.tspackages/analytics-js/src/components/userSessionManager/utils.tspackages/analytics-js-common/src/types/IRudderAnalytics.tspackages/analytics-js/src/app/RudderAnalytics.tspackages/analytics-js/src/components/core/Analytics.tspackages/analytics-js/src/components/userSessionManager/UserSessionManager.tspackages/analytics-js/src/components/userSessionManager/constants.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1754
File: packages/analytics-js-service-worker/README.md:0-0
Timestamp: 2024-10-08T15:52:59.819Z
Learning: saikumarrs prefers simplified language in documentation, avoiding redundant phrases.
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1754
File: packages/analytics-js-service-worker/README.md:0-0
Timestamp: 2024-06-14T09:50:33.511Z
Learning: saikumarrs prefers simplified language in documentation, avoiding redundant phrases.
📚 Learning: 2024-11-09T06:40:30.520Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1823
File: packages/analytics-js-common/src/types/LoadOptions.ts:0-0
Timestamp: 2024-11-09T06:40:30.520Z
Learning: In the `packages/analytics-js-common/src/types/LoadOptions.ts` file, the `dataplanes` property within the `SourceConfigResponse` type has been removed as it is no longer necessary.
Applied to files:
packages/analytics-js/src/components/core/IAnalytics.tspackages/analytics-js/src/components/userSessionManager/types.tspackages/analytics-js-common/src/types/EventApi.tspackages/analytics-js-common/src/types/IRudderAnalytics.tspackages/analytics-js/src/app/RudderAnalytics.tspackages/analytics-js/src/components/core/Analytics.tspackages/analytics-js/src/components/userSessionManager/constants.ts
📚 Learning: 2024-10-08T15:52:59.819Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1782
File: packages/analytics-js-common/src/utilities/eventMethodOverloads.ts:0-0
Timestamp: 2024-10-08T15:52:59.819Z
Learning: In `packages/analytics-js-common/src/utilities/eventMethodOverloads.ts`, the `delete` operator has been replaced with setting the value to `undefined` for better performance.
Applied to files:
packages/analytics-js/src/components/userSessionManager/types.tspackages/analytics-js-common/src/types/IRudderAnalytics.tspackages/analytics-js/src/app/RudderAnalytics.tspackages/analytics-js/src/components/core/Analytics.ts
📚 Learning: 2024-10-08T15:52:59.819Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1708
File: packages/analytics-js-plugins/__tests__/deviceModeTransformation/index.test.ts:10-11
Timestamp: 2024-10-08T15:52:59.819Z
Learning: The misuse of `IHttpClient` in type assertions within the file `packages/analytics-js-plugins/__tests__/deviceModeTransformation/index.test.ts` has been corrected by the user.
Applied to files:
packages/analytics-js/src/components/userSessionManager/types.ts
📚 Learning: 2024-10-09T06:41:05.073Z
Learnt from: MoumitaM
PR: rudderlabs/rudder-sdk-js#1876
File: packages/analytics-js/src/app/RudderAnalytics.ts:0-0
Timestamp: 2024-10-09T06:41:05.073Z
Learning: The `trackPageLifecycleEvents` method in `packages/analytics-js/src/app/RudderAnalytics.ts` does not require refactoring as the current implementation is preferred.
Applied to files:
packages/analytics-js/src/app/RudderAnalytics.tspackages/analytics-js/src/components/core/Analytics.ts
📚 Learning: 2024-10-28T08:19:43.438Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1902
File: packages/analytics-js/src/components/core/Analytics.ts:125-127
Timestamp: 2024-10-28T08:19:43.438Z
Learning: In `packages/analytics-js/src/components/core/Analytics.ts`, inputs to the `clone` function are sanitized prior, so error handling for clone operations is not needed.
Applied to files:
packages/analytics-js/src/components/core/Analytics.ts
📚 Learning: 2025-05-06T09:03:18.823Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#2209
File: packages/analytics-js/__tests__/components/userSessionManager/utils.test.ts:209-223
Timestamp: 2025-05-06T09:03:18.823Z
Learning: The `generateAutoTrackingSession` function in the RudderStack JS SDK always generates new `id` and `expiresAt` values based on the current timestamp, regardless of any values provided in the input object. It extracts only `timeout` and `cutOff` properties from the input.
Applied to files:
packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts
🧬 Code graph analysis (9)
packages/analytics-js/src/components/core/IAnalytics.ts (1)
packages/analytics-js-common/src/types/EventApi.ts (1)
ResetOptions(49-53)
packages/analytics-js/src/components/userSessionManager/types.ts (1)
packages/analytics-js-common/src/types/EventApi.ts (1)
ResetOptions(49-53)
packages/analytics-js-common/src/types/EventApi.ts (1)
packages/analytics-js-common/src/types/UserSessionStorage.ts (1)
UserSessionKey(1-10)
packages/analytics-js/src/components/userSessionManager/utils.ts (3)
packages/analytics-js-common/src/types/EventApi.ts (1)
ResetOptions(49-53)packages/analytics-js/src/components/userSessionManager/constants.ts (1)
DEFAULT_RESET_OPTIONS(34-34)packages/analytics-js-common/src/utilities/checks.ts (1)
isBoolean(100-100)
packages/analytics-js-common/src/types/IRudderAnalytics.ts (1)
packages/analytics-js-common/src/types/EventApi.ts (1)
ResetOptions(49-53)
packages/analytics-js/src/app/RudderAnalytics.ts (1)
packages/analytics-js-common/src/types/EventApi.ts (1)
ResetOptions(49-53)
packages/analytics-js/src/components/core/Analytics.ts (1)
packages/analytics-js-common/src/types/EventApi.ts (1)
ResetOptions(49-53)
packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts (4)
packages/analytics-js/src/components/userSessionManager/constants.ts (1)
DEFAULT_USER_SESSION_VALUES(34-34)packages/analytics-js-common/src/types/EventApi.ts (1)
ResetOptions(49-53)packages/analytics-js/src/components/userSessionManager/utils.ts (1)
getFinalResetOptions(147-147)packages/analytics-js-common/src/types/UserSessionStorage.ts (1)
UserSessionKey(1-10)
packages/analytics-js/src/components/userSessionManager/constants.ts (2)
packages/analytics-js-common/src/types/UserSessionStorage.ts (1)
UserSessionKey(1-10)packages/analytics-js-common/src/types/EventApi.ts (1)
ResetOptions(49-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Bundle size checks
- GitHub Check: Code quality checks
- GitHub Check: Unit Tests and Lint
🔇 Additional comments (10)
packages/analytics-js/src/components/userSessionManager/types.ts (1)
6-6: Import looks correct and aligns shared typing across packagesImporting ResetOptions from the common types is consistent with the cross-package usage introduced in this PR.
packages/analytics-js-common/src/types/EventApi.ts (1)
4-4: New dependency import is appropriateImporting UserSessionKey to scope ResetOptions keys is correct and limits drift between the options surface and actual session storage keys.
packages/analytics-js/src/components/userSessionManager/utils.ts (2)
14-18: New imports are appropriateUsing ResetOptions, DEFAULT_RESET_OPTIONS, isBoolean, and mergeDeepRight is consistent with the new reset flow.
147-148: Export is correctRe-exports the helper for use by the session manager; consistent with the updated flow.
packages/analytics-js-common/src/types/IRudderAnalytics.ts (1)
2-2: Extended import list is accurateIncluding ResetOptions here is necessary for the overload—looks good.
packages/analytics-js/src/components/core/IAnalytics.ts (1)
9-9: Importing ResetOptions into the core interface is correct and aligns types across layers.This keeps the public interface in sync with common types and prevents drift with downstream callers. No action needed.
packages/analytics-js/src/components/core/Analytics.ts (1)
18-18: Type import of ResetOptions is fine and forward-compatible.Keeps core analytics aligned with the shared EventApi contract. No action needed.
packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts (1)
471-519: Server-side cookie debounce map uses number for timers — OK for browsers and service workers.The use of
globalThis.setTimeoutensures DOM typings and avoids Node.js Timeout types, complying with the “browser + service worker” guideline.packages/analytics-js/src/components/userSessionManager/constants.ts (2)
18-30: DEFAULT_RESET_OPTIONS matches intended semantics (safe-by-default).
- Resets identifiers, traits, sessionInfo, and authToken by default.
- Preserves anonymousId and initial referrer info unless explicitly requested.
This aligns with the public docs and identify-reset flow. LGTM.
34-34: Exporting DEFAULT_RESET_OPTIONS alongside existing constants is a good move.Keeps a single source of truth for defaults.
size-limit report 📦
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2445 +/- ##
============================================
+ Coverage 65.69% 91.40% +25.70%
============================================
Files 483 207 -276
Lines 16673 5982 -10691
Branches 3358 1173 -2185
============================================
- Hits 10954 5468 -5486
+ Misses 4544 488 -4056
+ Partials 1175 26 -1149 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/analytics-js-common/__tests__/utilities/object.test.ts (2)
630-642: Fix Biome “noSparseArray” errors by replacing holes with explicit undefinedSparse array holes trigger Biome’s lint/suspicious/noSparseArray. Replace holes with undefined in both the input and expected objects.
Apply this diff:
- sparseArray: [1, , , 4, undefined, null], + sparseArray: [1, undefined, undefined, 4, undefined, null], @@ - sparseArray: [1, , , 4, undefined, null], + sparseArray: [1, undefined, undefined, 4, undefined, null],Alternative: if you need to preserve the “hole” semantics for a behavior check, add a Biome directive above the literals:
/* biome-ignore lint/suspicious/noSparseArray: testing sparse arrays intentionally */
543-797: Augment coverage: symbol-keyed children and circular graphsGreat breadth of cases. Two high-value additions will harden the contract given how deepFreeze will be used:
- Symbol-keyed children: ensure nested objects under symbol keys are frozen.
- Circular references: ensure we don’t blow the stack and everything remains frozen.
These will pass with the proposed deepFreeze refactor.
Add within this describe block:
it('freezes nested objects under symbol keys', () => { const sym = Symbol('k'); const obj: any = { normal: { x: 1 } }; obj[sym] = { y: 2 }; const frozen = deepFreeze(obj); expect(Object.isFrozen(frozen)).toBe(true); expect(Object.isFrozen(frozen[sym])).toBe(true); expect(() => { frozen[sym].y = 3; }).toThrow(); }); it('handles circular references without recursion errors', () => { const a: any = { name: 'a' }; const b: any = { name: 'b', a }; a.b = b; // a <-> b cycle const frozen = deepFreeze(a); expect(Object.isFrozen(frozen)).toBe(true); expect(Object.isFrozen(frozen.b)).toBe(true); expect(Object.isFrozen(frozen.b.a)).toBe(true); });If you want, I can push these test additions after you confirm target browser constraints.
Would you like me to open a follow-up test PR, or add these to this PR once we settle the deepFreeze implementation?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/analytics-js-common/__tests__/utilities/object.test.ts(2 hunks)packages/analytics-js-common/src/utilities/object.ts(2 hunks)packages/analytics-js/.size-limit.mjs(2 hunks)packages/analytics-js/src/components/userSessionManager/constants.ts(2 hunks)packages/analytics-js/src/components/userSessionManager/utils.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/analytics-js/src/components/userSessionManager/utils.ts
- packages/analytics-js/.size-limit.mjs
- packages/analytics-js/src/components/userSessionManager/constants.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/002-javascript-typescript.mdc)
Code must work in browsers AND service workers (no Node.js APIs)
Files:
packages/analytics-js-common/src/utilities/object.tspackages/analytics-js-common/__tests__/utilities/object.test.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1754
File: packages/analytics-js-service-worker/README.md:0-0
Timestamp: 2024-10-08T15:52:59.819Z
Learning: saikumarrs prefers simplified language in documentation, avoiding redundant phrases.
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1754
File: packages/analytics-js-service-worker/README.md:0-0
Timestamp: 2024-06-14T09:50:33.511Z
Learning: saikumarrs prefers simplified language in documentation, avoiding redundant phrases.
📚 Learning: 2024-10-28T08:09:31.840Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1902
File: packages/analytics-js-common/src/utilities/object.ts:12-13
Timestamp: 2024-10-28T08:09:31.840Z
Learning: In `packages/analytics-js-common/src/utilities/object.ts`, the `isObject` function is intended to check if a value is of type 'object', including when the value is `null`. Null checking is handled separately in functions like `isObjectAndNotNull`.
Applied to files:
packages/analytics-js-common/__tests__/utilities/object.test.ts
📚 Learning: 2024-11-08T13:17:51.356Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1823
File: packages/analytics-js-common/src/utilities/retryQueue/utilities.ts:0-0
Timestamp: 2024-11-08T13:17:51.356Z
Learning: The issue regarding missing test coverage for the `findOtherQueues` function in `packages/analytics-js-common/src/utilities/retryQueue/utilities.ts` is no longer applicable.
Applied to files:
packages/analytics-js-common/__tests__/utilities/object.test.ts
🪛 Biome (2.1.2)
packages/analytics-js-common/__tests__/utilities/object.test.ts
[error] 631-631: This array contains an empty slots..
The presences of empty slots may cause incorrect information and might be a typo.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
[error] 637-637: This array contains an empty slots..
The presences of empty slots may cause incorrect information and might be a typo.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Code quality checks
- GitHub Check: Bundle size checks
- GitHub Check: Unit Tests and Lint
🔇 Additional comments (3)
packages/analytics-js-common/src/utilities/object.ts (2)
163-164: Export looks goodNamed export aligns with module style and keeps the utility tree-shakeable.
141-149: deepFreeze: make cycle-safe, include Symbol keys, skip accessors & guard primitivesBrief: current impl recurses into objects (infinite on cyclic graphs), ignores symbol-keyed properties, and reads properties (may trigger getters). Repo does include a polyfill loader that can add Symbol at runtime (e.g. packages/loading-scripts, analytics-js polyfill loader, public examples), so symbol traversal is appropriate when Symbols exist — please confirm IE11/consumer polyfill policy.
Files to update
- packages/analytics-js-common/src/utilities/object.ts — replace deepFreeze implementation around lines ~141-149.
Suggested replacement (descriptor-safe variant — avoids invoking getters):
-const deepFreeze = <T>(obj: T): T => { - Object.getOwnPropertyNames(obj).forEach(function (prop) { - if (obj[prop as keyof T] && typeof obj[prop as keyof T] === 'object') { - deepFreeze(obj[prop as keyof T]); - } - }); - return Object.freeze(obj); -}; +const deepFreeze = <T>(obj: T): T => { + // Bail out for null/undefined and primitives + if (obj === null || obj === undefined || (typeof obj !== 'object' && typeof obj !== 'function')) { + return obj; + } + + // Freeze self first to short-circuit cycles. + Object.freeze(obj as any); + + // Collect own string keys; include symbols if supported. + const keys: (string | symbol)[] = Object.getOwnPropertyNames(obj as object) as (string | symbol)[]; + if (typeof Object.getOwnPropertySymbols === 'function') { + keys.push(...Object.getOwnPropertySymbols(obj as object)); + } + + for (const key of keys) { + try { + const desc = Object.getOwnPropertyDescriptor(obj as object, key as PropertyKey); + // Skip accessors (getters/setters) to avoid side effects. + if (!desc || !('value' in desc)) continue; + const value = desc.value; + if (value && (typeof value === 'object' || typeof value === 'function') && !Object.isFrozen(value)) { + deepFreeze(value); + } + } catch { + // Defensive: some host objects may throw; ignore and continue. + } + } + + return obj; +};Notes / next steps
- Confirm whether IE11 support must be preserved without polyfills. If IE11 must run without external polyfills, Symbols won't exist and the symbol-path is a no-op; if you rely on runtime polyfills (polyfill-fastly or core-js), the symbol traversal will work.
- If you prefer to also recurse into accessor-getter results (knowing it may invoke getters), remove the descriptor check and read property values directly — but that can cause side effects.
- If you want strongest protection against cycles in all environments, we can instead add an optional WeakSet visited param; I can provide that variant on request.
packages/analytics-js-common/__tests__/utilities/object.test.ts (1)
13-14: Import is correctImporting deepFreeze from the common utilities is consistent with the public API surface.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts (1)
733-756: Reset session: explicit no-op when neither autoTrack nor manualTrack is enabled (FYI).This mirrors prior discussion: resetAndStartNewSession intentionally does nothing if both tracking modes are disabled, relying on initialization to normalize sessionInfo. If you keep this choice, consider a short inline comment to signal intent to future readers.
Apply this diff to document the decision:
resetAndStartNewSession() { const session = state.session; const { manualTrack, autoTrack, timeout, cutOff } = session.sessionInfo.value; if (autoTrack) { @@ this.startOrRenewAutoTracking(session.sessionInfo.value); } else if (manualTrack) { this.startManualTrackingInternal(); + } else { + // Intentionally no-op: when both autoTrack and manualTrack are disabled, + // we do not mutate sessionInfo here. Initialization already normalizes + // the shape, and reset should not implicitly enable tracking. } }
🧹 Nitpick comments (5)
packages/analytics-js/__tests__/components/userSessionManager/utils.test.ts (1)
406-536: Great coverage for getFinalResetOptions; drop duplicate case and add a “top-level key preservation” check.
- There’s a duplicate testCase for the same input/expected pair (entries.userId: false) at Lines 451–463; it repeats the immediately preceding case. Removing it will keep the suite lean.
- Consider adding one test that verifies unknown top-level keys are preserved (not only unknown keys under entries). That documents the intended deep-merge behavior explicitly.
Apply this diff to remove the duplicate entry:
@@ { input: { entries: { userId: false, }, }, expected: { ...defaultOptions, entries: { ...defaultOptions.entries, userId: false, }, }, }, - { - input: { - entries: { - userId: false, - }, - }, - expected: { - ...defaultOptions, - entries: { - ...defaultOptions.entries, - userId: false, - }, - }, - },And add one more case to assert top-level key preservation:
@@ { input: { entries: { userId: false, }, + extra: { foo: 'bar' }, }, expected: { ...defaultOptions, entries: { ...defaultOptions.entries, userId: false, }, + extra: { foo: 'bar' }, }, },packages/analytics-js/__tests__/app/RudderAnalytics.test.ts (1)
398-419: Add one more negative-path check: boolean false should be forwarded unchanged.This complements the true/undefined cases you already cover and guards the legacy overload path.
Apply this diff near the other reset forwarding tests:
@@ it('should process reset arguments and forwards to reset call', () => { rudderAnalytics.reset(true); expect(analyticsInstanceMock.reset).toHaveBeenCalledWith(true); }); + it('should process reset arguments and forwards to reset call with boolean false (no-op legacy)', () => { + rudderAnalytics.reset(false); + expect(analyticsInstanceMock.reset).toHaveBeenCalledWith(false); + });packages/analytics-js/__tests__/components/core/Analytics.test.ts (1)
625-639: Optionally add a buffered boolean case to protect legacy overload.Right now we validate object-shaped buffering; adding a boolean-shaped buffered case will prevent regressions for older callers.
Add alongside the existing buffered test:
@@ it('should buffer events until loaded', () => { analytics.prepareInternalServices(); const resetSpy = jest.spyOn(analytics.userSessionManager!, 'reset'); analytics.reset({ entries: { anonymousId: true, }, }); expect(resetSpy).toHaveBeenCalledTimes(0); expect(state.eventBuffer.toBeProcessedArray.value).toStrictEqual([ ['reset', { entries: { anonymousId: true } }], ]); }); + + it('should buffer legacy boolean reset calls until loaded', () => { + analytics.prepareInternalServices(); + const resetSpy = jest.spyOn(analytics.userSessionManager!, 'reset'); + analytics.reset(true); + expect(resetSpy).toHaveBeenCalledTimes(0); + expect(state.eventBuffer.toBeProcessedArray.value).toStrictEqual([['reset', true]]); + });packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts (1)
763-787: Per-key reset logic is solid; minor readability tweak could help.Current flow iterates all default keys and early-returns when entries[key] !== true. Filtering first makes the “only reset requested keys” intent clearer and avoids nested control flow.
Refactor loop like this:
- batch(() => { - Object.keys(DEFAULT_USER_SESSION_VALUES).forEach(key => { - if (opts.entries[key as UserSessionKey] !== true) { - return; - } - - switch (key) { + batch(() => { + (Object.keys(DEFAULT_USER_SESSION_VALUES) as UserSessionKey[]) + .filter(key => opts.entries[key] === true) + .forEach(key => { + switch (key) { case 'anonymousId': this.setAnonymousId(); break; case 'sessionInfo': this.resetAndStartNewSession(); break; default: session[key as UserSessionKey].value = DEFAULT_USER_SESSION_VALUES[key as UserSessionKey]; break; - } - }); + } + }); });Also nice: by checking opts.entries[key] === true, unknown or non-boolean entries are naturally ignored, matching your tests.
packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts (1)
1775-1783: Reset API tests are thorough and map 1:1 to the new semantics.
- Good use of fake timers to stabilize time-dependent assertions.
- The “default reset” vs “options-driven reset” vs “legacy boolean reset” paths are all validated, including storage type edge cases (anonymousId: none).
- The sessionInfo invariants (preserve timeout/cutOff/manual vs regenerate id/expiresAt) are clearly asserted.
One more small addition could future-proof behavior:
- Add a case where options.entries includes unknown keys or non-boolean values and ensure reset ignores them (it should, by design). This validates safety against malformed inputs at the integration level (you already unit-test getFinalResetOptions for this).
Example to add near the other reset specs:
it('should ignore unknown/non-boolean entries in options during reset', () => { state.storage.entries.value = entriesWithOnlyCookieStorage; userSessionManager.init(); userSessionManager.setUserId('u1'); userSessionManager.reset({ // @ts-expect-error intentional malformed values entries: { userId: 'nope', abc: true, sessionInfo: {} as any }, }); // userId should not be reset because 'nope' !== true expect(state.session.userId.value).toBe('u1'); // unknown keys should be ignored and not throw expect(state.session.sessionInfo.value).toBeDefined(); });Also applies to: 1785-1852, 1859-1863, 1865-1877, 1884-1891, 1893-1913, 1915-1985, 1987-2051, 2053-2076
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
packages/analytics-js/.size-limit.mjs(3 hunks)packages/analytics-js/__tests__/app/RudderAnalytics.test.ts(2 hunks)packages/analytics-js/__tests__/components/core/Analytics.test.ts(1 hunks)packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts(3 hunks)packages/analytics-js/__tests__/components/userSessionManager/utils.test.ts(2 hunks)packages/analytics-js/src/app/RudderAnalytics.ts(2 hunks)packages/analytics-js/src/components/core/Analytics.ts(2 hunks)packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts(3 hunks)packages/analytics-js/src/components/userSessionManager/utils.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/analytics-js/src/components/core/Analytics.ts
- packages/analytics-js/src/app/RudderAnalytics.ts
- packages/analytics-js/.size-limit.mjs
- packages/analytics-js/src/components/userSessionManager/utils.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/002-javascript-typescript.mdc)
Code must work in browsers AND service workers (no Node.js APIs)
Files:
packages/analytics-js/__tests__/components/userSessionManager/utils.test.tspackages/analytics-js/__tests__/app/RudderAnalytics.test.tspackages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.tspackages/analytics-js/__tests__/components/core/Analytics.test.tspackages/analytics-js/src/components/userSessionManager/UserSessionManager.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1754
File: packages/analytics-js-service-worker/README.md:0-0
Timestamp: 2024-06-14T09:50:33.511Z
Learning: saikumarrs prefers simplified language in documentation, avoiding redundant phrases.
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1754
File: packages/analytics-js-service-worker/README.md:0-0
Timestamp: 2024-10-08T15:52:59.819Z
Learning: saikumarrs prefers simplified language in documentation, avoiding redundant phrases.
📚 Learning: 2024-10-08T15:52:59.819Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1708
File: packages/analytics-js-plugins/__tests__/deviceModeTransformation/index.test.ts:10-11
Timestamp: 2024-10-08T15:52:59.819Z
Learning: The misuse of `IHttpClient` in type assertions within the file `packages/analytics-js-plugins/__tests__/deviceModeTransformation/index.test.ts` has been corrected by the user.
Applied to files:
packages/analytics-js/__tests__/components/userSessionManager/utils.test.tspackages/analytics-js/__tests__/app/RudderAnalytics.test.tspackages/analytics-js/__tests__/components/core/Analytics.test.ts
📚 Learning: 2024-11-08T13:17:51.356Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1823
File: packages/analytics-js-common/src/utilities/retryQueue/utilities.ts:0-0
Timestamp: 2024-11-08T13:17:51.356Z
Learning: The issue regarding missing test coverage for the `findOtherQueues` function in `packages/analytics-js-common/src/utilities/retryQueue/utilities.ts` is no longer applicable.
Applied to files:
packages/analytics-js/__tests__/components/userSessionManager/utils.test.tspackages/analytics-js/__tests__/components/core/Analytics.test.ts
📚 Learning: 2024-11-09T05:04:51.002Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1823
File: packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts:1362-1362
Timestamp: 2024-11-09T05:04:51.002Z
Learning: In tests for the UserSessionManager, it's acceptable to spy on private methods like `private_syncValueToStorage` when necessary.
Applied to files:
packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts
📚 Learning: 2024-10-28T08:19:43.438Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1902
File: packages/analytics-js/src/components/core/Analytics.ts:125-127
Timestamp: 2024-10-28T08:19:43.438Z
Learning: In `packages/analytics-js/src/components/core/Analytics.ts`, inputs to the `clone` function are sanitized prior, so error handling for clone operations is not needed.
Applied to files:
packages/analytics-js/__tests__/components/core/Analytics.test.ts
📚 Learning: 2024-10-08T15:52:59.819Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1782
File: packages/analytics-js-common/src/utilities/eventMethodOverloads.ts:0-0
Timestamp: 2024-10-08T15:52:59.819Z
Learning: In `packages/analytics-js-common/src/utilities/eventMethodOverloads.ts`, the `delete` operator has been replaced with setting the value to `undefined` for better performance.
Applied to files:
packages/analytics-js/__tests__/components/core/Analytics.test.ts
📚 Learning: 2025-05-06T09:03:18.823Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#2209
File: packages/analytics-js/__tests__/components/userSessionManager/utils.test.ts:209-223
Timestamp: 2025-05-06T09:03:18.823Z
Learning: The `generateAutoTrackingSession` function in the RudderStack JS SDK always generates new `id` and `expiresAt` values based on the current timestamp, regardless of any values provided in the input object. It extracts only `timeout` and `cutOff` properties from the input.
Applied to files:
packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts
📚 Learning: 2024-10-09T06:41:05.073Z
Learnt from: MoumitaM
PR: rudderlabs/rudder-sdk-js#1876
File: packages/analytics-js/src/app/RudderAnalytics.ts:0-0
Timestamp: 2024-10-09T06:41:05.073Z
Learning: The `trackPageLifecycleEvents` method in `packages/analytics-js/src/app/RudderAnalytics.ts` does not require refactoring as the current implementation is preferred.
Applied to files:
packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts
🧬 Code graph analysis (2)
packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts (2)
packages/analytics-js/src/state/index.ts (1)
state(62-62)packages/analytics-js/__fixtures__/fixtures.ts (2)
entriesWithOnlyCookieStorage(744-744)anonymousIdWithNoStorageEntries(753-753)
packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts (5)
packages/analytics-js/src/state/index.ts (1)
state(62-62)packages/analytics-js/src/components/userSessionManager/constants.ts (1)
DEFAULT_USER_SESSION_VALUES(36-36)packages/analytics-js-common/src/types/EventApi.ts (1)
ResetOptions(49-53)packages/analytics-js/src/components/userSessionManager/utils.ts (1)
getFinalResetOptions(158-158)packages/analytics-js-common/src/types/UserSessionStorage.ts (1)
UserSessionKey(1-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Code quality checks
- GitHub Check: Bundle size checks
- GitHub Check: Unit Tests and Lint
🔇 Additional comments (5)
packages/analytics-js/__tests__/components/userSessionManager/utils.test.ts (1)
12-13: Import looks good and aligns tests with new API surface.Brings getFinalResetOptions under test; no concerns.
packages/analytics-js/__tests__/app/RudderAnalytics.test.ts (2)
379-391: Forwarding ResetOptions through RudderAnalytics.reset is verified correctly.The test ensures the options object is passed through as-is to analytics.reset; good guard for the facade.
393-397: No-arg reset forwarding behavior is captured.Asserting undefined is forwarded helps prevent accidental boolean fallbacks. Looks good.
packages/analytics-js/__tests__/components/core/Analytics.test.ts (1)
630-639: Buffered ResetOptions path is validated.Good assertion that the buffered payload is ['reset', { entries: { anonymousId: true } }].
packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts (1)
66-67: Imports and type-safety upgrades align with the new API.Bringing getFinalResetOptions and ResetOptions into the manager is the right separation of concerns.
Also applies to: 82-83
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new reset API with granular options to the analytics SDK, allowing users to selectively reset specific user session fields rather than having a simple boolean toggle for anonymous ID reset. The new API maintains backward compatibility while providing more flexibility for resetting user data.
- Added options-based reset API that accepts a ResetOptions object with configurable entries
- Introduced deepFreeze utility for creating immutable configuration objects
- Updated default reset behavior to reset most fields except anonymousId, initialReferrer, and initialReferringDomain
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/analytics-js/src/components/userSessionManager/utils.ts | Added getFinalResetOptions function to handle both legacy boolean and new options-based reset parameters |
| packages/analytics-js/src/components/userSessionManager/constants.ts | Added DEFAULT_RESET_OPTIONS configuration and applied deepFreeze to make constants immutable |
| packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts | Refactored reset method to use new options-based approach with granular field control |
| packages/analytics-js/src/app/RudderAnalytics.ts | Added method overloads and documentation for the new reset API while maintaining backward compatibility |
| packages/analytics-js-common/src/utilities/object.ts | Implemented deepFreeze utility for creating deeply immutable objects |
| packages/analytics-js-common/src/types/EventApi.ts | Added ResetOptions type definition for the new API |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/analytics-js/rollup.config.mjs (2)
38-41: Harden env fallbacks and normalize slashes deterministicallyUsing nullish coalescing keeps empty-string envs (e.g., REMOTE_MODULES_BASE_PATH="") instead of falling back, which then collapses to "/" after the trailing-slash append. Treat empty/whitespace-only as unset and normalize with a single replace to avoid edge cases.
Apply this diff:
-let remotePluginsBasePath = - process.env.REMOTE_MODULES_BASE_PATH ?? `${baseCdnUrl}/v3`; -remotePluginsBasePath = remotePluginsBasePath?.endsWith('/') ? remotePluginsBasePath : `${remotePluginsBasePath}/`; -let destSDKBaseURL = process.env.DEST_SDK_BASE_URL ?? `${baseCdnUrl}/v3`; -destSDKBaseURL = destSDKBaseURL?.endsWith('/') ? destSDKBaseURL : `${destSDKBaseURL}/`; +let remotePluginsBasePath = + ((process.env.REMOTE_MODULES_BASE_PATH || '').trim() || `${baseCdnUrl}/v3`); +remotePluginsBasePath = remotePluginsBasePath.replace(/\/+$/, '') + '/'; +let destSDKBaseURL = + ((process.env.DEST_SDK_BASE_URL || '').trim() || `${baseCdnUrl}/v3`); +destSDKBaseURL = destSDKBaseURL.replace(/\/+$/, '') + '/';
49-49: Avoid “//” in remote plugins URL and sanitize runtime override
remotePluginsBasePathalready ends with “/”, and the template adds another “/”, yielding.../v3//rsa-plugins.js. Also, sanitize the runtime override to avoid double slashes ifpluginsCDNPathends with “/”.Apply this diff:
-const remotePluginsHostPromise = `Promise.resolve(window.RudderStackGlobals && window.RudderStackGlobals.app && window.RudderStackGlobals.app.pluginsCDNPath ? \`\${window.RudderStackGlobals.app.pluginsCDNPath}/${remotePluginsExportsFilename}.js\` : \`${remotePluginsBasePath}/${remotePluginsExportsFilename}.js\`)`; +const remotePluginsHostPromise = `Promise.resolve( + window.RudderStackGlobals && window.RudderStackGlobals.app && window.RudderStackGlobals.app.pluginsCDNPath + ? \`\${window.RudderStackGlobals.app.pluginsCDNPath.replace(/\/+$/, '')}/${remotePluginsExportsFilename}.js\` + : \`${remotePluginsBasePath}${remotePluginsExportsFilename}.js\` +)`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/analytics-js/public/index.html(1 hunks)packages/analytics-js/rollup.config.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1754
File: packages/analytics-js-service-worker/README.md:0-0
Timestamp: 2024-06-14T09:50:33.511Z
Learning: saikumarrs prefers simplified language in documentation, avoiding redundant phrases.
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1754
File: packages/analytics-js-service-worker/README.md:0-0
Timestamp: 2024-10-08T15:52:59.819Z
Learning: saikumarrs prefers simplified language in documentation, avoiding redundant phrases.
📚 Learning: 2025-07-10T18:03:59.589Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#2330
File: packages/sanity-suite/public/v3/integrations/index-cdn.html:47-50
Timestamp: 2025-07-10T18:03:59.589Z
Learning: In the RudderStack SDK JavaScript project, when replacing hardcoded CDN URLs with configurable ones, trailing slash issues are handled at the build configuration level in rollup configs by trimming trailing slashes from the BASE_CDN_URL environment variable before injecting it as __BASE_CDN_URL__ placeholder, rather than adding runtime sanitization in individual files.
Applied to files:
packages/analytics-js/rollup.config.mjspackages/analytics-js/public/index.html
📚 Learning: 2024-10-08T15:52:59.819Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1730
File: packages/analytics-js/src/components/utilities/url.ts:10-10
Timestamp: 2024-10-08T15:52:59.819Z
Learning: The `removeTrailingSlashes` function in `packages/analytics-js/src/components/utilities/url.ts` now uses optional chaining for better safety and readability.
Applied to files:
packages/analytics-js/rollup.config.mjspackages/analytics-js/public/index.html
📚 Learning: 2024-11-09T06:40:30.520Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1823
File: packages/analytics-js-common/src/types/LoadOptions.ts:0-0
Timestamp: 2024-11-09T06:40:30.520Z
Learning: In the `packages/analytics-js-common/src/types/LoadOptions.ts` file, the `dataplanes` property within the `SourceConfigResponse` type has been removed as it is no longer necessary.
Applied to files:
packages/analytics-js/public/index.html
📚 Learning: 2024-07-27T07:02:57.329Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1708
File: packages/analytics-js/__tests__/nativeSdkLoader.js:31-33
Timestamp: 2024-07-27T07:02:57.329Z
Learning: The loading snippet in `packages/analytics-js/__tests__/nativeSdkLoader.js` is a standard part of the SDK, and no changes are desired on it.
Applied to files:
packages/analytics-js/public/index.html
📚 Learning: 2024-10-28T08:03:12.163Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1902
File: packages/analytics-js-plugins/src/utilities/eventsDelivery.ts:0-0
Timestamp: 2024-10-28T08:03:12.163Z
Learning: In `packages/analytics-js-plugins/src/utilities/eventsDelivery.ts`, the issue regarding inconsistent error handling approaches in `getDMTDeliveryPayload` is no longer valid.
Applied to files:
packages/analytics-js/public/index.html
📚 Learning: 2024-07-27T07:02:57.329Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1740
File: packages/analytics-js-common/src/utilities/url.ts:10-10
Timestamp: 2024-07-27T07:02:57.329Z
Learning: The `isValidURL` function in `packages/analytics-js-common/src/utilities/url.ts` uses `string | undefined` instead of `any` for the `url` parameter to enhance type safety.
Applied to files:
packages/analytics-js/public/index.html
📚 Learning: 2024-10-08T15:52:59.819Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1708
File: packages/analytics-js-plugins/__tests__/deviceModeTransformation/index.test.ts:10-11
Timestamp: 2024-10-08T15:52:59.819Z
Learning: The misuse of `IHttpClient` in type assertions within the file `packages/analytics-js-plugins/__tests__/deviceModeTransformation/index.test.ts` has been corrected by the user.
Applied to files:
packages/analytics-js/public/index.html
📚 Learning: 2024-10-28T08:02:55.044Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1902
File: packages/analytics-js-common/src/utilities/sanitize.ts:0-0
Timestamp: 2024-10-28T08:02:55.044Z
Learning: The previous suggestions on the `getSanitizedValue` function in `packages/analytics-js-common/src/utilities/sanitize.ts` are no longer valid.
Applied to files:
packages/analytics-js/public/index.html
📚 Learning: 2024-10-08T15:52:59.819Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1782
File: packages/analytics-js-common/src/utilities/eventMethodOverloads.ts:0-0
Timestamp: 2024-10-08T15:52:59.819Z
Learning: In `packages/analytics-js-common/src/utilities/eventMethodOverloads.ts`, the `delete` operator has been replaced with setting the value to `undefined` for better performance.
Applied to files:
packages/analytics-js/public/index.html
📚 Learning: 2024-08-26T09:34:28.341Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1823
File: examples/integrations/Ninetailed/sample-apps/app-using-v1.1-cdn/src/App.js:18-18
Timestamp: 2024-08-26T09:34:28.341Z
Learning: When reviewing the `examples/integrations/Ninetailed/sample-apps/app-using-v1.1-cdn/src/App.js` file, ignore the assignment pattern `let e = (window.rudderanalytics = window.rudderanalytics || []);` as it is a standard snippet.
Applied to files:
packages/analytics-js/public/index.html
🧬 Code graph analysis (1)
packages/analytics-js/rollup.config.mjs (2)
packages/sanity-suite/rollup.config.mjs (1)
baseCdnUrl(20-20)packages/analytics-v1.1/rollup-configs/rollup.utilities.mjs (1)
baseCdnUrl(22-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Code quality checks
- GitHub Check: Bundle size checks
- GitHub Check: Unit Tests and Lint
🔇 Additional comments (2)
packages/analytics-js/public/index.html (2)
179-181: Good guard against accidental 'undefined/' in env-derived dest SDK base URLThis prevents injecting a broken base path when the placeholder expands to 'undefined/'. This aligns with the rollup defaults that now ensure a concrete
${baseCdnUrl}/v3/base.
185-187: Consistent protection for plugins base URLSame positive note here—blocks 'undefined/' from slipping into
loadOptions.pluginsSDKBaseURL. Keeps the sample resilient during local/dev builds.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/analytics-js/rollup.config.mjs (1)
38-41: Optional: extract a tiny helper for trailing-slash normalization.You normalize both
remotePluginsBasePathanddestSDKBaseURL. A small inline util (e.g.,ensureTrailingSlash) would reduce duplication and future drift.-remotePluginsBasePath = remotePluginsBasePath?.endsWith('/') ? remotePluginsBasePath : `${remotePluginsBasePath}/`; -let destSDKBaseURL = process.env.DEST_SDK_BASE_URL ?? `${baseCdnUrl}/v3`; -destSDKBaseURL = destSDKBaseURL?.endsWith('/') ? destSDKBaseURL : `${destSDKBaseURL}/`; +const ensureTrailingSlash = (s) => (s?.endsWith('/') ? s : `${s}/`); +remotePluginsBasePath = ensureTrailingSlash(process.env.REMOTE_MODULES_BASE_PATH ?? `${baseCdnUrl}/v3`); +let destSDKBaseURL = ensureTrailingSlash(process.env.DEST_SDK_BASE_URL ?? `${baseCdnUrl}/v3`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/analytics-js/public/index.html(1 hunks)packages/analytics-js/rollup.config.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1754
File: packages/analytics-js-service-worker/README.md:0-0
Timestamp: 2024-06-14T09:50:33.511Z
Learning: saikumarrs prefers simplified language in documentation, avoiding redundant phrases.
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1754
File: packages/analytics-js-service-worker/README.md:0-0
Timestamp: 2024-10-08T15:52:59.819Z
Learning: saikumarrs prefers simplified language in documentation, avoiding redundant phrases.
📚 Learning: 2024-07-27T07:02:57.329Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1708
File: packages/analytics-js/__tests__/nativeSdkLoader.js:31-33
Timestamp: 2024-07-27T07:02:57.329Z
Learning: The loading snippet in `packages/analytics-js/__tests__/nativeSdkLoader.js` is a standard part of the SDK, and no changes are desired on it.
Applied to files:
packages/analytics-js/public/index.html
📚 Learning: 2024-11-09T06:40:30.520Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1823
File: packages/analytics-js-common/src/types/LoadOptions.ts:0-0
Timestamp: 2024-11-09T06:40:30.520Z
Learning: In the `packages/analytics-js-common/src/types/LoadOptions.ts` file, the `dataplanes` property within the `SourceConfigResponse` type has been removed as it is no longer necessary.
Applied to files:
packages/analytics-js/public/index.html
📚 Learning: 2024-10-08T15:52:59.819Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1782
File: packages/analytics-js-common/src/utilities/eventMethodOverloads.ts:0-0
Timestamp: 2024-10-08T15:52:59.819Z
Learning: In `packages/analytics-js-common/src/utilities/eventMethodOverloads.ts`, the `delete` operator has been replaced with setting the value to `undefined` for better performance.
Applied to files:
packages/analytics-js/public/index.html
📚 Learning: 2024-10-28T08:03:12.163Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1902
File: packages/analytics-js-plugins/src/utilities/eventsDelivery.ts:0-0
Timestamp: 2024-10-28T08:03:12.163Z
Learning: In `packages/analytics-js-plugins/src/utilities/eventsDelivery.ts`, the issue regarding inconsistent error handling approaches in `getDMTDeliveryPayload` is no longer valid.
Applied to files:
packages/analytics-js/public/index.html
📚 Learning: 2024-10-08T15:52:59.819Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1708
File: packages/analytics-js-plugins/__tests__/deviceModeTransformation/index.test.ts:10-11
Timestamp: 2024-10-08T15:52:59.819Z
Learning: The misuse of `IHttpClient` in type assertions within the file `packages/analytics-js-plugins/__tests__/deviceModeTransformation/index.test.ts` has been corrected by the user.
Applied to files:
packages/analytics-js/public/index.html
📚 Learning: 2024-10-28T08:02:55.044Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1902
File: packages/analytics-js-common/src/utilities/sanitize.ts:0-0
Timestamp: 2024-10-28T08:02:55.044Z
Learning: The previous suggestions on the `getSanitizedValue` function in `packages/analytics-js-common/src/utilities/sanitize.ts` are no longer valid.
Applied to files:
packages/analytics-js/public/index.html
📚 Learning: 2025-07-31T05:21:08.210Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#2417
File: packages/analytics-js/public/index.html:192-203
Timestamp: 2025-07-31T05:21:08.210Z
Learning: The file `packages/analytics-js/public/index.html` is a test/demo HTML page, and PII logging concerns in this file are not applicable since it's used only for testing purposes.
Applied to files:
packages/analytics-js/public/index.html
📚 Learning: 2024-07-27T07:02:57.329Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1740
File: packages/analytics-js-common/src/utilities/url.ts:10-10
Timestamp: 2024-07-27T07:02:57.329Z
Learning: The `isValidURL` function in `packages/analytics-js-common/src/utilities/url.ts` uses `string | undefined` instead of `any` for the `url` parameter to enhance type safety.
Applied to files:
packages/analytics-js/public/index.html
📚 Learning: 2025-07-10T18:03:59.589Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#2330
File: packages/sanity-suite/public/v3/integrations/index-cdn.html:47-50
Timestamp: 2025-07-10T18:03:59.589Z
Learning: In the RudderStack SDK JavaScript project, when replacing hardcoded CDN URLs with configurable ones, trailing slash issues are handled at the build configuration level in rollup configs by trimming trailing slashes from the BASE_CDN_URL environment variable before injecting it as __BASE_CDN_URL__ placeholder, rather than adding runtime sanitization in individual files.
Applied to files:
packages/analytics-js/public/index.htmlpackages/analytics-js/rollup.config.mjs
📚 Learning: 2024-08-26T09:34:28.341Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1823
File: examples/integrations/Ninetailed/sample-apps/app-using-v1.1-cdn/src/App.js:18-18
Timestamp: 2024-08-26T09:34:28.341Z
Learning: When reviewing the `examples/integrations/Ninetailed/sample-apps/app-using-v1.1-cdn/src/App.js` file, ignore the assignment pattern `let e = (window.rudderanalytics = window.rudderanalytics || []);` as it is a standard snippet.
Applied to files:
packages/analytics-js/public/index.html
📚 Learning: 2024-10-08T15:52:59.819Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1730
File: packages/analytics-js/src/components/utilities/url.ts:10-10
Timestamp: 2024-10-08T15:52:59.819Z
Learning: The `removeTrailingSlashes` function in `packages/analytics-js/src/components/utilities/url.ts` now uses optional chaining for better safety and readability.
Applied to files:
packages/analytics-js/rollup.config.mjs
🧬 Code graph analysis (1)
packages/analytics-js/rollup.config.mjs (2)
packages/sanity-suite/rollup.config.mjs (1)
baseCdnUrl(20-20)packages/analytics-v1.1/rollup-configs/rollup.utilities.mjs (1)
baseCdnUrl(22-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Code quality checks
- GitHub Check: Bundle size checks
- GitHub Check: Unit Tests and Lint
🔇 Additional comments (3)
packages/analytics-js/public/index.html (2)
176-181: Env guard updated to 'undefined/': aligns with trailing-slash normalization.Given rollup now normalizes base paths with a trailing slash, checking against the 'undefined/' sentinel is reasonable and prevents accidental opt-in when an env is literally the string "undefined". No action needed.
183-187: Same guard for plugins base URL looks good.Mirrors the dest base URL logic and avoids the 'undefined/' sentinel case after injection. Consistent with the build-time changes.
packages/analytics-js/rollup.config.mjs (1)
40-41: DEST_SDK default via nullish coalescing: looks correct.Defaulting to
${baseCdnUrl}/v3and then normalizing with a trailing slash matches how the demo page constructs.../<buildType>/js-integrations. No issues spotted.
saikumarrs
force-pushed
the
feature/sdk-3362-js-implementation-of-overload-reset-method
branch
from
b51b4c1 to
4667437
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
packages/analytics-js/rollup.config.mjs (1)
49-49: Micro: drop the extra ‘/’ to avoid//in the fallback URL.
remotePluginsBasePathalready ends with/.Apply:
-const remotePluginsHostPromise = `Promise.resolve(window.RudderStackGlobals && window.RudderStackGlobals.app && window.RudderStackGlobals.app.pluginsCDNPath ? \`\${window.RudderStackGlobals.app.pluginsCDNPath}/${remotePluginsExportsFilename}.js\` : \`\${remotePluginsBasePath}/${remotePluginsExportsFilename}.js\`)`; +const remotePluginsHostPromise = `Promise.resolve(window.RudderStackGlobals && window.RudderStackGlobals.app && window.RudderStackGlobals.app.pluginsCDNPath ? \`\${window.RudderStackGlobals.app.pluginsCDNPath}/${remotePluginsExportsFilename}.js\` : \`\${remotePluginsBasePath}${remotePluginsExportsFilename}.js\`)`;packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts (1)
733-756: Reset when neither autoTrack nor manualTrack is enabled — confirm intended no-op.If both flags are off, resetAndStartNewSession currently does nothing. If the intent of resetting sessionInfo is to clear to defaults, consider setting session.sessionInfo.value to DEFAULT_USER_SESSION_VALUES.sessionInfo in this branch.
Optionally:
} else if (manualTrack) { this.startManualTrackingInternal(); + } else { + // Neither auto nor manual tracking is enabled; clear to defaults. + session.sessionInfo.value = DEFAULT_USER_SESSION_VALUES.sessionInfo; }
🧹 Nitpick comments (14)
packages/analytics-js/rollup.config.mjs (2)
38-39: Avoid treating empty env values as “valid”; guard against '' leading to root '/'.
??keeps an empty string, which then normalizes to/. If an env is set but empty, the fallback breaks. Prefer||or a trim guard.Apply:
-let remotePluginsBasePath = - process.env.REMOTE_MODULES_BASE_PATH ?? `${baseCdnUrl}/v3`; -remotePluginsBasePath = remotePluginsBasePath?.endsWith('/') ? remotePluginsBasePath : `${remotePluginsBasePath}/`; +const envRemoteModulesBasePath = process.env.REMOTE_MODULES_BASE_PATH; +let remotePluginsBasePath = + envRemoteModulesBasePath && envRemoteModulesBasePath.trim() !== '' + ? envRemoteModulesBasePath + : `${baseCdnUrl}/v3`; +remotePluginsBasePath = remotePluginsBasePath.endsWith('/') ? remotePluginsBasePath : `${remotePluginsBasePath}/`;
40-41: Mirror the same guard for DEST_SDK_BASE_URL.Prevents
''from normalizing to/.Apply:
-let destSDKBaseURL = process.env.DEST_SDK_BASE_URL ?? `${baseCdnUrl}/v3`; -destSDKBaseURL = destSDKBaseURL?.endsWith('/') ? destSDKBaseURL : `${destSDKBaseURL}/`; +const envDestSDKBaseURL = process.env.DEST_SDK_BASE_URL; +let destSDKBaseURL = + envDestSDKBaseURL && envDestSDKBaseURL.trim() !== '' + ? envDestSDKBaseURL + : `${baseCdnUrl}/v3`; +destSDKBaseURL = destSDKBaseURL.endsWith('/') ? destSDKBaseURL : `${destSDKBaseURL}/`;packages/analytics-js/__tests__/app/RudderAnalytics.test.ts (1)
379-397: Add a legacy false case to lock BC explicitly.Include a test for reset(false) to assert it forwards false unchanged.
Apply this diff to add the case:
@@ it('should process reset arguments and forwards to reset call without any arguments', () => { rudderAnalytics.reset(); expect(analyticsInstanceMock.reset).toHaveBeenCalledWith(undefined); }); + + it('should process reset arguments and forwards legacy boolean false unchanged', () => { + rudderAnalytics.reset(false); + expect(analyticsInstanceMock.reset).toHaveBeenCalledWith(false); + });packages/analytics-js/__tests__/components/userSessionManager/utils.test.ts (3)
406-544: Use DEFAULT_RESET_OPTIONS instead of in-test replica.Importing the source of truth avoids drift if defaults evolve.
Add this import near the top (outside this hunk):
import { DEFAULT_RESET_OPTIONS } from '../../../src/components/userSessionManager/constants';Then replace the local constant:
- const defaultOptions = { - entries: { - userId: true, - userTraits: true, - groupId: true, - groupTraits: true, - sessionInfo: true, - authToken: true, - anonymousId: false, - initialReferrer: false, - initialReferringDomain: false, - }, - }; + const defaultOptions = DEFAULT_RESET_OPTIONS;
451-463: Remove duplicate test case.This block duplicates the previous case and provides no additional coverage.
Apply this diff:
@@ - { - input: { - entries: { - userId: false, - }, - }, - expected: { - ...defaultOptions, - entries: { - ...defaultOptions.entries, - userId: false, - }, - }, - },
421-536: Add explicit coverage for initialReferrer/initialReferringDomain toggles.Since the PR introduces these, validate they can be turned on.
Apply this diff to extend testCases:
@@ { input: true, expected: { ...defaultOptions, entries: { ...defaultOptions.entries, anonymousId: true, }, }, }, + { + input: { + entries: { + initialReferrer: true, + initialReferringDomain: true, + }, + }, + expected: { + ...defaultOptions, + entries: { + ...defaultOptions.entries, + initialReferrer: true, + initialReferringDomain: true, + }, + }, + },packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts (1)
768-786: Iterate known keys instead of Object.keys(DEFAULT_USER_SESSION_VALUES).Using USER_SESSION_KEYS is more explicit and type-safe.
Apply this diff:
- batch(() => { - Object.keys(DEFAULT_USER_SESSION_VALUES).forEach(key => { - const userSessionKey = key as UserSessionKey; - if (opts.entries[userSessionKey] !== true) { - return; - } - - switch (key) { + batch(() => { + USER_SESSION_KEYS.forEach(userSessionKey => { + if (opts.entries[userSessionKey] !== true) return; + + switch (userSessionKey) { case 'anonymousId': this.setAnonymousId(); break; case 'sessionInfo': this.resetAndStartNewSession(); break; default: session[userSessionKey].value = DEFAULT_USER_SESSION_VALUES[userSessionKey]; break; } }); });packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts (5)
1775-1783: Use modern fake timers with inline time seed to simplify setupPrefer a single call with now to avoid the extra setSystemTime and reduce timer state churn.
- beforeEach(() => { - jest.useFakeTimers(); - jest.setSystemTime(0); - }); + beforeEach(() => { + jest.useFakeTimers({ now: 0 }); + });
1841-1851: Tighten assertions for regenerated sessionAdd minimal type/ordering checks to ensure a fresh session is created and is forward-looking.
// new session will be generated expect(state.session.sessionInfo.value.autoTrack).toBe(dataBeforeReset.sessionInfo.autoTrack); expect(state.session.sessionInfo.value.manualTrack).toBe( dataBeforeReset.sessionInfo.manualTrack, ); expect(state.session.sessionInfo.value.cutOff).toEqual(dataBeforeReset.sessionInfo.cutOff); expect(state.session.sessionInfo.value.timeout).toBe(dataBeforeReset.sessionInfo.timeout); expect(state.session.sessionInfo.value.expiresAt).not.toBe( dataBeforeReset.sessionInfo.expiresAt, ); expect(state.session.sessionInfo.value.id).not.toBe(dataBeforeReset.sessionInfo.id); + expect(typeof state.session.sessionInfo.value.id).toBe('number'); + expect(state.session.sessionInfo.value.expiresAt).toBeGreaterThan(Date.now());
1859-1863: Back-compat check: cover boolean false pathYou covered reset(true). Consider a quick test to assert reset(false) does not clear anonymousId, preserving the legacy boolean contract.
Add near this block:
it('should not clear anonymousId when first parameter is false', () => { state.storage.entries.value = entriesWithOnlyCookieStorage; userSessionManager.init(); userSessionManager.setAnonymousId(dummyAnonymousId); jest.advanceTimersByTime(1000); userSessionManager.reset(false); expect(state.session.anonymousId.value).toEqual(dummyAnonymousId); });
1889-1891: Assert against constant, not literal, for resilienceTie the expectation to DEFAULT_USER_SESSION_VALUES to avoid brittleness if defaults change later.
- expect(state.session.anonymousId.value).toEqual(''); + expect(state.session.anonymousId.value).toEqual(DEFAULT_USER_SESSION_VALUES.anonymousId);
2018-2028: Explicitly set non-reset entries to false to decouple from defaultsFuture changes to DEFAULT_RESET_OPTIONS won’t accidentally change this test’s meaning.
userSessionManager.reset({ entries: { userId: false, userTraits: false, groupId: false, groupTraits: false, sessionInfo: false, authToken: false, + anonymousId: false, + initialReferrer: false, + initialReferringDomain: false, }, });packages/analytics-js-common/src/utilities/object.ts (1)
141-148: Optional typing improvementConsider narrowing to objects and returning a readonly type for better DX; adopt only if it doesn’t ripple through call sites.
// Optional alternative signature: // const deepFreeze = <T extends object>(obj: T): Readonly<T> => { /* impl above */ }packages/analytics-js-common/__tests__/utilities/object.test.ts (1)
543-797: Fix Symbol matcher, quiet linter on sparse arrays, and assert deep immutability
expect.any(Symbol)won’t work because Symbols aren’t constructible; useexpect.anything()and add a focused type assert.- Silence Biome’s sparse-array warnings with targeted ignores.
- Add a couple of mutations on nested members to assert deep freeze behavior.
output: { mixedArray: [ 1, 'string', null, undefined, { nested: true }, [1, 2, { deep: 'value' }], expect.any(Function), expect.any(Date), expect.any(RegExp), - expect.any(Symbol), + expect.anything(), ], }, }, @@ - input: { - sparseArray: [1, , , 4, undefined, null], + input: { + /* biome-ignore lint/suspicious/noSparseArray: intentional to validate sparse array handling */ + sparseArray: [1, , , 4, undefined, null], emptyObject: {}, emptyArray: [], circularRef: { self: null }, }, output: { - sparseArray: [1, , , 4, undefined, null], + /* biome-ignore lint/suspicious/noSparseArray: intentional to validate sparse array handling */ + sparseArray: [1, , , 4, undefined, null], emptyObject: {}, emptyArray: [], circularRef: { self: null }, }, }, @@ it.each(tcData)( 'should freeze the object', ({ input, output }: { input: any; output: any }) => { const outcome = deepFreeze(input); expect(outcome).toEqual(output); @@ expect(() => { outcome.newProperty = 'added'; }).toThrow(); }, ); + + it('freezes nested members and preserves symbol type', () => { + const sym = Symbol('test'); + const input = { nested: { a: 1 }, arr: [{ b: 2 }, sym] }; + const outcome = deepFreeze(input); + expect(typeof outcome.arr[1]).toBe('symbol'); + expect(() => { + (outcome.nested as any).a = 2; + }).toThrow(); + expect(() => { + (outcome.arr[0] as any).b = 3; + }).toThrow(); + });After updating, please run the test suite locally to confirm Jest + Biome both pass.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
packages/analytics-js-common/__tests__/utilities/object.test.ts(2 hunks)packages/analytics-js-common/src/types/EventApi.ts(2 hunks)packages/analytics-js-common/src/types/IRudderAnalytics.ts(3 hunks)packages/analytics-js-common/src/utilities/object.ts(2 hunks)packages/analytics-js/.size-limit.mjs(3 hunks)packages/analytics-js/__tests__/app/RudderAnalytics.test.ts(2 hunks)packages/analytics-js/__tests__/components/core/Analytics.test.ts(1 hunks)packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts(3 hunks)packages/analytics-js/__tests__/components/userSessionManager/utils.test.ts(2 hunks)packages/analytics-js/public/index.html(1 hunks)packages/analytics-js/rollup.config.mjs(1 hunks)packages/analytics-js/src/app/RudderAnalytics.ts(2 hunks)packages/analytics-js/src/components/core/Analytics.ts(2 hunks)packages/analytics-js/src/components/core/IAnalytics.ts(2 hunks)packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts(3 hunks)packages/analytics-js/src/components/userSessionManager/constants.ts(2 hunks)packages/analytics-js/src/components/userSessionManager/types.ts(2 hunks)packages/analytics-js/src/components/userSessionManager/utils.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/analytics-js/src/components/core/Analytics.ts
- packages/analytics-js-common/src/types/EventApi.ts
- packages/analytics-js/src/components/userSessionManager/types.ts
- packages/analytics-js-common/src/types/IRudderAnalytics.ts
- packages/analytics-js/.size-limit.mjs
- packages/analytics-js/public/index.html
- packages/analytics-js/src/app/RudderAnalytics.ts
- packages/analytics-js/tests/components/core/Analytics.test.ts
- packages/analytics-js/src/components/userSessionManager/constants.ts
- packages/analytics-js/src/components/userSessionManager/utils.ts
🧰 Additional context used
📓 Path-based instructions (8)
packages/*/rollup.config.mjs
📄 CodeRabbit inference engine (CLAUDE.md)
Each package should define its build configuration in packages/[package-name]/rollup.config.mjs
Files:
packages/analytics-js/rollup.config.mjs
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/002-javascript-typescript.mdc)
Code must work in browsers AND service workers (no Node.js APIs)
Files:
packages/analytics-js/src/components/core/IAnalytics.tspackages/analytics-js/__tests__/app/RudderAnalytics.test.tspackages/analytics-js/__tests__/components/userSessionManager/utils.test.tspackages/analytics-js/src/components/userSessionManager/UserSessionManager.tspackages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.tspackages/analytics-js-common/src/utilities/object.tspackages/analytics-js-common/__tests__/utilities/object.test.ts
packages/*/src/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place all package source code under packages/[package-name]/src/
Files:
packages/analytics-js/src/components/core/IAnalytics.tspackages/analytics-js/src/components/userSessionManager/UserSessionManager.tspackages/analytics-js-common/src/utilities/object.ts
packages/analytics-js/src/**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use @preact/signals-core for reactive state in the core SDK (analytics-js)
Files:
packages/analytics-js/src/components/core/IAnalytics.tspackages/analytics-js/src/components/userSessionManager/UserSessionManager.ts
packages/!(analytics-js-common)/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer shared TypeScript types from analytics-js-common across packages
Files:
packages/analytics-js/src/components/core/IAnalytics.tspackages/analytics-js/src/components/userSessionManager/UserSessionManager.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Adhere to the repository’s ESLint configuration for JavaScript/TypeScript code
Files:
packages/analytics-js/src/components/core/IAnalytics.tspackages/analytics-js/__tests__/app/RudderAnalytics.test.tspackages/analytics-js/__tests__/components/userSessionManager/utils.test.tspackages/analytics-js/src/components/userSessionManager/UserSessionManager.tspackages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.tspackages/analytics-js-common/src/utilities/object.tspackages/analytics-js-common/__tests__/utilities/object.test.ts
packages/*/__tests__/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place unit tests under packages/[package-name]/tests/
Files:
packages/analytics-js/__tests__/app/RudderAnalytics.test.tspackages/analytics-js/__tests__/components/userSessionManager/utils.test.tspackages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.tspackages/analytics-js-common/__tests__/utilities/object.test.ts
packages/*/__tests__/**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Jest for tests and MSW for HTTP mocking
Files:
packages/analytics-js/__tests__/app/RudderAnalytics.test.tspackages/analytics-js/__tests__/components/userSessionManager/utils.test.tspackages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.tspackages/analytics-js-common/__tests__/utilities/object.test.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1754
File: packages/analytics-js-service-worker/README.md:0-0
Timestamp: 2024-10-08T15:52:59.819Z
Learning: saikumarrs prefers simplified language in documentation, avoiding redundant phrases.
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1754
File: packages/analytics-js-service-worker/README.md:0-0
Timestamp: 2024-06-14T09:50:33.511Z
Learning: saikumarrs prefers simplified language in documentation, avoiding redundant phrases.
📚 Learning: 2025-07-10T18:03:59.589Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#2330
File: packages/sanity-suite/public/v3/integrations/index-cdn.html:47-50
Timestamp: 2025-07-10T18:03:59.589Z
Learning: In the RudderStack SDK JavaScript project, when replacing hardcoded CDN URLs with configurable ones, trailing slash issues are handled at the build configuration level in rollup configs by trimming trailing slashes from the BASE_CDN_URL environment variable before injecting it as __BASE_CDN_URL__ placeholder, rather than adding runtime sanitization in individual files.
Applied to files:
packages/analytics-js/rollup.config.mjs
📚 Learning: 2024-10-08T15:52:59.819Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1730
File: packages/analytics-js/src/components/utilities/url.ts:10-10
Timestamp: 2024-10-08T15:52:59.819Z
Learning: The `removeTrailingSlashes` function in `packages/analytics-js/src/components/utilities/url.ts` now uses optional chaining for better safety and readability.
Applied to files:
packages/analytics-js/rollup.config.mjs
📚 Learning: 2025-07-15T14:23:42.846Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#2339
File: packages/analytics-js-integrations/scripts/integrationBuildScript.js:4-5
Timestamp: 2025-07-15T14:23:42.846Z
Learning: User saikumarrs is willing to accept cross-package relative imports (e.g., `../../analytics-js-legacy-utilities/src/...`) temporarily in the rudder-sdk-js repository, even when they violate workspace boundaries, if fixing them immediately is not a priority.
Applied to files:
packages/analytics-js/rollup.config.mjs
📚 Learning: 2024-11-09T06:40:30.520Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1823
File: packages/analytics-js-common/src/types/LoadOptions.ts:0-0
Timestamp: 2024-11-09T06:40:30.520Z
Learning: In the `packages/analytics-js-common/src/types/LoadOptions.ts` file, the `dataplanes` property within the `SourceConfigResponse` type has been removed as it is no longer necessary.
Applied to files:
packages/analytics-js/src/components/core/IAnalytics.ts
📚 Learning: 2024-10-08T15:52:59.819Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1708
File: packages/analytics-js-plugins/__tests__/deviceModeTransformation/index.test.ts:10-11
Timestamp: 2024-10-08T15:52:59.819Z
Learning: The misuse of `IHttpClient` in type assertions within the file `packages/analytics-js-plugins/__tests__/deviceModeTransformation/index.test.ts` has been corrected by the user.
Applied to files:
packages/analytics-js/__tests__/app/RudderAnalytics.test.tspackages/analytics-js/__tests__/components/userSessionManager/utils.test.ts
📚 Learning: 2024-11-08T13:17:51.356Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1823
File: packages/analytics-js-common/src/utilities/retryQueue/utilities.ts:0-0
Timestamp: 2024-11-08T13:17:51.356Z
Learning: The issue regarding missing test coverage for the `findOtherQueues` function in `packages/analytics-js-common/src/utilities/retryQueue/utilities.ts` is no longer applicable.
Applied to files:
packages/analytics-js/__tests__/components/userSessionManager/utils.test.ts
📚 Learning: 2025-05-06T09:03:18.823Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#2209
File: packages/analytics-js/__tests__/components/userSessionManager/utils.test.ts:209-223
Timestamp: 2025-05-06T09:03:18.823Z
Learning: The `generateAutoTrackingSession` function in the RudderStack JS SDK always generates new `id` and `expiresAt` values based on the current timestamp, regardless of any values provided in the input object. It extracts only `timeout` and `cutOff` properties from the input.
Applied to files:
packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts
📚 Learning: 2024-10-09T06:41:05.073Z
Learnt from: MoumitaM
PR: rudderlabs/rudder-sdk-js#1876
File: packages/analytics-js/src/app/RudderAnalytics.ts:0-0
Timestamp: 2024-10-09T06:41:05.073Z
Learning: The `trackPageLifecycleEvents` method in `packages/analytics-js/src/app/RudderAnalytics.ts` does not require refactoring as the current implementation is preferred.
Applied to files:
packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts
📚 Learning: 2024-11-09T05:04:51.002Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1823
File: packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts:1362-1362
Timestamp: 2024-11-09T05:04:51.002Z
Learning: In tests for the UserSessionManager, it's acceptable to spy on private methods like `private_syncValueToStorage` when necessary.
Applied to files:
packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts
📚 Learning: 2024-10-28T08:09:31.840Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1902
File: packages/analytics-js-common/src/utilities/object.ts:12-13
Timestamp: 2024-10-28T08:09:31.840Z
Learning: In `packages/analytics-js-common/src/utilities/object.ts`, the `isObject` function is intended to check if a value is of type 'object', including when the value is `null`. Null checking is handled separately in functions like `isObjectAndNotNull`.
Applied to files:
packages/analytics-js-common/__tests__/utilities/object.test.ts
🧬 Code graph analysis (5)
packages/analytics-js/rollup.config.mjs (2)
packages/sanity-suite/rollup.config.mjs (1)
baseCdnUrl(20-20)packages/analytics-v1.1/rollup-configs/rollup.utilities.mjs (1)
baseCdnUrl(22-22)
packages/analytics-js/src/components/core/IAnalytics.ts (1)
packages/analytics-js-common/src/types/EventApi.ts (1)
ResetOptions(49-53)
packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts (4)
packages/analytics-js/src/components/userSessionManager/constants.ts (1)
DEFAULT_USER_SESSION_VALUES(36-36)packages/analytics-js-common/src/types/EventApi.ts (1)
ResetOptions(49-53)packages/analytics-js/src/components/userSessionManager/utils.ts (1)
getFinalResetOptions(158-158)packages/analytics-js-common/src/types/UserSessionStorage.ts (1)
UserSessionKey(1-10)
packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts (1)
packages/analytics-js/__fixtures__/fixtures.ts (2)
entriesWithOnlyCookieStorage(744-744)anonymousIdWithNoStorageEntries(753-753)
packages/analytics-js-common/__tests__/utilities/object.test.ts (1)
packages/analytics-js-common/src/utilities/object.ts (1)
deepFreeze(163-163)
🪛 Biome (2.1.2)
packages/analytics-js-common/__tests__/utilities/object.test.ts
[error] 631-631: This array contains an empty slots..
The presences of empty slots may cause incorrect information and might be a typo.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
[error] 637-637: This array contains an empty slots..
The presences of empty slots may cause incorrect information and might be a typo.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Bundle size checks
- GitHub Check: Code quality checks
- GitHub Check: Unit Tests and Lint
🔇 Additional comments (9)
packages/analytics-js/src/components/core/IAnalytics.ts (1)
9-9: LGTM: type import is correct and scoped as type-only.
UsingResetOptionsfrom analytics-js-common keeps API types centralized. No issues.packages/analytics-js/__tests__/app/RudderAnalytics.test.ts (2)
379-397: Good coverage for object/undefined overloads of reset().Forwarding the options object and undefined looks correct and matches the new API.
408-409: LGTM: error path updated to no-arg reset().Still exercises the dispatchErrorEvent flow correctly.
packages/analytics-js/__tests__/components/userSessionManager/utils.test.ts (1)
519-535: Confirm pass-through of non-boolean/unknown keys is intentional.The test expects preservation of entries like userId: 'xyz' and abc: false. This is tolerant, but verify downstream consumers won’t misinterpret these values.
packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts (1)
763-767: Good: centralized option normalization.Using getFinalResetOptions() up front simplifies the reset logic and preserves BC for boolean inputs.
packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts (2)
1865-1877: LGTM: options-based anonymousId reset pathCovers the new ResetOptions shape correctly and asserts the regenerated value.
2062-2076: LGTM: advancing timers before reset ensures deterministic new sessionAdvancing by 1000ms avoids id/expiresAt collisions with the initial session started at time 0.
packages/analytics-js-common/src/utilities/object.ts (1)
141-148: Add cycle detection, symbol-keyed props, and function recursion to deepFreeze
Refactor deepFreeze in packages/analytics-js-common/src/utilities/object.ts to feature-detect and use a WeakSet for seen objects, recurse over both Object.getOwnPropertyNames and Object.getOwnPropertySymbols, and include function-typed values before freezing. Run the browser matrix (including IE11) to confirm no regressions.packages/analytics-js-common/__tests__/utilities/object.test.ts (1)
13-14: LGTM on importImporting
deepFreezehere is correct.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts (4)
1802-1812: DRY the repeated snapshot constructionYou build the same snapshot object in multiple tests. Consider extracting a tiny helper to reduce duplication and make future edits safer.
Apply outside the selected ranges (helper), then simplify call sites:
// near other helpers above const snapshotSession = () => JSON.parse(JSON.stringify({ userId: state.session.userId.value, userTraits: state.session.userTraits.value, groupId: state.session.groupId.value, groupTraits: state.session.groupTraits.value, initialReferrer: state.session.initialReferrer.value, initialReferringDomain: state.session.initialReferringDomain.value, anonymousId: state.session.anonymousId.value, sessionInfo: state.session.sessionInfo.value, authToken: state.session.authToken.value, }));-const dataBeforeReset = JSON.parse(JSON.stringify({ ... })); +const dataBeforeReset = snapshotSession();-const dataAfterReset = JSON.parse(JSON.stringify({ ... })); +const dataAfterReset = snapshotSession();Also applies to: 1934-1944, 2030-2040
1879-1891: Also assert non-persistence when storage type is noneMinor enhancement: verify the ID isn’t written to storage.
userSessionManager.reset({ entries: { anonymousId: true }, }); expect(state.session.anonymousId.value).toEqual(''); +expect(clientDataStoreCookie.get(COOKIE_KEYS.anonymousId)).toBeUndefined();
1915-1985: Strengthen expectations for referrer fields and anonymousIdYou currently assert “changed” (notEqual). Prefer asserting the exact post-reset values for clarity and to catch accidental regressions.
-Expect(dataAfterReset.initialReferrer).not.toEqual(dataBeforeReset.initialReferrer); -Expect(dataAfterReset.initialReferringDomain).not.toEqual(dataBeforeReset.initialReferringDomain); -Expect(dataAfterReset.anonymousId).not.toEqual(dataBeforeReset.anonymousId); +expect(dataAfterReset.initialReferrer).toBe('$direct'); +expect(dataAfterReset.initialReferringDomain).toBe(''); +expect(dataAfterReset.anonymousId).toBe('test_uuid');
2062-2062: Tiny nit: advance 1ms instead of 1000msWith fake timers, advancing by 1ms is sufficient to guarantee new timestamps/IDs and keeps intent obvious.
-jest.advanceTimersByTime(1000); +jest.advanceTimersByTime(1);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/002-javascript-typescript.mdc)
Code must work in browsers AND service workers (no Node.js APIs)
Files:
packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts
packages/*/__tests__/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place unit tests under packages/[package-name]/tests/
Files:
packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts
packages/*/__tests__/**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Jest for tests and MSW for HTTP mocking
Files:
packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Adhere to the repository’s ESLint configuration for JavaScript/TypeScript code
Files:
packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1754
File: packages/analytics-js-service-worker/README.md:0-0
Timestamp: 2024-06-14T09:50:33.511Z
Learning: saikumarrs prefers simplified language in documentation, avoiding redundant phrases.
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1754
File: packages/analytics-js-service-worker/README.md:0-0
Timestamp: 2024-10-08T15:52:59.819Z
Learning: saikumarrs prefers simplified language in documentation, avoiding redundant phrases.
📚 Learning: 2024-10-28T08:19:43.438Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1902
File: packages/analytics-js/src/components/core/Analytics.ts:125-127
Timestamp: 2024-10-28T08:19:43.438Z
Learning: In `packages/analytics-js/src/components/core/Analytics.ts`, inputs to the `clone` function are sanitized prior, so error handling for clone operations is not needed.
Applied to files:
packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts
📚 Learning: 2024-11-09T05:04:51.002Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1823
File: packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts:1362-1362
Timestamp: 2024-11-09T05:04:51.002Z
Learning: In tests for the UserSessionManager, it's acceptable to spy on private methods like `private_syncValueToStorage` when necessary.
Applied to files:
packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts
📚 Learning: 2024-11-09T05:32:00.696Z
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1823
File: packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts:72-72
Timestamp: 2024-11-09T05:32:00.696Z
Learning: In `UserSessionManager.test.ts`, when necessary, it's acceptable to directly set raw cookie values by accessing the original storage engine with `getOriginalEngine()`, instead of using the `Store` interface methods.
Applied to files:
packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Code quality checks
- GitHub Check: Bundle size checks
- GitHub Check: Unit Tests and Lint
🔇 Additional comments (6)
packages/analytics-js/__tests__/components/userSessionManager/UserSessionManager.test.ts (6)
1775-1783: Good use of fake timers to make reset-suite deterministicIsolating the suite with
useFakeTimers/setSystemTimein beforeEach and restoring in afterEach avoids cross-test flakiness. LGTM.
1784-1852: Reset default behavior test is precise and robust
- Nice end-to-end coverage of per-key expectations.
- Deep-cloned snapshot prevents aliasing. LGTM.
1841-1851: Session re-generation assertions look correctAsserting same auto/manual/timeout/cutOff and different id/expiresAt with sessionStart undefined matches the contract. LGTM.
1854-1863: Boolean overload and options-path for anonymousId reset are coveredBoth
reset(true)and{ entries: { anonymousId: true } }paths validated against the mocked UUID. LGTM.Also applies to: 1865-1877
1893-1913: “Do not start new session” option verified via strict equalityCapturing a deep clone and asserting equality is solid. LGTM.
1987-2051: “No resets when configured false” is comprehensiveGood negative test covering all entries including sessionInfo. LGTM.
saikumarrs
deleted the
feature/sdk-3362-js-implementation-of-overload-reset-method
branch
PR Description
The
resetAPI now supportsoptionsargument along withresetAnonymousIdflag for backward compatibility.The options also support to reset the
initialReferrerandinitialReferringDomainentries.By default, anonymous ID, initial referrer and initial referring domain values are not cleared.
Linear task (optional)
https://linear.app/rudderstack/issue/SDK-3362/js-implementation-of-overload-reset-method
Cross Browser Tests
Please confirm you have tested for the following browsers:
Sanity Suite
Security
Summary by CodeRabbit
New Features
Utilities
Documentation
Tests
Chores