feat(core): add namespace support to LocalStorageAdapter for micro-fr…#97
Merged
purushpsm147 merged 3 commits intomainfrom Mar 29, 2026
Merged
feat(core): add namespace support to LocalStorageAdapter for micro-fr…#97purushpsm147 merged 3 commits intomainfrom
purushpsm147 merged 3 commits intomainfrom
Conversation
…ontend isolation - Introduced a `namespace` parameter to `LocalStorageAdapter` to prevent key collisions between multiple instances sharing the same origin. - Updated methods to use namespaced keys for `load`, `save`, and `delete` operations. - Added documentation for namespace usage in the adapter. feat(core): extend IntentManagerConfig and IntentEngineConfig with namespace option - Added `namespace` option to `IntentManagerConfig` and `IntentEngineConfig` to allow configuration of localStorage key prefixes for instance isolation. fix(tests): update import paths in tests to use the correct index file - Changed import statements in various test files to reference the correct module entry point. chore(react): deprecate usePropensityScore hook in favor of usePropensity - Marked `usePropensityScore` as deprecated and updated documentation to recommend `usePropensity`. - Added a console warning for the deprecated hook to guide users towards migration. fix(remix): improve error handling in createIntentClientLoader - Wrapped serverLoader calls in try-catch to provide clearer diagnostic messages when serverLoader is missing. test(remix): enhance tests for new features and deprecations - Added tests for deprecation warnings in `usePropensityScore`. - Expanded test coverage for `createIntentClientLoader` and `useRoutePassiveIntent` to ensure correct behavior with new implementations.
Contributor
📝 WalkthroughWalkthroughAdds optional namespacing for browser/localStorage persistence (default "passiveintent:"), a Changes
Sequence Diagram(s)sequenceDiagram
participant Consumer
participant BrowserStorageAdapter
participant localStorage as LocalStorage
Note over BrowserStorageAdapter: namespace = "passiveintent:"
Consumer->>BrowserStorageAdapter: getItem(key)
BrowserStorageAdapter->>localStorage: getItem("passiveintent:" + key)
alt namespaced value found
localStorage-->>BrowserStorageAdapter: value
BrowserStorageAdapter-->>Consumer: value
else not found AND using default namespace
localStorage-->>BrowserStorageAdapter: null
BrowserStorageAdapter->>localStorage: getItem(key) %% legacy lookup
alt legacy value found
localStorage-->>BrowserStorageAdapter: legacyValue
BrowserStorageAdapter->>localStorage: setItem("passiveintent:" + key, legacyValue)
BrowserStorageAdapter->>localStorage: removeItem(key)
BrowserStorageAdapter-->>Consumer: legacyValue
else
localStorage-->>BrowserStorageAdapter: null
BrowserStorageAdapter-->>Consumer: null
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/remix/src/withPassiveIntent.tsx (1)
1-56:⚠️ Potential issue | 🟡 MinorAddress Prettier formatting warning.
The CI pipeline reports a Prettier formatting issue in this file. Please run
prettier --writeto resolve the warning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/remix/src/withPassiveIntent.tsx` around lines 1 - 56, Run Prettier to fix formatting issues in this file: format the file containing withPassiveIntent and ClientOnly by running your project's Prettier config (e.g., prettier --write) so the export function withPassiveIntent, the PassiveIntentRoot component, imports, and JSDoc block match repo style; ensure the JSX return and multi-line strings are wrapped/indented according to Prettier rules and commit the reformatted file.
🧹 Nitpick comments (1)
packages/core/tests/integration-contract.test.mjs (1)
97-123: Consider adding one custom-namespace integration case.Lines [97-123] and Line [222] validate the default prefix well. Adding one test using a non-default namespace would close the key objective’s most important regression gap.
Also applies to: 222-222
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/tests/integration-contract.test.mjs` around lines 97 - 123, Add a new integration test that exercises IntentManager with a custom/non-default storage key to verify the prefixing behavior and persistence semantics; instantiate IntentManager with a unique storageKey (e.g., 'custom-namespace-test'), call manager.track(...) and manager.flushNow(), assert storage.getItem('custom-namespace-test') exists and equals the persisted payload, then call manager.track(...) again, flushNow(), and assert the stored value changed as expected; reference IntentManager, storageKey, track, flushNow, and storage.getItem to locate where to add the test alongside the existing dirty-flag tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/cypress/e2e/browser-intent.cy.ts`:
- Line 113: The test reads localStorage key
'passiveintent:passive-intent-browser-test' (seen in the stored =
win.localStorage.getItem(...) line) but the test setup/teardown still clears
'passive-intent-browser-test'; update the cleanup in the beforeEach/afterEach
block that currently removes 'passive-intent-browser-test' to remove the
namespaced key 'passiveintent:passive-intent-browser-test' (and search/replace
any other occurrences of the old key), ensuring tests fully isolate persisted
state.
In `@packages/core/src/adapters.ts`:
- Around line 55-70: The current BrowserStorageAdapter changes keys by
prepending the namespace which breaks restores for existing installs; update
BrowserStorageAdapter so when the constructor receives the default namespace
('passiveintent:'), the storage access methods (at minimum getItem, and also
setItem and removeItem if present) first try the namespaced key (nsKey(key)) and
if that returns null fall back to the legacy unprefixed key (key); implement the
same fallback logic in the corresponding LocalStorageAdapter
(packages/core/src/plugins/web/LocalStorageAdapter.ts) to ensure compatibility
for existing persisted graph/Bloom state.
In `@packages/core/src/plugins/web/LocalStorageAdapter.ts`:
- Around line 51-52: The class LocalStorageAdapter implements
IPersistenceAdapter but adds a delete(key) method that isn't declared on the
IPersistenceAdapter interface, so callers using the interface can't see or call
it; update the IPersistenceAdapter interface to include an optional delete?(key:
string): void (so existing adapters remain compatible) and update callsites to
invoke persistenceAdapter.delete?.(key) (optional chaining) where deletion may
occur; ensure both occurrences referenced in the diff (the LocalStorageAdapter
class and the other implementation block around lines 106-117) are consistent
with the new optional method.
In `@packages/core/src/types/microkernel.ts`:
- Around line 254-259: IntentEngineConfig exposes a namespace string that should
be prepended to every persistence key but IntentEngine currently uses storageKey
directly; update the IntentEngine constructor and persistence usage to compose a
namespaced key (e.g., const persistedKey = `${config.namespace ??
'passiveintent:'}${this.storageKey}`) and use that composed key when calling
persistence.load(...) and _persist(...); ensure any other internal places that
reference this.storageKey for persistence (in methods like persistence.load and
_persist) use the composed persistedKey so the namespace is consistently
applied.
In `@packages/remix/tests/with-passive-intent.test.tsx`:
- Around line 158-197: The tests are only asserting the absence of warnings but
never the positive case; update the suite to actually assert that console.warn
is called with a PassiveIntent-prefixed message when the HOC is created with a
different config reference. Specifically, in the first or second test use
vi.spyOn(console, 'warn') as warnSpy, call withPassiveIntent(Page, configA) then
withPassiveIntent(Page, configB) (or render two wrapped components created with
different config objects), render the second wrapped component to trigger the
warning inside withPassiveIntent, and add
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('[PassiveIntent]'));
then restore warnSpy; reference withPassiveIntent, Page, warnSpy, render and
rerender to locate where to change.
---
Outside diff comments:
In `@packages/remix/src/withPassiveIntent.tsx`:
- Around line 1-56: Run Prettier to fix formatting issues in this file: format
the file containing withPassiveIntent and ClientOnly by running your project's
Prettier config (e.g., prettier --write) so the export function
withPassiveIntent, the PassiveIntentRoot component, imports, and JSDoc block
match repo style; ensure the JSX return and multi-line strings are
wrapped/indented according to Prettier rules and commit the reformatted file.
---
Nitpick comments:
In `@packages/core/tests/integration-contract.test.mjs`:
- Around line 97-123: Add a new integration test that exercises IntentManager
with a custom/non-default storage key to verify the prefixing behavior and
persistence semantics; instantiate IntentManager with a unique storageKey (e.g.,
'custom-namespace-test'), call manager.track(...) and manager.flushNow(), assert
storage.getItem('custom-namespace-test') exists and equals the persisted
payload, then call manager.track(...) again, flushNow(), and assert the stored
value changed as expected; reference IntentManager, storageKey, track, flushNow,
and storage.getItem to locate where to add the test alongside the existing
dirty-flag tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4ae8e8de-282c-4f9e-badd-884239d906aa
📒 Files selected for processing (44)
packages/core/CHANGELOG.mdpackages/core/README.mdpackages/core/cypress/e2e/amazon.cy.tspackages/core/cypress/e2e/browser-intent.cy.tspackages/core/cypress/e2e/intent.cy.tspackages/core/package.jsonpackages/core/sandbox/amazon/app.tspackages/core/sandbox/app.tspackages/core/scripts/perf-runner.mjspackages/core/scripts/roc-experiment.mjspackages/core/scripts/smoothing-alpha-benchmark.mjspackages/core/src/adapters.tspackages/core/src/engine/config-normalizer.tspackages/core/src/engine/intent-manager.tspackages/core/src/factory.tspackages/core/src/index.tspackages/core/src/intent-sdk-performance.tspackages/core/src/intent-sdk.tspackages/core/src/plugins/web/LocalStorageAdapter.tspackages/core/src/types/events.tspackages/core/src/types/microkernel.tspackages/core/tests/anomaly-dispatcher.test.mjspackages/core/tests/exit-intent.test.mjspackages/core/tests/idle-detection.test.mjspackages/core/tests/integration-contract.test.mjspackages/core/tests/policies.test.mjspackages/core/tests/probabilistic.test.mjspackages/core/tests/propensity-calculator.test.mjspackages/core/tests/property-based.test.mjspackages/core/tests/unit-fast.test.mjspackages/react/CHANGELOG.mdpackages/react/README.mdpackages/react/package.jsonpackages/react/src/hooks.tspackages/react/tests/use-propensity-score-deprecation.test.tspackages/remix/CHANGELOG.mdpackages/remix/README.mdpackages/remix/package.jsonpackages/remix/src/loaders.tspackages/remix/src/useRoutePassiveIntent.tspackages/remix/src/withPassiveIntent.tsxpackages/remix/tests/loaders.test.tspackages/remix/tests/use-route-passive-intent.test.tsxpackages/remix/tests/with-passive-intent.test.tsx
💤 Files with no reviewable changes (1)
- packages/core/src/intent-sdk.ts
…rageAdapter and LocalStorageAdapter
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react/tests/use-propensity-score-deprecation.test.ts (1)
139-155: Consider usingvi.stubEnv()for more reliable environment stubbing.Directly mutating
process.env.NODE_ENVcan be fragile if the value is statically replaced at build time. Vitest providesvi.stubEnv()for this purpose.♻️ Proposed refactor using vi.stubEnv
describe('3 — suppressed in production', () => { it('does not call console.warn when NODE_ENV is production', () => { - const original = process.env.NODE_ENV; - // `@ts-expect-error` — overriding read-only env for test - process.env.NODE_ENV = 'production'; + vi.stubEnv('NODE_ENV', 'production'); try { renderHook(() => usePropensityScore('/checkout'), { wrapper: ({ children }) => withProvider(children), }); expect(warnSpy).not.toHaveBeenCalled(); } finally { - // `@ts-expect-error` - process.env.NODE_ENV = original; + vi.unstubAllEnvs(); } }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/tests/use-propensity-score-deprecation.test.ts` around lines 139 - 155, The test directly mutates process.env.NODE_ENV which is brittle; replace that mutation with Vitest's vi.stubEnv('NODE_ENV', 'production') before calling renderHook (in the "does not call console.warn when NODE_ENV is production" case) and remove the manual restore in the finally block, instead calling vi.unstubAllEnvs() in the finally to revert the stub; update the test to use vi.stubEnv and vi.unstubAllEnvs while keeping the same renderHook(() => usePropensityScore('/checkout'), { wrapper: ({ children }) => withProvider(children) }) and the warnSpy assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/react/tests/use-propensity-score-deprecation.test.ts`:
- Around line 139-155: The test directly mutates process.env.NODE_ENV which is
brittle; replace that mutation with Vitest's vi.stubEnv('NODE_ENV',
'production') before calling renderHook (in the "does not call console.warn when
NODE_ENV is production" case) and remove the manual restore in the finally
block, instead calling vi.unstubAllEnvs() in the finally to revert the stub;
update the test to use vi.stubEnv and vi.unstubAllEnvs while keeping the same
renderHook(() => usePropensityScore('/checkout'), { wrapper: ({ children }) =>
withProvider(children) }) and the warnSpy assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 43f51da1-a132-4a74-a1a3-2e08fef8c49c
📒 Files selected for processing (12)
packages/core/CHANGELOG.mdpackages/core/README.mdpackages/core/cypress/e2e/browser-intent.cy.tspackages/core/src/adapters.tspackages/core/src/plugins/web/LocalStorageAdapter.tspackages/core/src/types/microkernel.tspackages/react/README.mdpackages/react/src/hooks.tspackages/react/tests/use-propensity-score-deprecation.test.tspackages/remix/src/loaders.tspackages/remix/src/withPassiveIntent.tsxpackages/remix/tests/with-passive-intent.test.tsx
✅ Files skipped from review due to trivial changes (2)
- packages/core/CHANGELOG.md
- packages/react/src/hooks.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/core/cypress/e2e/browser-intent.cy.ts
- packages/remix/src/withPassiveIntent.tsx
- packages/remix/src/loaders.ts
- packages/remix/tests/with-passive-intent.test.tsx
- packages/core/src/plugins/web/LocalStorageAdapter.ts
- packages/core/src/adapters.ts
- packages/react/README.md
- packages/core/README.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a new namespacing feature for localStorage keys across the PassiveIntent core SDK, which is crucial for preventing key collisions when multiple PassiveIntent instances (such as in micro-frontend architectures) share the same browser origin. It also includes a significant internal refactor to the module export structure, removing the internal barrel file and exporting all symbols directly from their canonical modules. Documentation and tests have been updated to reflect these changes.
Key changes:
Namespacing and Persistence Isolation
Added an optional
namespaceparameter to theBrowserStorageAdapterandLocalStorageAdapterconstructors, defaulting to'passiveintent:'. This prefix is prepended to every localStorage key to isolate data between multiple PassiveIntent instances. TheIntentManagerConfigandBrowserConfiginterfaces now also accept anamespacefield, which is forwarded to the storage adapter. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]Cypress tests and sandboxes updated to expect namespaced keys in localStorage, ensuring correct usage and coverage of the new feature. [1] [2] [3]
Internal Refactoring & Exports
intent-sdk.tsbarrel file; all exports now resolve directly from their source modules, improving maintainability and clarity. This is a non-breaking internal change; all exports remain available from@passiveintent/core. [1] [2] [3] [4] [5] [6] [7] [8]Documentation
README.mdandCHANGELOG.mdto document the newnamespaceoption, its default value, and usage examples for both the storage adapters and configuration objects. [1] [2] [3] [4]Version Bump
1.3.0to reflect the new feature and internal refactor.…ontend isolationnamespaceparameter toLocalStorageAdapterto prevent key collisions between multiple instances sharing the same origin.load,save, anddeleteoperations.feat(core): extend IntentManagerConfig and IntentEngineConfig with namespace option
namespaceoption toIntentManagerConfigandIntentEngineConfigto allow configuration of localStorage key prefixes for instance isolation.fix(tests): update import paths in tests to use the correct index file
chore(react): deprecate usePropensityScore hook in favor of usePropensity
usePropensityScoreas deprecated and updated documentation to recommendusePropensity.fix(remix): improve error handling in createIntentClientLoader
test(remix): enhance tests for new features and deprecations
usePropensityScore.createIntentClientLoaderanduseRoutePassiveIntentto ensure correct behavior with new implementations.Description
Type of change
Checklist
npm run lintandnpm run typechecklocallynpm run test:unit -w @passiveintent/core)Summary by CodeRabbit
New Features
Deprecation
Bug Fixes
Documentation