Skip to content

refactor: replace cross-tab chrome.storage sync with sinking #160

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

Open
aidenybai wants to merge 3 commits into
base: main
Choose a base branch
from
Open

refactor: replace cross-tab chrome.storage sync with sinking #160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d0ac364
refactor: replace cross-tab chrome.storage sync with sinking (Indexed…
aidenybai Feb 6, 2026
a1c739a
refactor: clean up sinking integration
aidenybai Feb 6, 2026
7fe1b47
Fix sinking cross-tab subscription race condition
cursoragent Feb 6, 2026
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
1 change: 1 addition & 0 deletions packages/react-grab/src/components/renderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ export const ReactGrabRenderer: Component<ReactGrabRendererProps> = (props) => {
onStateChange={props.onToolbarStateChange}
onSubscribeToStateChanges={props.onSubscribeToToolbarStateChanges}
onSelectHoverChange={props.onToolbarSelectHoverChange}
persistToolbarState={props.persistToolbarState}
/>
</Show>

Expand Down
5 changes: 3 additions & 2 deletions packages/react-grab/src/components/toolbar/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ interface ToolbarProps {
callback: (state: ToolbarState) => void,
) => () => void;
onSelectHoverChange?: (isHovered: boolean) => void;
persistToolbarState?: boolean;
}

export const Toolbar: Component<ToolbarProps> = (props) => {
Expand Down Expand Up @@ -757,12 +758,12 @@ export const Toolbar: Component<ToolbarProps> = (props) => {
};

const saveAndNotify = (state: ToolbarState) => {
saveToolbarState(state);
saveToolbarState(state, props.persistToolbarState);
props.onStateChange?.(state);
};

onMount(() => {
const savedState = loadToolbarState();
const savedState = loadToolbarState(props.persistToolbarState);
const rect = containerRef?.getBoundingClientRect();
const viewport = getVisualViewport();

Expand Down
11 changes: 9 additions & 2 deletions packages/react-grab/src/components/toolbar/state.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ export type SnapEdge = "top" | "bottom" | "left" | "right";

const STORAGE_KEY = "react-grab-toolbar-state";

export const loadToolbarState = (): ToolbarState | null => {
export const loadToolbarState = (
persistToolbarState = true,
): ToolbarState | null => {
if (!persistToolbarState) return null;
try {
const serializedToolbarState = localStorage.getItem(STORAGE_KEY);
if (!serializedToolbarState) return null;
Expand All @@ -28,7 +31,11 @@ export const loadToolbarState = (): ToolbarState | null => {
return null;
};

export const saveToolbarState = (state: ToolbarState): void => {
export const saveToolbarState = (
state: ToolbarState,
persistToolbarState = true,
): void => {
if (!persistToolbarState) return;
try {
localStorage.setItem(STORAGE_KEY, JSON.stringify(state));
} catch (error) {
Expand Down
19 changes: 13 additions & 6 deletions packages/react-grab/src/core/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,13 @@ export const init = (rawOptions?: Options): ReactGrabAPI => {
store.current.isPendingDismiss,
);

const savedToolbarState = loadToolbarState();
const shouldPersistToolbarState = () =>
pluginRegistry.store.options.persistToolbarState !== false;
const resolveToolbarState = (): ToolbarState | null =>
loadToolbarState(shouldPersistToolbarState()) ?? currentToolbarState();
const savedToolbarState = loadToolbarState(
initialOptions.persistToolbarState !== false,
);
Comment on lines +254 to +256
Copy link
Contributor

@pullfrog pullfrog bot Feb 6, 2026

Choose a reason for hiding this comment

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

Critical inconsistency: Initial load uses initialOptions.persistToolbarState but runtime checks use pluginRegistry.store.options.persistToolbarState. If setOptions({ persistToolbarState: false }) is called (which the extension does at line 93 of react-grab.ts), shouldPersistToolbarState() will correctly return false, but this initial load already happened with the wrong value. The extension explicitly passes persistToolbarState: false in initial options, so this specific code path works, but the inconsistent pattern is fragile—if initialization order changes or if someone calls setOptions before the toolbar mounts, this breaks.

const [isEnabled, setIsEnabled] = createSignal(
savedToolbarState?.enabled ?? true,
);
Expand Down Expand Up @@ -1514,14 +1520,14 @@ export const init = (rawOptions?: Options): ReactGrabAPI => {
const handleToggleEnabled = () => {
const newEnabled = !isEnabled();
setIsEnabled(newEnabled);
const currentState = loadToolbarState();
const currentState = resolveToolbarState();
const newState = {
edge: currentState?.edge ?? "bottom",
ratio: currentState?.ratio ?? 0.5,
collapsed: currentState?.collapsed ?? false,
enabled: newEnabled,
};
saveToolbarState(newState);
saveToolbarState(newState, shouldPersistToolbarState());
setCurrentToolbarState(newState);
toolbarStateChangeCallbacks.forEach((cb) => cb(newState));
if (!newEnabled) {
Expand Down Expand Up @@ -3395,6 +3401,7 @@ export const init = (rawOptions?: Options): ReactGrabAPI => {
};
}}
onToolbarSelectHoverChange={setIsToolbarSelectHovered}
persistToolbarState={shouldPersistToolbarState()}
contextMenuPosition={contextMenuPosition()}
contextMenuBounds={contextMenuBounds()}
contextMenuTagName={contextMenuTagName()}
Expand Down Expand Up @@ -3505,16 +3512,16 @@ export const init = (rawOptions?: Options): ReactGrabAPI => {
inToggleFeedbackPeriod = false;
}
},
getToolbarState: () => loadToolbarState(),
getToolbarState: () => resolveToolbarState(),
setToolbarState: (state: Partial<ToolbarState>) => {
const currentState = loadToolbarState();
const currentState = resolveToolbarState();
const newState = {
edge: state.edge ?? currentState?.edge ?? "bottom",
ratio: state.ratio ?? currentState?.ratio ?? 0.5,
collapsed: state.collapsed ?? currentState?.collapsed ?? false,
enabled: state.enabled ?? currentState?.enabled ?? true,
};
saveToolbarState(newState);
saveToolbarState(newState, shouldPersistToolbarState());
setCurrentToolbarState(newState);
if (state.enabled !== undefined && state.enabled !== isEnabled()) {
setIsEnabled(state.enabled);
Expand Down
2 changes: 2 additions & 0 deletions packages/react-grab/src/core/plugin-registry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ interface OptionsState {
activationKey: ActivationKey | undefined;
getContent: ((elements: Element[]) => Promise<string> | string) | undefined;
freezeReactUpdates: boolean;
persistToolbarState: boolean;
}

const DEFAULT_OPTIONS: OptionsState = {
Expand All @@ -45,6 +46,7 @@ const DEFAULT_OPTIONS: OptionsState = {
activationKey: undefined,
getContent: undefined,
freezeReactUpdates: true,
persistToolbarState: true,
};

interface PluginStoreState {
Expand Down
7 changes: 7 additions & 0 deletions packages/react-grab/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,12 @@ export interface Options {
* @default true
*/
freezeReactUpdates?: boolean;
/**
* Whether to persist toolbar state (position, collapsed, enabled) to localStorage.
* Set to false when an external consumer (e.g. web extension) handles persistence.
* @default true
*/
persistToolbarState?: boolean;
}

export interface SettableOptions extends Options {
Expand Down Expand Up @@ -505,6 +511,7 @@ export interface ReactGrabRendererProps {
callback: (state: ToolbarState) => void,
) => () => void;
onToolbarSelectHoverChange?: (isHovered: boolean) => void;
persistToolbarState?: boolean;
contextMenuPosition?: { x: number; y: number } | null;
contextMenuBounds?: OverlayBounds | null;
contextMenuTagName?: string;
Expand Down
1 change: 1 addition & 0 deletions packages/web-extension/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"react": "19.1.2",
"react-dom": "19.1.2",
"react-grab": "workspace:*",
"sinking": "^0.0.6",
"turndown": "^7.2.0",
"webextension-polyfill": "^0.12.0"
},
Expand Down
40 changes: 13 additions & 27 deletions packages/web-extension/src/content/bridge.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,6 @@ chrome.storage.onChanged.addListener((changes) => {
"*",
);
}

if (changes.react_grab_toolbar_state) {
const newState = changes.react_grab_toolbar_state.newValue;
if (newState) {
window.postMessage(
{ type: "__REACT_GRAB_TOOLBAR_STATE_CHANGE__", state: newState },
"*",
);
}
}
});

chrome.runtime.onMessage.addListener((message, _sender, sendResponse) => {
Expand All @@ -38,25 +28,21 @@ chrome.runtime.onMessage.addListener((message, _sender, sendResponse) => {

window.addEventListener("message", (event) => {
if (event.data?.type === "__REACT_GRAB_QUERY_STATE__") {
chrome.storage.local.get(
["react_grab_enabled", "react_grab_toolbar_state"],
(result) => {
const enabled = result.react_grab_enabled ?? true;
const toolbarState = result.react_grab_toolbar_state ?? null;
chrome.storage.local.get(["react_grab_enabled"], (result) => {
const enabled = result.react_grab_enabled ?? true;

window.postMessage(
{
type: "__REACT_GRAB_STATE_RESPONSE__",
enabled,
toolbarState,
},
"*",
);
},
);
window.postMessage(
{
type: "__REACT_GRAB_STATE_RESPONSE__",
enabled,
},
"*",
);
});
}

if (event.data?.type === "__REACT_GRAB_TOOLBAR_STATE_SAVE__") {
chrome.storage.local.set({ react_grab_toolbar_state: event.data.state });
if (event.data?.type === "__REACT_GRAB_GET_WORKER_URL__") {
const workerUrl = chrome.runtime.getURL("src/worker.ts");
Copy link
Contributor

@pullfrog pullfrog bot Feb 6, 2026

Choose a reason for hiding this comment

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

Incorrect path: Similar to manifest.json issue—this references the source TypeScript file instead of the compiled JavaScript. chrome.runtime.getURL('src/worker.ts') will return a URL to the .ts file which won't exist in the built extension. Should be 'src/worker.js' or wherever the built worker ends up.

window.postMessage({ type: "__REACT_GRAB_WORKER_URL__", workerUrl }, "*");
}
});
Loading
Loading