Update dependencies #133
Update dependencies #133
Conversation
Bumps [eslint-plugin-react-hooks](https://github.com/facebook/react/tree/HEAD/packages/eslint-plugin-react-hooks) from 5.2.0 to 7.0.1. - [Release notes](https://github.com/facebook/react/releases) - [Changelog](https://github.com/facebook/react/blob/main/packages/eslint-plugin-react-hooks/CHANGELOG.md) - [Commits](https://github.com/facebook/react/commits/HEAD/packages/eslint-plugin-react-hooks) --- updated-dependencies: - dependency-name: eslint-plugin-react-hooks dependency-version: 7.0.1 dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [@vitest/coverage-v8](https://github.com/vitest-dev/vitest/tree/HEAD/packages/coverage-v8) from 3.2.4 to 4.0.8. - [Release notes](https://github.com/vitest-dev/vitest/releases) - [Commits](https://github.com/vitest-dev/vitest/commits/v4.0.8/packages/coverage-v8) --- updated-dependencies: - dependency-name: "@vitest/coverage-v8" dependency-version: 4.0.8 dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [globals](https://github.com/sindresorhus/globals) from 16.4.0 to 16.5.0. - [Release notes](https://github.com/sindresorhus/globals/releases) - [Commits](sindresorhus/globals@v16.4.0...v16.5.0) --- updated-dependencies: - dependency-name: globals dependency-version: 16.5.0 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [zod](https://github.com/colinhacks/zod) from 4.1.11 to 4.1.12. - [Release notes](https://github.com/colinhacks/zod/releases) - [Commits](colinhacks/zod@v4.1.11...v4.1.12) --- updated-dependencies: - dependency-name: zod dependency-version: 4.1.12 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [jsdom](https://github.com/jsdom/jsdom) from 26.1.0 to 27.1.0. - [Release notes](https://github.com/jsdom/jsdom/releases) - [Changelog](https://github.com/jsdom/jsdom/blob/main/Changelog.md) - [Commits](jsdom/jsdom@26.1.0...27.1.0) --- updated-dependencies: - dependency-name: jsdom dependency-version: 27.1.0 dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
…lugin-react-hooks-7.0.1' into td-deps
…overage-v8-4.0.8' into td-deps
…16.5.0' into td-deps
….1.0' into td-deps
❌ Deploy Preview for jam-state-viewer failed. Why did it fail? →
|
📝 WalkthroughWalkthroughThe PR updates dependencies across the project, refactors test mocks from arrow functions to traditional function expressions, adds ESLint suppressions for state updates within effects and memoization contexts, relocates a state exposure effect, mocks UI components in tests, and introduces async content loading in UploadScreen with loading state integration. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as UploadScreen UI
participant Modal as ExamplesModal
participant Handler as handleExampleLoad
participant Loader as Content Loader
participant State as State & Validation
User->>UI: Click example option
UI->>Modal: Open modal
User->>Modal: Select example file
Modal->>Handler: onSelect(content: async factory)
Note over Handler: isLoading = true, disable buttons
Handler->>Loader: Invoke content() to load async
Loader->>Handler: Return Promise<string>
Note over Handler: await content()
Handler->>State: validateJson(content string)
Note over State: Validate JSON
Handler->>State: setEditorContent(content)
Handler->>UI: isLoading = false, re-enable buttons
Note over UI: Show "Loading example…" during<br/>async operation, hide browse button
UI->>User: Display loaded content
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/trie/TrieView.tsx (1)
48-64: Critical: Move state updates from useMemo to useEffect.Setting state inside
useMemois a React anti-pattern that violates the rules of hooks.useMemoexecutes during the render phase and should only compute derived values, not trigger side effects like state updates. This can cause unexpected re-renders, infinite loops, or inconsistent state.Apply this diff to properly separate computation from side effects:
const trie = useMemo(() => { if (rows.length === 0) { return; } - // eslint-disable-next-line react-hooks/set-state-in-render - setError(null); try { return getTrie(rows); } catch (error: unknown) { - if (error instanceof Error) { - // eslint-disable-next-line react-hooks/set-state-in-render - setError(error.message); - } - return null; + return { error: error instanceof Error ? error.message : 'Unknown error' }; } }, [rows]); + useEffect(() => { + if (!trie) { + setError(null); + } else if (typeof trie === 'object' && 'error' in trie) { + setError(trie.error); + } else { + setError(null); + } + }, [trie]);Alternatively, refactor to avoid error state entirely:
- const [error, setError] = useState<string | null>(null); const trie = useMemo(() => { if (rows.length === 0) { - return; + return null; } try { return getTrie(rows); } catch (error: unknown) { - if (error instanceof Error) { - setError(error.message); - } - return null; + return { error: error instanceof Error ? error.message : 'Unknown error' }; } }, [rows]); + const error = trie && typeof trie === 'object' && 'error' in trie ? trie.error : null;src/components/UploadScreen.tsx (1)
135-159: Add error handling for async content loading failures.If
exampleContent()throws (e.g., import failure), thecontentvariable remains an empty string and validation proceeds with'', potentially showing confusing error messages to the user.Apply this diff to handle errors properly:
const handleExampleLoad = useCallback(async (exampleContent: () => Promise<string>) => { clearUpload(); setIsLoading(true); - let content = ''; try { - content = await exampleContent(); + const content = await exampleContent(); + const validation = validateJsonContent(content); + + const newUploadState = { + file: null, + content: validation.content, + error: validation.error, + isValidJson: validation.isValid, + format: validation.format, + formatDescription: validation.formatDescription, + availableStates: validation.availableStates, + selectedState: validation.availableStates?.includes('diff') ? 'diff' : validation.availableStates?.[0], + }; + + handleUploadStateWithStorage(newUploadState, validation); + } catch (error) { + // Handle import or loading failures + const errorMessage = error instanceof Error ? error.message : 'Failed to load example content'; + handleUploadStateWithStorage({ + file: null, + content: '', + error: errorMessage, + isValidJson: false, + format: 'unknown', + formatDescription: '', + }); } finally { setIsLoading(false); } - - const validation = validateJsonContent(content); - - const newUploadState = { - file: null, - content: validation.content, - error: validation.error, - isValidJson: validation.isValid, - format: validation.format, - formatDescription: validation.formatDescription, - availableStates: validation.availableStates, - selectedState: validation.availableStates?.includes('diff') ? 'diff' : validation.availableStates?.[0], - }; - - handleUploadStateWithStorage(newUploadState, validation); }, [clearUpload, handleUploadStateWithStorage]);
🧹 Nitpick comments (4)
src/components/SettingsDialog.tsx (1)
25-31: Consider using a key to reset dialog state instead of effect-based state updates.While resetting form state on dialog open is a legitimate pattern, the lint suppression suggests an opportunity for a cleaner approach using component remounting.
Apply this diff to use a key-based reset:
useEffect(() => { if (!isOpen) return; - // eslint-disable-next-line react-hooks/set-state-in-effect setSelectedGpVersion(utils.CURRENT_VERSION as unknown as string); - // eslint-disable-next-line react-hooks/set-state-in-effect setSelectedSuite(utils.CURRENT_SUITE as unknown as string); }, [isOpen]); if (!isOpen) return null;Alternative approach (wrap dialog content in a component with key):
{isOpen && <SettingsDialogContent key={String(isOpen)} onClose={onClose} onApply={handleApply} />}This forces React to create a fresh component instance with initial state each time the dialog opens.
src/trie/components/trie/index.tsx (1)
117-124: Removeelementsfrom effect dependencies to prevent potential loops.While the
deepEqualcheck guards against infinite loops, includingelementsin the dependency array of an effect that updateselementsis fragile. IfdeepEqualever fails to detect equality correctly, this will cause an infinite loop.Apply this diff to remove the circular dependency:
useEffect(() => { const graphElements = buildCytoscapeGraphData(treeData, containerSize.width, containerSize.height); // Perform deep equal to make sure the values are different and prevent trie re-rendering. It's more expensive operation if (!deepEqual(graphElements, elements)) { - // eslint-disable-next-line react-hooks/set-state-in-effect setElements(graphElements); } - }, [containerSize.height, containerSize.width, elements, treeData]); + }, [containerSize.height, containerSize.width, treeData]);The
deepEqualcheck already prevents updates when elements haven't changed, so the dependency is redundant and potentially problematic.src/components/JsonEditorDialog.tsx (1)
69-76: Consider alternatives to setting state within effects.Setting state inside
useEffectcan lead to unnecessary re-renders and is generally discouraged (hence the ESLint rule). Consider these alternatives:
- Derive state from props: If
editorContentalways mirrorsinitialContentwhen the dialog opens, you might not need separate state.- Use a key prop: Reset component state by adding
key={isOpen ? initialContent : 'closed'}to force remount.- Set state in event handlers: Move
setEditorContent(initialContent)toonCloseor when the dialog opens via user interaction.Example using derived state:
- const [editorContent, setEditorContent] = useState(initialContent); + const [editorContent, setEditorContent] = useState(''); + + // Derive editor content from initialContent when dialog is open + const effectiveContent = isOpen && !editorContent ? initialContent : editorContent; - // Update editor content when initialContent changes - useEffect(() => { - if (isOpen) { - // eslint-disable-next-line react-hooks/set-state-in-effect - setEditorContent(initialContent); - validateJson(initialContent); - } - }, [isOpen, initialContent]); + // Validate when dialog opens + useEffect(() => { + if (isOpen) { + validateJson(initialContent); + } + }, [isOpen, initialContent]); // Handle content changes in editor const handleContentChange = (value: string) => { setEditorContent(value); validateJson(value); };Then update the editor to use
effectiveContent:<JsonEditor - editorContent={editorContent} + editorContent={effectiveContent} onContentChange={handleContentChange} isDark={isDark} />src/components/UploadScreen.tsx (1)
60-60: Loading state implementation looks good.The loading indicator and conditional button rendering provide clear feedback during async content loading. However, the button label logic could be slightly simplified.
Consider extracting the label condition for better readability:
+ const hasContent = uploadState.file || uploadState.content; + {!isLoading && ( <div className="flex flex-wrap gap-3 justify-center"> <Button onClick={open} variant="primary" size="lg" > <FolderOpen className="h-4 w-4" /> - <span>{(!uploadState.file && !uploadState.content) ? 'Upload' : 'Change'}</span> + <span>{hasContent ? 'Change' : 'Upload'}</span> </Button> <Button onClick={handleManualEdit} variant="secondary" size="lg" > <Edit className="h-4 w-4" /> - <span>{(!uploadState.file && !uploadState.content) ? 'JSON' : 'Edit'}</span> + <span>{hasContent ? 'Edit' : 'JSON'}</span> </Button> </div> )}Also applies to: 266-269, 283-304
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
package.json(3 hunks)src/App.test.tsx(1 hunks)src/components/InspectStateViewer.tsx(2 hunks)src/components/JsonEditorDialog.test.tsx(1 hunks)src/components/JsonEditorDialog.tsx(1 hunks)src/components/ServiceViewer.tsx(1 hunks)src/components/SettingsDialog.tsx(1 hunks)src/components/UploadScreen.test.tsx(1 hunks)src/components/UploadScreen.tsx(10 hunks)src/components/service/ServiceCard.test.tsx(6 hunks)src/trie/TrieView.tsx(1 hunks)src/trie/components/trie/index.tsx(1 hunks)src/utils/jsonValidation.test.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/UploadScreen.tsx (1)
src/utils/jsonValidation.ts (1)
validateJsonContent(209-228)
🪛 GitHub Actions: CI/CD Pipeline
src/components/service/ServiceCard.test.tsx
[error] 26-26: TS2347: Untyped function calls may not accept type arguments.
🪛 GitHub Check: Build
src/components/service/ServiceCard.test.tsx
[failure] 26-26:
Untyped function calls may not accept type arguments.
🔇 Additional comments (13)
src/components/service/ServiceCard.test.tsx (3)
7-20: LGTM!The mock implementation correctly forwards role attributes and props, which is essential for the accessibility-focused tests in this file.
80-106: LGTM! Intentional refactor to traditional function expressions.The change from arrow functions to traditional function expressions aligns with the PR's broader refactoring strategy for test mocks, enabling proper mock behavior with function boundaries.
157-159: LGTM! Tab name updates align with component changes.The updated tab label expectations correctly reflect the component's new naming convention, removing separators while maintaining the subscript identifiers (aₛ, aₚ, aₗ).
src/components/JsonEditorDialog.test.tsx (1)
36-60: LGTM! Test mock updates align with dependency upgrades.The conversion from arrow functions to
functionexpressions forwindow.matchMediaandMutationObservermocks changes the binding context, which is likely required for compatibility with the updated test environment (jsdom ^27.1.0, vitest ^4.0.8). Theas anytype assertion on the MutationObserver mock is a pragmatic approach for test doubles.src/components/InspectStateViewer.tsx (1)
59-64: Excellent! Side effects properly moved to useEffect.Moving the window property assignments from the render phase into
useEffectfollows React best practices. Side effects like mutating global objects should occur in effects, not during render, to avoid issues with concurrent rendering and Strict Mode double-invocations.The dependencies
[preStateAccess, stateAccess]correctly ensure the exposed values stay synchronized with the state access objects.src/utils/jsonValidation.test.ts (3)
56-64: LGTM! FileReader mock updated for test environment compatibility.The conversion from arrow functions to
functionexpressions forFileReadermocks aligns with the pattern used in other test files and is required for compatibility with the updated test environment (jsdom ^27.1.0, vitest ^4.0.8).
288-296: LGTM! Consistent mock pattern applied.The FileReader mock follows the same function-expression pattern as line 58, maintaining consistency across the test file.
598-606: LGTM! Mock helper updated consistently.The
mockFileReaderWithContenthelper uses the same function-expression pattern, ensuring consistency across all FileReader mocks in the test file.package.json (1)
57-67: Versions confirmed as latest stable and secure; complete remaining manual verification.The automated checks confirm all three packages are at their latest stable releases with no active security vulnerabilities:
- Latest versions verified: [email protected], [email protected], [email protected]
- Security status: No vulnerabilities in current versions. The CRITICAL Vitest vulnerabilities listed affect older versions (< 3.0.5, < 2.1.9, < 1.6.1), not your current 4.0.8.
To complete verification, manually confirm:
- All tests pass with [email protected]
- Breaking changes from major bumps (especially eslint-plugin-react-hooks) are addressed—the original comment mentions "new lint warnings," suggesting these have already surfaced and need resolution
src/App.test.tsx (1)
79-90: LGTM!The refactor from arrow function to traditional function expression maintains the same mock behavior and aligns with the consistent test mocking style applied across the PR.
src/components/UploadScreen.test.tsx (1)
31-42: LGTM!The mock refactor maintains identical behavior and is consistent with the test mocking style updates across the PR.
src/components/UploadScreen.tsx (2)
15-15: LGTM! Good use of dynamic imports for code splitting.Converting example content to async factories enables lazy loading of fixture data, reducing the initial bundle size. The dynamic
import()statements will only load these fixtures when users click the example buttons.Also applies to: 22-22, 27-27, 32-32, 37-37
171-171: LGTM! Proper loading state management.Disabling example buttons during loading prevents race conditions from multiple simultaneous content loads.
Also applies to: 180-180, 189-189, 198-198, 214-214
| // Mock the UI components | ||
| vi.mock('../ui', () => { | ||
| const React = require('react'); | ||
|
|
||
| const TabsContext = React.createContext<{ value?: string; onValueChange?: (value: string) => void }>({}); | ||
|
|
||
| const Tabs = ({ children, value, onValueChange, ...props }: any) => { | ||
| const [internalValue, setInternalValue] = React.useState('info'); | ||
| const currentValue = value ?? internalValue; | ||
| const handleValueChange = React.useCallback((newValue: string) => { | ||
| if (value === undefined) { | ||
| setInternalValue(newValue); | ||
| } | ||
| onValueChange?.(newValue); | ||
| }, [value, onValueChange]); | ||
|
|
||
| return ( | ||
| <TabsContext.Provider value={{ value: currentValue, onValueChange: handleValueChange }}> | ||
| <div {...props}>{children}</div> | ||
| </TabsContext.Provider> | ||
| ); | ||
| }; | ||
|
|
||
| const TabsList = ({ children, ...props }: any) => ( | ||
| <div role="tablist" {...props}>{children}</div> | ||
| ); | ||
|
|
||
| const TabsTrigger = ({ children, value, className, ...props }: any) => { | ||
| const context = React.useContext(TabsContext); | ||
| const isActive = context.value === value; | ||
|
|
||
| return ( | ||
| <button | ||
| role="tab" | ||
| aria-selected={isActive} | ||
| className={className} | ||
| onClick={() => context.onValueChange?.(value)} | ||
| {...props} | ||
| > | ||
| {children} | ||
| </button> | ||
| ); | ||
| }; | ||
|
|
||
| const TabsContent = ({ children, value, ...props }: any) => { | ||
| const context = React.useContext(TabsContext); | ||
| if (context.value !== value) return null; | ||
| return <div role="tabpanel" {...props}>{children}</div>; | ||
| }; | ||
|
|
||
| return { | ||
| Tabs, | ||
| TabsList, | ||
| TabsTrigger, | ||
| TabsContent, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
Fix TypeScript error: use import instead of require for typed React.
The build is failing because require('react') returns an untyped module, so calling createContext with a type argument is not allowed.
Apply this diff to fix the TypeScript error:
vi.mock('../ui', () => {
- const React = require('react');
+ import React from 'react';
const TabsContext = React.createContext<{ value?: string; onValueChange?: (value: string) => void }>({});Alternative fix (if import syntax causes issues in the mock context):
- const TabsContext = React.createContext<{ value?: string; onValueChange?: (value: string) => void }>({});
+ const TabsContext = React.createContext({});The second approach removes the type argument, which is acceptable for test mocks.
📝 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.
| // Mock the UI components | |
| vi.mock('../ui', () => { | |
| const React = require('react'); | |
| const TabsContext = React.createContext<{ value?: string; onValueChange?: (value: string) => void }>({}); | |
| const Tabs = ({ children, value, onValueChange, ...props }: any) => { | |
| const [internalValue, setInternalValue] = React.useState('info'); | |
| const currentValue = value ?? internalValue; | |
| const handleValueChange = React.useCallback((newValue: string) => { | |
| if (value === undefined) { | |
| setInternalValue(newValue); | |
| } | |
| onValueChange?.(newValue); | |
| }, [value, onValueChange]); | |
| return ( | |
| <TabsContext.Provider value={{ value: currentValue, onValueChange: handleValueChange }}> | |
| <div {...props}>{children}</div> | |
| </TabsContext.Provider> | |
| ); | |
| }; | |
| const TabsList = ({ children, ...props }: any) => ( | |
| <div role="tablist" {...props}>{children}</div> | |
| ); | |
| const TabsTrigger = ({ children, value, className, ...props }: any) => { | |
| const context = React.useContext(TabsContext); | |
| const isActive = context.value === value; | |
| return ( | |
| <button | |
| role="tab" | |
| aria-selected={isActive} | |
| className={className} | |
| onClick={() => context.onValueChange?.(value)} | |
| {...props} | |
| > | |
| {children} | |
| </button> | |
| ); | |
| }; | |
| const TabsContent = ({ children, value, ...props }: any) => { | |
| const context = React.useContext(TabsContext); | |
| if (context.value !== value) return null; | |
| return <div role="tabpanel" {...props}>{children}</div>; | |
| }; | |
| return { | |
| Tabs, | |
| TabsList, | |
| TabsTrigger, | |
| TabsContent, | |
| }; | |
| }); | |
| // Mock the UI components | |
| vi.mock('../ui', () => { | |
| const React = require('react'); | |
| const TabsContext = React.createContext({}); | |
| const Tabs = ({ children, value, onValueChange, ...props }: any) => { | |
| const [internalValue, setInternalValue] = React.useState('info'); | |
| const currentValue = value ?? internalValue; | |
| const handleValueChange = React.useCallback((newValue: string) => { | |
| if (value === undefined) { | |
| setInternalValue(newValue); | |
| } | |
| onValueChange?.(newValue); | |
| }, [value, onValueChange]); | |
| return ( | |
| <TabsContext.Provider value={{ value: currentValue, onValueChange: handleValueChange }}> | |
| <div {...props}>{children}</div> | |
| </TabsContext.Provider> | |
| ); | |
| }; | |
| const TabsList = ({ children, ...props }: any) => ( | |
| <div role="tablist" {...props}>{children}</div> | |
| ); | |
| const TabsTrigger = ({ children, value, className, ...props }: any) => { | |
| const context = React.useContext(TabsContext); | |
| const isActive = context.value === value; | |
| return ( | |
| <button | |
| role="tab" | |
| aria-selected={isActive} | |
| className={className} | |
| onClick={() => context.onValueChange?.(value)} | |
| {...props} | |
| > | |
| {children} | |
| </button> | |
| ); | |
| }; | |
| const TabsContent = ({ children, value, ...props }: any) => { | |
| const context = React.useContext(TabsContext); | |
| if (context.value !== value) return null; | |
| return <div role="tabpanel" {...props}>{children}</div>; | |
| }; | |
| return { | |
| Tabs, | |
| TabsList, | |
| TabsTrigger, | |
| TabsContent, | |
| }; | |
| }); |
🧰 Tools
🪛 GitHub Actions: CI/CD Pipeline
[error] 26-26: TS2347: Untyped function calls may not accept type arguments.
🪛 GitHub Check: Build
[failure] 26-26:
Untyped function calls may not accept type arguments.
🤖 Prompt for AI Agents
In src/components/service/ServiceCard.test.tsx around lines 22 to 78, the mock
uses require('react') which yields an untyped module causing a TypeScript error
when calling createContext with a type parameter; replace the require call with
an ES import for React (e.g., import * as React from 'react') at the top of the
mock so createContext can be typed, or as an alternative remove the generic type
argument from createContext and any typed callbacks (use untyped {} or unknown)
to keep the mock TypeScript-clean if using import is not possible in this mock
scope.
| useEffect(() => { | ||
| if (discoveredServiceIds.length > 0) { | ||
| // eslint-disable-next-line react-hooks/set-state-in-effect | ||
| setServiceIdsInput(discoveredServiceIds.map(formatServiceIdUnsigned).join(', ')); | ||
| } | ||
| }, [discoveredServiceIds]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Refactor to avoid state synchronization in effect.
Setting state inside useEffect to mirror derived values is an anti-pattern. The lint suppression masks a design issue rather than a legitimate exception.
Consider these alternatives:
- Derive directly from state (preferred): Remove
serviceIdsInputstate and compute the display value fromdiscoveredServiceIdsin the render phase. - Use a reset key: If user edits are needed, add a key to the input component that changes when
discoveredServiceIdschanges, triggering a re-mount with fresh initial state.
Apply this diff to derive the value directly:
const discoveredServiceIds = useMemo(() => {
return extractServiceIdsFromState(state);
}, [state]);
- const [serviceIdsInput, setServiceIdsInput] = useState(() => {
- return discoveredServiceIds.length > 0 ? discoveredServiceIds.map(formatServiceIdUnsigned).join(', ') : '0';
- });
-
- useEffect(() => {
- if (discoveredServiceIds.length > 0) {
- // eslint-disable-next-line react-hooks/set-state-in-effect
- setServiceIdsInput(discoveredServiceIds.map(formatServiceIdUnsigned).join(', '));
- }
- }, [discoveredServiceIds]);
+ const defaultServiceIdsInput = useMemo(() => {
+ return discoveredServiceIds.length > 0 ? discoveredServiceIds.map(formatServiceIdUnsigned).join(', ') : '0';
+ }, [discoveredServiceIds]);
+
+ const [serviceIdsInput, setServiceIdsInput] = useState(defaultServiceIdsInput);
const serviceIds = useMemo(() => {
return parseServiceIds(serviceIdsInput);
}, [serviceIdsInput]);If the input needs to reset when state changes, add a key to force remount:
- <ServiceIdsInput
+ <ServiceIdsInput
+ key={JSON.stringify(discoveredServiceIds)}
value={serviceIdsInput}
onChange={setServiceIdsInput}
/>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/ServiceViewer.tsx around lines 25 to 30 the component sets
serviceIdsInput inside a useEffect to mirror discoveredServiceIds (with an
eslint suppression) which is an anti-pattern; remove the effect and either
(preferred) delete the serviceIdsInput state and compute the input display value
directly from discoveredServiceIds in render (e.g.
value={discoveredServiceIds.map(formatServiceIdUnsigned).join(', ')}) or (if
users must edit and you need to reset on discovery changes) keep local state but
add a changing key on the input tied to discoveredServiceIds (e.g.
key={discoveredServiceIds.join(',')} ) so the input remounts with fresh initial
state when discoveredServiceIds change.
Supersedes #132 #131 #130 #129 #118