Conversation
The current workspace was tracked in four places that fought each other (React state, the `org` query param, the selected app's owner, and localStorage), which caused races: switching orgs snapped back to personal, switching to personal snapped back to the org, and the org settings gear flashed then redirected to /dash. Derive currentWorkspaceId from the `org` query param instead, so there's no lag between navigation and state. localStorage is now only a cold-start default (restore the last workspace on a bare /dash), read once and consumed on an explicit switch. Navigation flows through a single goToApp helper that always carries `org` forward. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace goToApp (which coupled query-building with the push/replace decision) with a pure dashQueryParams() builder. Callers hand the result to router.replace/push directly, keeping the navigation explicit while still guaranteeing `org` rides along. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR migrates workspace and organization state management from localStorage and manual router pushes to a centralized, context-driven approach. MainDashLayout now manages workspace persistence and derives the active workspace from URL query params. Components delegate workspace selection to the context via ChangesWorkspace State Management & Query-Driven Navigation
Sequence DiagramsequenceDiagram
participant Component as ProfilePanel/<br/>BackToAppsButton
participant setWorkspace as setWorkspace<br/>(from context)
participant Context as MainDashLayout<br/>DashFetchProvider
participant Router as Next.js Router
participant Storage as localStorage
Component->>setWorkspace: setWorkspace(workspaceId, { replace? })
setWorkspace->>Router: router.push or router.replace<br/>with org query param
Router->>Context: route updates, router.query.org changes
Context->>Context: getWorkspaceId prioritizes<br/>router.query.org
Context->>Storage: save currentWorkspaceId<br/>to WORKSPACE_STORAGE_KEY
Context-->>Component: context updates, component re-renders
🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Contributor
|
View Vercel preview at instant-www-js-roar2-jsv.vercel.app. |
![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: 3
🧹 Nitpick comments (1)
client/www/components/dash/ProfilePanel.tsx (1)
128-129: ⚡ Quick winPrefer query-object
hreffor org settings navigation.Using a string URL here can mis-handle special characters in
org.id; object-formhrefkeeps query serialization safe and consistent with Next.js routing.Suggested fix
- <Link - href={`/dash/org?org=${org.id}`} - onClick={() => close()} - > + <Link + href={{ pathname: '/dash/org', query: { org: org.id } }} + onClick={() => close()} + >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/www/components/dash/ProfilePanel.tsx` around lines 128 - 129, Replace the string-form href used in ProfilePanel (the href={`/dash/org?org=${org.id}`} prop) with Next.js object-form href so query serialization is safe: pass an object with pathname '/dash/org' and query containing org: org.id on the same element (keep the existing onClick={() => close()} unchanged); update the Link/anchor href prop in ProfilePanel.tsx accordingly to use the object-form href.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/www/components/dash/BackToAppsButton.tsx`:
- Around line 8-16: router.query.org is cast unsafely; normalize it before use
and switch Link to object-form href so Next.js encodes queries safely. In the
BackToAppsButton component, derive a string org by checking router.query.org
with Array.isArray and using the first element when it's an array (or undefined
otherwise), then pass href as an object to Link (e.g., pathname '/dash' and
query set to { org } only when org is defined) instead of interpolating a
string; keep the createPortal and element lookup unchanged.
In `@client/www/components/dash/MainDashLayout.tsx`:
- Around line 85-91: The current effect unconditionally redirects to personal
whenever workspace.error is truthy; change it to only redirect for expected
org-access failures by checking the error kind/status on workspace.error (e.g.,
404 Not Found or 403 Unauthorized / an "not found"/"unauthorized" error
indicator) before calling setWorkspace('personal', { replace: true }); otherwise
do not swallow the error and let the component surface workspace.error (and keep
the existing dashResult.error behavior for personal fetches). Update the
useEffect that references workspace.error to perform the conditional check and
only call setWorkspace for those specific error cases.
In `@client/www/pages/dash/index.tsx`:
- Around line 1044-1049: AppCombobox's pinnedIds state is initialized from
getPinnedAppIds(workspaceId) but isn't reset when workspaceId changes; update
the component to reset pinnedIds whenever workspaceId changes by either: adding
a key={workspaceId} to the component that renders AppCombobox so it remounts on
workspace switch, or (preferably) add a useEffect inside AppCombobox that
watches workspaceId (from useFetchedDash().data.currentWorkspaceId) and calls
setPinnedIds(getPinnedAppIds(workspaceId)) to reinitialize pinned ordering;
reference pinnedIds, setPinnedIds, getPinnedAppIds, workspaceId, and AppCombobox
when making the change.
---
Nitpick comments:
In `@client/www/components/dash/ProfilePanel.tsx`:
- Around line 128-129: Replace the string-form href used in ProfilePanel (the
href={`/dash/org?org=${org.id}`} prop) with Next.js object-form href so query
serialization is safe: pass an object with pathname '/dash/org' and query
containing org: org.id on the same element (keep the existing onClick={() =>
close()} unchanged); update the Link/anchor href prop in ProfilePanel.tsx
accordingly to use the object-form href.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 94eb1ac7-c715-4d37-b4f4-235a6c881d9f
📒 Files selected for processing (4)
client/www/components/dash/BackToAppsButton.tsxclient/www/components/dash/MainDashLayout.tsxclient/www/components/dash/ProfilePanel.tsxclient/www/pages/dash/index.tsx
Comment on lines
+85
to
91
| // If we can't load the org (e.g. we were removed from it) fall back to | ||
| // the personal account. | ||
| useEffect(() => { | ||
| if (workspace.error) { | ||
| setWorkspace('personal'); | ||
| setWorkspace('personal', { replace: true }); | ||
| } | ||
| }, [workspace.error]); |
Contributor
There was a problem hiding this comment.
Only redirect to personal for expected org-access failures.
workspace.error now always rewrites the URL to personal, but the context still exposes only dashResult.error. A transient workspace fetch failure on the personal workspace will never hit the error UI and will sit in the loading state instead. Please gate this fallback to the specific not-found/unauthorized org case and otherwise surface workspace.error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/www/components/dash/MainDashLayout.tsx` around lines 85 - 91, The
current effect unconditionally redirects to personal whenever workspace.error is
truthy; change it to only redirect for expected org-access failures by checking
the error kind/status on workspace.error (e.g., 404 Not Found or 403
Unauthorized / an "not found"/"unauthorized" error indicator) before calling
setWorkspace('personal', { replace: true }); otherwise do not swallow the error
and let the component surface workspace.error (and keep the existing
dashResult.error behavior for personal fetches). Update the useEffect that
references workspace.error to perform the conditional check and only call
setWorkspace for those specific error cases.
Contributor
Author
There was a problem hiding this comment.
I think it's fine to do this, this is how it was already in the code.
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Resolve the review comment
gh pr review-comment 2712 --repo instantdb/instant resolve r3314251687 2>/dev/null || \
gh api graphql -f query='
mutation {
resolveReviewThread(input: {threadId: "PRRT_kwDOMSxPXs5mcbDH"}) {
thread {
isResolved
}
}
}' 2>/dev/null || echo "Could not auto-resolve via API"Length of output: 628
@stopachka, acknowledged — the pre-existing behavior is intentional and this PR only adds { replace: true } to the existing fallback. I wasn't able to auto-resolve the comment via the API, but feel free to resolve it manually. Sorry for the noise!
🧠 Learnings used
Learnt from: dwwoelfel
Repo: instantdb/instant PR: 2618
File: client/www/components/dash/WebhookEvents.tsx:85-86
Timestamp: 2026-05-13T21:47:49.260Z
Learning: In this repo, ISN (invalidation sequence number) values have a settled encoding that is designed to be safely represented in URL path segments even when the encoded form can include "/". Therefore, during code review, do not flag raw `isn` interpolation into URL path segments (e.g., `/.../${isn}...`) as a missing `encodeURIComponent` issue in `.tsx` files. Only require additional URL encoding if the interpolation is not using ISN’s settled encoding (i.e., it’s a different value/type than ISN or otherwise not part of the ISN URL-safe scheme).
Learnt from: stopachka
Repo: instantdb/instant PR: 2670
File: client/www/app/getadb/make/generateMarkdown.ts:0-0
Timestamp: 2026-05-15T00:01:01.658Z
Learning: For code reviewed in client/www (Instant’s Next.js app), remember that `output: 'standalone'` in `next.config.js` is only used when `NEXT_PUBLIC_SELF_HOSTED === 'true'`. On the primary Vercel deployment (where standalone is not enabled), runtime reads of co-located static assets (e.g., using `fs.readFile(...)` with `process.cwd()` to reference files alongside the repo) are accessible without requiring `outputFileTracingIncludes`. Only apply concerns about missing standalone output-file tracing / asset bundling for the self-hosted standalone path (the `NEXT_PUBLIC_SELF_HOSTED === 'true'` case).
Comment on lines
+1044
to
1049
| const workspaceId = useFetchedDash().data.currentWorkspaceId; | ||
| const currentApp = apps.find((a) => a.id === appId) || null; | ||
| const [appQuery, setAppQuery] = useState(''); | ||
| const [pinnedIds, setPinnedIds] = useState(() => | ||
| getPinnedAppIds(localStorage.getItem('workspace')), | ||
| getPinnedAppIds(workspaceId), | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In React 18, does the initializer passed to useState(() => ...) run again when a prop or context value used inside it changes after the component has already mounted?
💡 Result:
No, the initializer function passed to useState does not run again when a prop or context value changes after the component has mounted [1][2][3]. In React, the initializer function (often called "lazy initialization") is executed only once, during the initial render, to determine the initial state [1][3][4]. After the component has mounted, React ignores this initializer function during subsequent re-renders [1][2][3]. If you need to update the state in response to changes in props or context, you should use the useEffect hook to synchronize your state with those values [5][6][2]. Alternatively, if you want the state to reset entirely when a prop changes, you can use the key prop to force the component to re-mount [6][7].
Citations:
- 1: https://react.dev/reference/react/useState
- 2: https://javascript.plainenglish.io/react-anti-patterns-using-props-as-initial-state-in-usestate-25f1f3906a3d
- 3: https://uk.react.dev/reference/react/useState
- 4: https://kentcdodds.com/blog/use-state-lazy-initialization-and-function-updates
- 5: https://stackoverflow.com/questions/54865764/react-usestate-does-not-reload-state-from-props
- 6: Prop updates updating state in hooks facebook/react#14830
- 7: https://react.dev/learn/preserving-and-resetting-state
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="client/www/pages/dash/index.tsx"
echo "=== Context around lines 1020-1080 ==="
nl -ba "$FILE" | sed -n '1010,1090p'
echo
echo "=== Search in file for pinnedIds / setPinnedIds / getPinnedAppIds / workspaceId sync ==="
rg -n "pinnedIds|setPinnedIds|getPinnedAppIds|currentWorkspaceId|workspaceId" "$FILE" || trueRepository: instantdb/instant
Length of output: 161
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="client/www/pages/dash/index.tsx"
echo "=== File existence ==="
test -f "$FILE" && echo "FOUND: $FILE" || (echo "MISSING: $FILE" && exit 1)
echo
echo "=== Context around lines 1010-1090 ==="
awk 'NR>=1010 && NR<=1090 {printf "%d:%s\n", NR, $0}' "$FILE"
echo
echo "=== Search in file for pinnedIds / setPinnedIds / getPinnedAppIds / currentWorkspaceId / workspaceId ==="
rg -n "pinnedIds|setPinnedIds|getPinnedAppIds|currentWorkspaceId|workspaceId" "$FILE" || trueRepository: instantdb/instant
Length of output: 4427
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="client/www/pages/dash/index.tsx"
echo "=== Find AppCombobox usage ==="
rg -n "<AppCombobox|AppCombobox" "$FILE"
echo
echo "=== Inspect surrounding render where AppCombobox is used ==="
# show a small window around each match (best effort)
rg -n "<AppCombobox" "$FILE" | cut -d: -f1 | head -n 20 | while read -r line; do
start=$((line-40))
end=$((line+40))
echo "--- Window around line $line ($start-$end) ---"
awk -v s="$start" -v e="$end" 'NR>=s && NR<=e {printf "%d:%s\n", NR, $0}' "$FILE"
doneRepository: instantdb/instant
Length of output: 2963
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="client/www/pages/dash/index.tsx"
echo "=== Find Nav usage ==="
rg -n "<Nav\\b" "$FILE"
echo
echo "=== Inspect windows around Nav render sites (best effort) ==="
rg -n "<Nav\\b" "$FILE" | cut -d: -f1 | head -n 20 | while read -r line; do
start=$((line-60))
end=$((line+60))
echo "--- Window around line $line ($start-$end) ---"
awk -v s="$start" -v e="$end" 'NR>=s && NR<=e {printf "%d:%s\n", NR, $0}' "$FILE"
doneRepository: instantdb/instant
Length of output: 4249
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="client/www/pages/dash/index.tsx"
echo "=== Check top-of-file React imports for useEffect ==="
head -n 80 "$FILE"
echo
echo "=== Search for appId source / updates related to org/workspace switching ==="
rg -n "appId\\b|router\\.query\\.org|currentWorkspaceId|workspaceId" "$FILE" | head -n 80
echo
echo "=== Search for key props related to workspace/org (e.g., key={workspaceId} / org) ==="
rg -n "key=\\{[^}]*workspace|key=\\{[^}]*org|workspaceId\\}" "$FILE" || trueRepository: instantdb/instant
Length of output: 5988
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="client/www/pages/dash/index.tsx"
echo "=== Check top-of-file React imports for useEffect ==="
head -n 120 "$FILE"
echo
echo "=== Search for appId source / updates related to org/workspace switching ==="
rg -n "router\\.query\\.(org|workspace)|currentWorkspaceId|workspaceId|appId\\b" "$FILE" | head -n 120
echo
echo "=== Search for key props related to workspace/org ==="
rg -n "key=\\{[^}]*workspace|key=\\{[^}]*org" "$FILE" || trueRepository: instantdb/instant
Length of output: 7034
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the pinnedApps implementation file
fd -a "pinnedApps" -p . | head -n 50
echo "=== Search for getPinnedAppIds definition ==="
fd -a "*pinnedApps*" client -p . 2>/dev/null | head -n 50
rg -n "function getPinnedAppIds|const getPinnedAppIds|getPinnedAppIds\\s*=" -S . --glob='*.ts' --glob='*.tsx' --glob='*.js' --glob='*.jsx'
# Print the matching file(s)
FILE=$(rg -l "getPinnedAppIds" . --glob='pinnedApps*' --glob='*.ts' --glob='*.tsx' | head -n 1)
echo
echo "=== Showing context from $FILE ==="
[ -n "${FILE:-}" ] && sed -n '1,220p' "$FILE" || trueRepository: instantdb/instant
Length of output: 155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="client/www/lib/pinnedApps.ts"
echo "=== File head ==="
head -n 120 "$FILE"
echo
echo "=== Search for getPinnedAppIds export in pinnedApps.ts ==="
rg -n "getPinnedAppIds|togglePinnedApp" "$FILE"
echo
echo "=== Show around getPinnedAppIds in pinnedApps.ts ==="
LINE=$(rg -n "getPinnedAppIds" "$FILE" | head -n 1 | cut -d: -f1)
echo "First match line: $LINE"
start=$((LINE-20))
end=$((LINE+60))
awk -v s="$start" -v e="$end" 'NR>=s && NR<=e {printf "%d:%s\n", NR, $0}' "$FILE"Repository: instantdb/instant
Length of output: 1610
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="client/www/pages/dash/index.tsx"
echo "=== Context around pinned star toggle (approx 1110-1150) ==="
awk 'NR>=1100 && NR<=1160 {printf "%d:%s\n", NR, $0}' "$FILE"
echo
echo "=== Inspect effect around dependency [dashResponse.data?.currentWorkspaceId, appId, router.query.org] (approx 420-470) ==="
awk 'NR>=400 && NR<=480 {printf "%d:%s\n", NR, $0}' "$FILE"Repository: instantdb/instant
Length of output: 5326
Reset pinned app state when the workspace changes.
In AppCombobox, pinnedIds is set with useState(() => getPinnedAppIds(workspaceId)), but the component isn’t remounted on org/personal switch (no key on workspaceId), so it keeps the previous workspace’s pinned ordering until the user toggles a star.
♻️ Proposed fix
const [pinnedIds, setPinnedIds] = useState(() =>
getPinnedAppIds(workspaceId),
);
+ useEffect(() => {
+ setPinnedIds(getPinnedAppIds(workspaceId));
+ }, [workspaceId]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const workspaceId = useFetchedDash().data.currentWorkspaceId; | |
| const currentApp = apps.find((a) => a.id === appId) || null; | |
| const [appQuery, setAppQuery] = useState(''); | |
| const [pinnedIds, setPinnedIds] = useState(() => | |
| getPinnedAppIds(localStorage.getItem('workspace')), | |
| getPinnedAppIds(workspaceId), | |
| ); | |
| const workspaceId = useFetchedDash().data.currentWorkspaceId; | |
| const currentApp = apps.find((a) => a.id === appId) || null; | |
| const [appQuery, setAppQuery] = useState(''); | |
| const [pinnedIds, setPinnedIds] = useState(() => | |
| getPinnedAppIds(workspaceId), | |
| ); | |
| useEffect(() => { | |
| setPinnedIds(getPinnedAppIds(workspaceId)); | |
| }, [workspaceId]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/www/pages/dash/index.tsx` around lines 1044 - 1049, AppCombobox's
pinnedIds state is initialized from getPinnedAppIds(workspaceId) but isn't reset
when workspaceId changes; update the component to reset pinnedIds whenever
workspaceId changes by either: adding a key={workspaceId} to the component that
renders AppCombobox so it remounts on workspace switch, or (preferably) add a
useEffect inside AppCombobox that watches workspaceId (from
useFetchedDash().data.currentWorkspaceId) and calls
setPinnedIds(getPinnedAppIds(workspaceId)) to reinitialize pinned ordering;
reference pinnedIds, setPinnedIds, getPinnedAppIds, workspaceId, and AppCombobox
when making the change.
Contributor
Screen.Recording.2026-05-28.at.11.22.30.AM.movThe path for switching to an org that has no apps, and then back to one with apps is really awkward. The "never mind" button on my personal account should just take me to the normal dash screen. |
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
2 participants
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.
Problem
I noticed that clicking the 'Gear' icon in our org dropdown would often fail.
I would try to go from 'Personal' to a 'Particular Org', and after a flash I would just get back to the exact app I was looking at.
Reason for problem
We tracked the current workspace in a few different places:
currentWorkspaceId)orgURL paramThen we had a few useEffects that tried to reconcile them. For example:
When I clicked the 'Gear' icon, it:
That set up a race:
The same kind of bug could happen with clicking a different org:
Solution
I made the URL the single source of truth for the workspace.
currentWorkspaceIdis now read straight from the org param.setWorkspacejust updates the URL@dwwoelfel @nezaj @drew-harris