Skip to content

Commit 4befe38

Browse files
committed
refactor(ui): derive three values instead of storing or memoizing them
- import-modal: browserId and profileId were state corrected by two effects when a reload dropped the selection. That commits and paints one frame in which the profile still belongs to the previously selected browser — and Import is enabled during it, submitting via a `profiles.find` that searches every browser's profiles. Both now fall back during render, and `selected` searches only the current browser's profiles. Covered by the existing 'never leaves a profile selected that belongs to another browser' test. - workflow.tsx: isWorkflowEmpty was a second useMemo over the same [blocks] dep computing exactly !hasBlocks, allocating its own Object.keys array. Both feed primitives, so neither memo bought identity stability. - thinking-loader: an effect seeding cycleVariant whenever variant is defined, which `shown = variant ?? cycleVariant` can never read. On the one transition where cycleVariant becomes visible (variant going undefined) the cycling effect assigns it in every branch — settle, reduced-motion, and tick — in the same flush, so the seed was never observable.
1 parent 9cd4bbf commit 4befe38

3 files changed

Lines changed: 20 additions & 26 deletions

File tree

apps/sim/app/workspace/[workspaceId]/settings/components/browser/components/import-modal/import-modal.tsx

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use client'
22

3-
import { useEffect, useMemo, useState } from 'react'
3+
import { useMemo, useState } from 'react'
44
import type { BrowserImportProfile } from '@sim/desktop-bridge'
55
import {
66
ChipModal,
@@ -39,29 +39,29 @@ function browserOptions(profiles: BrowserImportProfile[]) {
3939
*/
4040
export function ImportModal({ open, onOpenChange, profiles, pending, onImport }: ImportModalProps) {
4141
const browsers = useMemo(() => browserOptions(profiles), [profiles])
42-
const [browserId, setBrowserId] = useState(browsers[0]?.value ?? '')
42+
const [pickedBrowserId, setPickedBrowserId] = useState(browsers[0]?.value ?? '')
43+
44+
/**
45+
* A reload can drop the browser or profile that was picked. Falling back here
46+
* rather than correcting in an effect matters: the effect form commits and
47+
* paints one frame in which the profile still belongs to the previously
48+
* selected browser, and Import is enabled during it.
49+
*/
50+
const browserId = browsers.some((browser) => browser.value === pickedBrowserId)
51+
? pickedBrowserId
52+
: (browsers[0]?.value ?? '')
4353

4454
const profilesForBrowser = useMemo(
4555
() => profiles.filter((profile) => profile.browserId === browserId),
4656
[browserId, profiles]
4757
)
48-
const [profileId, setProfileId] = useState(profilesForBrowser[0]?.id ?? '')
49-
50-
// Keep the selection valid as the browser changes or the list reloads,
51-
// rather than leaving a profile selected that belongs to another browser.
52-
useEffect(() => {
53-
if (!profilesForBrowser.some((profile) => profile.id === profileId)) {
54-
setProfileId(profilesForBrowser[0]?.id ?? '')
55-
}
56-
}, [profileId, profilesForBrowser])
58+
const [pickedProfileId, setPickedProfileId] = useState(profilesForBrowser[0]?.id ?? '')
5759

58-
useEffect(() => {
59-
if (!browsers.some((browser) => browser.value === browserId)) {
60-
setBrowserId(browsers[0]?.value ?? '')
61-
}
62-
}, [browserId, browsers])
60+
const profileId = profilesForBrowser.some((profile) => profile.id === pickedProfileId)
61+
? pickedProfileId
62+
: (profilesForBrowser[0]?.id ?? '')
6363

64-
const selected = profiles.find((profile) => profile.id === profileId) ?? null
64+
const selected = profilesForBrowser.find((profile) => profile.id === profileId) ?? null
6565

6666
return (
6767
<ChipModal open={open} onOpenChange={onOpenChange} srTitle='Import from your browser'>
@@ -79,7 +79,7 @@ export function ImportModal({ open, onOpenChange, profiles, pending, onImport }:
7979
title='Browser'
8080
options={browsers}
8181
value={browserId}
82-
onChange={setBrowserId}
82+
onChange={setPickedBrowserId}
8383
placeholder='Select a browser'
8484
align='start'
8585
disabled={pending || browsers.length === 0}
@@ -92,7 +92,7 @@ export function ImportModal({ open, onOpenChange, profiles, pending, onImport }:
9292
label: profile.profileLabel,
9393
}))}
9494
value={profileId}
95-
onChange={setProfileId}
95+
onChange={setPickedProfileId}
9696
placeholder='Select a profile'
9797
align='start'
9898
disabled={pending || profilesForBrowser.length === 0}

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,7 @@ const WorkflowContent = React.memo(
574574
embedded,
575575
})
576576

577-
const isWorkflowEmpty = useMemo(() => Object.keys(blocks).length === 0, [blocks])
577+
const isWorkflowEmpty = !hasBlocks
578578

579579
/** Handles OAuth connect events dispatched by Copilot tools. */
580580
useEffect(() => {

apps/sim/components/ui/thinking-loader.tsx

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -320,12 +320,6 @@ export function ThinkingLoader({
320320
const cycling = variant === undefined
321321
const [retainMorphStages, setRetainMorphStages] = useState(cycling)
322322

323-
useEffect(() => {
324-
if (variant !== undefined) {
325-
setCycleVariant(variant)
326-
}
327-
}, [variant])
328-
329323
useEffect(() => {
330324
if (!cycling) return
331325
// Settle: stop the cycle and melt to the terminal orb (goo handles the morph).

0 commit comments

Comments
 (0)