Skip to content

Manager API: Namespace localStorage keys by project ID #34352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
cyphercodes wants to merge 1 commit into from
Closed

Manager API: Namespace localStorage keys by project ID #34352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions code/core/src/manager-api/root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import type {

import { isEqual } from 'es-toolkit/predicate';

import { getAnonymousProjectId } from '../telemetry/anonymous-id';

@coderabbitai coderabbitai Bot Mar 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if getAnonymousProjectId has browser-compatible implementation or is build-time resolved

echo "=== Checking getAnonymousProjectId implementation ==="
cat code/core/src/telemetry/anonymous-id.ts 2>/dev/null || fd "anonymous-id" --exec cat {}

echo ""
echo "=== Check if value is injected via build/bundler config ==="
rg -n "anonymousProjectId|getAnonymousProjectId" --type ts --type js -g '!*.test.*' code/core/src/manager/ code/builders/ 2>/dev/null | head -30

echo ""
echo "=== Check for any globals or window injection ==="
rg -n "STORYBOOK.*PROJECT|projectId" code/core/src/manager-api/ --type ts -A 2 -B 2 | head -40

Repository: storybookjs/storybook

Length of output: 5171

🏁 Script executed:

# Check the exact usage in root.tsx
echo "=== Usage in root.tsx ===" 
cat -n code/core/src/manager-api/root.tsx | sed -n '40,160p'

echo ""
echo "=== Check if there's any webpack/build config handling this ===" 
fd -e config.js -e webpack -e vite | xargs grep -l "getAnonymousProjectId\|anonymous-id" 2>/dev/null || echo "No matching build config found"

echo ""
echo "=== Check if the import has any side-effects disabled ===" 
rg "getAnonymousProjectId" code/core/src/manager-api/root.tsx -B 5 -A 5

Repository: storybookjs/storybook

Length of output: 5025

🏁 Script executed:

# Check for comments explaining this pattern in telemetry or store files
echo "=== Comments in anonymous-id.ts ===" 
grep -n "// \|/\*" code/core/src/telemetry/anonymous-id.ts

echo ""
echo "=== Check if this is SSR or build-time executed ===" 
rg "import.*anonymous-id\|getAnonymousProjectId" code/core/src/manager-api/ -B 3 -A 3

echo ""
echo "=== Check if there's server-side setup that would inject projectId ===" 
rg "projectId" code/core/src/main/manager/manager-main.ts -A 2 -B 2 2>/dev/null | head -30

Repository: storybookjs/storybook

Length of output: 606

Remove Node.js-dependent code from browser context, or defer projectId resolution.

getAnonymousProjectId() calls process.cwd() and executeCommandSync() which are not available in the browser. The function's try-catch silently fails, returning undefined, causing the Store to use an un-namespaced storage key and defeating the project isolation purpose of this PR.

Solution options:

  1. Move this call to a build-time step or server-side initialization that passes projectId as a prop
  2. Accept projectId as an optional prop to ManagerProvider (from server-side setup)
  3. Wrap the Node.js code with proper typeof process !== 'undefined' checks if SSR-compatible behavior is intended
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@code/core/src/manager-api/root.tsx` at line 48, The code calls
getAnonymousProjectId() (which uses process.cwd() and executeCommandSync()) in
the browser context causing undefined projectId and un-namespaced Store keys;
stop resolving projectId in the client—update ManagerProvider to accept an
optional projectId prop (e.g., add projectId?: string to ManagerProvider props
and use that when constructing the Store) and remove the direct
getAnonymousProjectId() invocation from root.tsx; alternatively, if SSR-safe
resolution is intended, wrap any getAnonymousProjectId() usage with a runtime
guard (typeof process !== 'undefined') or move the call to server/build-time and
pass the resulting projectId into ManagerProvider as a prop.

import { createContext } from './context';
import getInitialState from './initial-state';
import { types } from './lib/addons';
Expand Down Expand Up @@ -148,6 +149,7 @@ class ManagerProvider extends Component<ManagerProviderProps, State> {
} = props;

const store = new Store({
projectId: getAnonymousProjectId(),
getState: () => this.state,
setState: (stateChange: Partial<State>, callback) => {
this.setState(stateChange, () => callback(this.state));
Expand Down
37 changes: 24 additions & 13 deletions code/core/src/manager-api/store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,23 @@ storeSetup(store._);

export const STORAGE_KEY = '@storybook/manager/store';

function get(storage: StoreAPI) {
const data = storage.get(STORAGE_KEY);
function getStorageKey(projectId?: string) {
return projectId ? `${STORAGE_KEY}:${projectId}` : STORAGE_KEY;
}

function get(storage: StoreAPI, projectId?: string) {
const data = storage.get(getStorageKey(projectId));
return data || {};
}

function set(storage: StoreAPI, value: Patch) {
return storage.set(STORAGE_KEY, value);
function set(storage: StoreAPI, value: Patch, projectId?: string) {
return storage.set(getStorageKey(projectId), value);
}

function update(storage: StoreAPI, patch: Patch) {
const previous = get(storage);
function update(storage: StoreAPI, patch: Patch, projectId?: string) {
const previous = get(storage, projectId);
// Apply the same behaviour as react here
return set(storage, { ...previous, ...patch });
return set(storage, { ...previous, ...patch }, projectId);
}

type GetState = () => State;
Expand All @@ -33,6 +37,11 @@ export interface Upstream {
* persistence in Storybook's own tests. True by default.
*/
allowPersistence?: boolean;
/**
* Unique project identifier used to namespace localStorage/sessionStorage keys.
* Prevents state from being shared across different Storybook instances.
*/
projectId?: string;
getState: GetState;
setState: SetState;
}
Expand Down Expand Up @@ -60,10 +69,12 @@ export default class Store {
upstreamPersistence: boolean;
upstreamGetState: GetState;
upstreamSetState: SetState;
projectId: string | undefined;
private persistenceHandlers: Map<string, PersistenceHandler> = new Map();

constructor({ allowPersistence, setState, getState }: Upstream) {
constructor({ allowPersistence, projectId, setState, getState }: Upstream) {
this.upstreamPersistence = allowPersistence ?? true;
this.projectId = projectId;
this.upstreamSetState = setState;
this.upstreamGetState = getState;
}
Expand All @@ -79,18 +90,18 @@ export default class Store {
// One-time migration: tag filter state moved from localStorage to URL persistence.
// Remove the old keys so they no longer interfere with URL-derived initial state.
for (const storage of [store.local, store.session] as const) {
const persisted = get(storage);
const persisted = get(storage, this.projectId);
if ('includedTagFilters' in persisted || 'excludedTagFilters' in persisted) {
const { includedTagFilters: _i, excludedTagFilters: _e, ...rest } = persisted;
set(storage, rest);
set(storage, rest, this.projectId);
}
}

// We don't only merge at the very top level (the same way as React setState)
// when you set keys, so it makes sense to do the same in combining the two storage modes
// Really, you shouldn't store the same key in both places
const local = get(store.local);
const session = get(store.session);
const local = get(store.local, this.projectId);
const session = get(store.session, this.projectId);

return { ...base, ...local, ...session };
}
Expand Down Expand Up @@ -147,7 +158,7 @@ export default class Store {
}
} else {
const storage = persistence === 'session' ? store.session : store.local;
await update(storage, delta);
await update(storage, delta, this.projectId);
}
Comment on lines 160 to 162

@coderabbitai coderabbitai Bot Mar 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify test coverage for projectId-namespaced storage paths

echo "=== Checking Store instantiations in tests ==="
rg -n "new Store\(" --type js --type ts -g '*test*' -A 5

echo ""
echo "=== Checking if any tests pass projectId ==="
rg -n "projectId" --type js --type ts -g '*test*' code/core/src/manager-api/

Repository: storybookjs/storybook

Length of output: 5292

🏁 Script executed:

#!/bin/bash
# Check Store class definition and projectId usage
echo "=== Store class constructor signature ==="
head -50 code/core/src/manager-api/store.ts | cat -n

echo ""
echo "=== Lines around 160-162 in store.ts ==="
sed -n '150,170p' code/core/src/manager-api/store.ts | cat -n

echo ""
echo "=== Check if projectId is a constructor parameter ==="
rg -n "constructor\|projectId" code/core/src/manager-api/store.ts -A 2 -B 1

Repository: storybookjs/storybook

Length of output: 2681

🏁 Script executed:

#!/bin/bash
# Find the Store class constructor and projectId assignment
echo "=== Full Store class with constructor ==="
sed -n '50,130p' code/core/src/manager-api/store.ts | cat -n

echo ""
echo "=== Check root.tsx for Store instantiation ==="
rg -n "new Store" code/core/src/root.tsx -B 2 -A 5

Repository: storybookjs/storybook

Length of output: 3769

🏁 Script executed:

#!/bin/bash
# Find root.tsx
echo "=== Searching for root.tsx ==="
fd -name "root.tsx" code/

echo ""
echo "=== Searching for Store instantiation in production ==="
rg -n "new Store" code/core/src/ -t ts -t tsx --max-count 5

Repository: storybookjs/storybook

Length of output: 428

🏁 Script executed:

#!/bin/bash
# Search for root.tsx and Store instantiation with correct syntax
echo "=== Searching for root.tsx in entire repo ==="
fd root.tsx

echo ""
echo "=== Searching for Store instantiation in manager-api ==="
rg "new Store" code/core/src/manager-api/ -A 3

echo ""
echo "=== Check how Store is instantiated in production ==="
rg "new Store" code/ -A 3 --type ts --type js | head -50

Repository: storybookjs/storybook

Length of output: 6829

Add tests with projectId to cover namespaced storage paths.

The update call on line 162 correctly uses this.projectId to namespace the storage key. However, all existing tests instantiate the Store without a projectId, so they only test the fallback (non-namespaced) path. The production code in root.tsx always passes projectId: getAnonymousProjectId(), creating a coverage gap.

Consider adding test cases that pass a projectId to verify the namespaced storage behavior.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@code/core/src/manager-api/store.ts` around lines 160 - 162, Tests currently
don't cover namespaced storage paths because they instantiate Store without a
projectId; add unit tests that construct the Store with a non-empty projectId
(e.g., using the same constructor used in production) and assert that
update(storage, delta, this.projectId) writes to the namespaced key.
Specifically, create tests that call Store methods which trigger the update call
(referencing the update function and the Store/instance property
this.projectId), verifying both session and local persistence branches
(store.session and store.local) produce keys/values that include the provided
projectId namespace.

}

Expand Down