fix: response format auto-switch on content type change#6773
fix: response format auto-switch on content type change#6773sid-bruno merged 4 commits intousebruno:mainfrom
Conversation
shubh-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
WalkthroughAdd content-type awareness to ResponsePane initialization: Changes
Sequence Diagram(s)sequenceDiagram
participant UI as ResponsePane (UI)
participant Hook as useInitialResponseFormat
participant Store as Dispatcher/Store
UI->>Hook: call useInitialResponseFormat()
Hook-->>UI: return { initialFormat, initialTab, contentType }
UI->>UI: compare contentType vs previousContentRef
alt contentType changed or no persisted values
UI->>Store: dispatch updateResponseFormat(initialFormat)
UI->>Store: dispatch updateResponseViewTab(initialTab)
else no change
UI-->>Store: no dispatch
end
UI->>UI: update previousContentRef with contentType
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bruno-app/src/components/ResponsePane/index.js (1)
58-84: Major: current conditions can overwrite user-selected format/tab on new responses.
if (persistedFormat === null || persistedFormat !== initialFormat)(and same for view tab) will force-reset persisted values whenever a new response’sinitialFormat/initialTabdiffers—this includes cases where the user explicitly chose a different format/tab earlier.A safer heuristic is: auto-update only when the persisted value is unset or still equals the previous response’s initial value (i.e., it was auto-applied last time).
Proposed fix
- const previousResponseRef = useRef({ dataBuffer: null, headers: null }); + const previousResponseRef = useRef({ + dataBuffer: null, + headers: null, + initialFormat: null, + initialTab: null + }); useEffect(() => { if (!focusedTab || initialFormat === null || initialTab === null) { return; } - // Check if response data actually changed (not just format/tab) - const responseChanged - = previousResponseRef.current.dataBuffer !== response?.dataBuffer - || previousResponseRef.current.headers !== response?.headers; + // Check if response data actually changed (not just format/tab) + const responseChanged = + previousResponseRef.current.dataBuffer !== response?.dataBuffer || + previousResponseRef.current.headers !== response?.headers; if (responseChanged) { - // Update ref to track current response - previousResponseRef.current = { - dataBuffer: response?.dataBuffer, - headers: response?.headers - }; + const shouldAutoUpdateFormat = + persistedFormat == null || persistedFormat === previousResponseRef.current.initialFormat; + const shouldAutoUpdateTab = + persistedViewTab == null || persistedViewTab === previousResponseRef.current.initialTab; - // Only auto-update format/tab when response data changes - if (persistedFormat === null || persistedFormat !== initialFormat) { + // Only auto-update format/tab when response data changes *and* user hasn't customized + if (shouldAutoUpdateFormat && persistedFormat !== initialFormat) { dispatch(updateResponseFormat({ uid: item.uid, responseFormat: initialFormat })); } - if (persistedViewTab === null || persistedViewTab !== initialTab) { + if (shouldAutoUpdateTab && persistedViewTab !== initialTab) { dispatch(updateResponseViewTab({ uid: item.uid, responseViewTab: initialTab })); } + + // Update ref to track current response + derived initial values + previousResponseRef.current = { + dataBuffer: response?.dataBuffer, + headers: response?.headers, + initialFormat, + initialTab + }; } }, [response?.dataBuffer, response?.headers, initialFormat, initialTab, persistedFormat, persistedViewTab, focusedTab, item.uid, dispatch]);
🧹 Nitpick comments (1)
packages/bruno-app/src/components/ResponsePane/index.js (1)
44-46: Minor: keep the response-change ref aligned with the “don’t override user choice” goal.
previousResponseRefis a good direction, but you’ll likely also need to track the previousinitialFormat/initialTab(or “last auto-applied” values) to distinguish “auto-set last time” vs “user customized”. Otherwise the effect below will still overwrite user selections on the next response change.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bruno-app/src/components/ResponsePane/index.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-app/src/components/ResponsePane/index.js
🧠 Learnings (3)
📚 Learning: 2025-12-02T09:45:31.709Z
Learnt from: sid-bruno
Repo: usebruno/bruno PR: 6266
File: packages/bruno-app/src/components/ResponsePane/ResponseCopy/index.js:38-38
Timestamp: 2025-12-02T09:45:31.709Z
Learning: In the ResponseCopy component (packages/bruno-app/src/components/ResponsePane/ResponseCopy/index.js), the copy button is intentionally disabled using `!response.data` to prevent copying stream resets which result in empty strings.
Applied to files:
packages/bruno-app/src/components/ResponsePane/index.js
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-app/src/components/ResponsePane/index.js
📚 Learning: 2026-01-09T18:25:14.640Z
Learnt from: kanakkholwal
Repo: usebruno/bruno PR: 6767
File: packages/bruno-app/src/components/ResponseExample/index.js:221-226
Timestamp: 2026-01-09T18:25:14.640Z
Learning: In the Bruno Electron renderer code (packages/bruno-app), assume window.ipcRenderer is always available and skip existence checks. Do not guard for ipcRenderer in this Electron context; use window.ipcRenderer directly (e.g., window.ipcRenderer.send(...), window.ipcRenderer.on(...)). If there are non-Electron contexts (such as test environments or non-Electron builds), add guards or mocks to avoid runtime errors there, but for the intended Electron renderer files, this pattern should be applied broadly within packages/bruno-app.
Applied to files:
packages/bruno-app/src/components/ResponsePane/index.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Unit Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
| dispatch(updateResponseViewTab({ uid: item.uid, responseViewTab: initialTab })); | ||
|
|
||
| // Check if response data actually changed (not just format/tab) | ||
| const responseChanged |
There was a problem hiding this comment.
Will never be true for the dataBuffer since they are Buffer instances and the array references will always be different on each request.
If you wish to compare, compare using Buffer.compare instead
| const focusedTab = find(tabs, (t) => t.uid === activeTabUid); | ||
|
|
||
| // Track previous response data to detect when response actually changes | ||
| const previousResponseRef = useRef({ dataBuffer: null, headers: null }); |
There was a problem hiding this comment.
Is this needed if you are just checking for initialFormat and persistedFormat for the actual check below?
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/bruno-app/src/components/ResponsePane/index.js (1)
59-75: Logic is sound for the bug fix.The header-change detection correctly addresses the issue where content-type changes weren't triggering format updates. The flow properly handles:
- Initial response → sets format based on content-type
- Content-type change → auto-updates format
- Manual format selection → preserved when headers are unchanged
One minor note:
persistedFormatandpersistedViewTabare in the dependency array but aren't used in the effect body. They don't break anything (theheadersChangedguard prevents unwanted dispatches), but removing them would reduce unnecessary effect executions.♻️ Optional: Remove unused dependencies
- }, [response?.headers, initialFormat, initialTab, persistedFormat, persistedViewTab, focusedTab, item.uid, dispatch]); + }, [response?.headers, initialFormat, initialTab, focusedTab, item.uid, dispatch]);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bruno-app/src/components/ResponsePane/index.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-app/src/components/ResponsePane/index.js
🧠 Learnings (3)
📚 Learning: 2025-12-02T09:45:31.709Z
Learnt from: sid-bruno
Repo: usebruno/bruno PR: 6266
File: packages/bruno-app/src/components/ResponsePane/ResponseCopy/index.js:38-38
Timestamp: 2025-12-02T09:45:31.709Z
Learning: In the ResponseCopy component (packages/bruno-app/src/components/ResponsePane/ResponseCopy/index.js), the copy button is intentionally disabled using `!response.data` to prevent copying stream resets which result in empty strings.
Applied to files:
packages/bruno-app/src/components/ResponsePane/index.js
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-app/src/components/ResponsePane/index.js
📚 Learning: 2026-01-09T18:25:14.640Z
Learnt from: kanakkholwal
Repo: usebruno/bruno PR: 6767
File: packages/bruno-app/src/components/ResponseExample/index.js:221-226
Timestamp: 2026-01-09T18:25:14.640Z
Learning: In the Bruno Electron renderer code (packages/bruno-app), assume window.ipcRenderer is always available and skip existence checks. Do not guard for ipcRenderer in this Electron context; use window.ipcRenderer directly (e.g., window.ipcRenderer.send(...), window.ipcRenderer.on(...)). If there are non-Electron contexts (such as test environments or non-Electron builds), add guards or mocks to avoid runtime errors there, but for the intended Electron renderer files, this pattern should be applied broadly within packages/bruno-app.
Applied to files:
packages/bruno-app/src/components/ResponsePane/index.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Playwright E2E Tests
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - macOS
🔇 Additional comments (2)
packages/bruno-app/src/components/ResponsePane/index.js (2)
3-3: LGTM!Using
isEqualfor deep comparison of headers is the right approach here—shallow comparison would fail on object references.
45-46: LGTM!Using a ref to track previous headers is the correct pattern here—persists across renders without triggering additional re-renders.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/bruno-app/src/components/ResponsePane/QueryResult/index.js (1)
45-58: LGTM — hook correctly exposescontentTypefor change detection.The return object shape is now consistent across both code paths, enabling the parent component to track content-type changes and auto-switch the response format. This directly addresses the linked issue.
Minor style nit: you can use ES6 shorthand property syntax for cleaner reads.
✨ Optional: Use ES6 shorthand property syntax
// Wait until both content types are available if (detectedContentType === null || contentType === undefined) { - return { initialFormat: null, initialTab: null, contentType: contentType }; + return { initialFormat: null, initialTab: null, contentType }; } const initial = getDefaultResponseFormat(contentType); - return { initialFormat: initial.format, initialTab: initial.tab, contentType: contentType }; + return { initialFormat: initial.format, initialTab: initial.tab, contentType };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bruno-app/src/components/ResponsePane/QueryResult/index.jspackages/bruno-app/src/components/ResponsePane/index.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-app/src/components/ResponsePane/index.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-app/src/components/ResponsePane/QueryResult/index.js
🧠 Learnings (2)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-app/src/components/ResponsePane/QueryResult/index.js
📚 Learning: 2026-01-09T18:25:14.640Z
Learnt from: kanakkholwal
Repo: usebruno/bruno PR: 6767
File: packages/bruno-app/src/components/ResponseExample/index.js:221-226
Timestamp: 2026-01-09T18:25:14.640Z
Learning: In the Bruno Electron renderer code (packages/bruno-app), assume window.ipcRenderer is always available and skip existence checks. Do not guard for ipcRenderer in this Electron context; use window.ipcRenderer directly (e.g., window.ipcRenderer.send(...), window.ipcRenderer.on(...)). If there are non-Electron contexts (such as test environments or non-Electron builds), add guards or mocks to avoid runtime errors there, but for the intended Electron renderer files, this pattern should be applied broadly within packages/bruno-app.
Applied to files:
packages/bruno-app/src/components/ResponsePane/QueryResult/index.js
🧬 Code graph analysis (1)
packages/bruno-app/src/components/ResponsePane/QueryResult/index.js (1)
packages/bruno-app/src/utils/response/index.js (2)
getDefaultResponseFormat(8-59)getDefaultResponseFormat(8-59)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Unit Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Windows
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/bruno-app/src/components/ResponsePane/index.js (2)
48-49: Minor: Comment could be more precise.The comment says "response headers" but this ref specifically tracks the content-type value. Consider updating for clarity.
Suggested wording
- // Track previous response headers to detect when content-type changes + // Track previous content-type to detect when it changes between responses const previousContentRef = useRef(contentType);
63-74: Fix misleading comment: this is strict equality, not deep comparison.Line 63 says "deep comparison" but
!==is just strict (reference/value) equality. For primitive strings like content-type, this works correctly, but the comment is misleading.Suggested fix
- // Check if response headers (content-type) changed using deep comparison + // Check if content-type changed const contentTypeChanged = contentType !== previousContentRef.current;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bruno-app/src/components/ResponsePane/index.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-app/src/components/ResponsePane/index.js
🧠 Learnings (2)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-app/src/components/ResponsePane/index.js
📚 Learning: 2026-01-09T18:25:14.640Z
Learnt from: kanakkholwal
Repo: usebruno/bruno PR: 6767
File: packages/bruno-app/src/components/ResponseExample/index.js:221-226
Timestamp: 2026-01-09T18:25:14.640Z
Learning: In the Bruno Electron renderer code (packages/bruno-app), assume window.ipcRenderer is always available and skip existence checks. Do not guard for ipcRenderer in this Electron context; use window.ipcRenderer directly (e.g., window.ipcRenderer.send(...), window.ipcRenderer.on(...)). If there are non-Electron contexts (such as test environments or non-Electron builds), add guards or mocks to avoid runtime errors there, but for the intended Electron renderer files, this pattern should be applied broadly within packages/bruno-app.
Applied to files:
packages/bruno-app/src/components/ResponsePane/index.js
🧬 Code graph analysis (1)
packages/bruno-app/src/components/ResponsePane/index.js (1)
packages/bruno-app/src/components/ResponsePane/QueryResult/index.js (5)
useInitialResponseFormat(45-58)useInitialResponseFormat(45-58)useResponsePreviewFormatOptions(61-89)useResponsePreviewFormatOptions(61-89)contentType(102-102)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Playwright E2E Tests
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
🔇 Additional comments (1)
packages/bruno-app/src/components/ResponsePane/index.js (1)
45-45: LGTM!Correctly consuming the new
contentTypereturn value fromuseInitialResponseFormat.
3 participants
Description
Auto-updates the response format/tab when the response content type changes, while preserving manual format selections
Fixes: #6769
Jira
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.