add: apispec support #6337
add: apispec support #6337
Conversation
naman-bruno
requested review from
bijin-bruno,
helloanoop and
lohit-bruno
as code owners
WalkthroughIntroduces comprehensive API specification support to Bruno, including frontend UI components for editing and rendering OpenAPI specs, Redux state management for API specs, Electron backend file handling with watchers, and integration with workspace and sidebar navigation systems. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as Frontend (React)
participant Redux
participant IPC as Electron IPC
participant FS as Filesystem
participant Watcher as ApiSpecWatcher
User->>UI: Click "Open API Spec"
UI->>IPC: renderer:open-api-spec
IPC->>FS: File dialog selection
FS-->>IPC: Selected path
IPC->>FS: Read & parse file
FS-->>IPC: Content + metadata
IPC->>Watcher: Register watcher for path
Watcher->>FS: Monitor file changes
IPC->>UI: Send apispec-tree-updated (addFile)
UI->>Redux: Dispatch apiSpecAddFileEvent
Redux-->>UI: Update state (apiSpecs)
UI->>UI: Render ApiSpecPanel
User->>UI: Edit YAML content
UI->>Redux: Dispatch onEdit (local state update)
User->>UI: Click Save
UI->>Redux: Dispatch saveApiSpecToFile
Redux->>IPC: renderer:save-api-spec
IPC->>FS: Write file
FS-->>IPC: Success
Watcher->>FS: Detect change
Watcher->>IPC: Emit main:apispec-tree-updated
IPC->>UI: Send change event
UI->>Redux: Dispatch apiSpecChangeFileEvent
Redux-->>UI: Update state
UI-->>User: Show toast (Saved)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~75 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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: 7
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (14)
packages/bruno-electron/src/ipc/collection.js-1558-1576 (1)
1558-1576: Synchronous I/O operations block the main Electron process.Using
fs.readdirSync,fs.lstatSync, andfs.readFileSyncin an async IPC handler will block the main process, degrading UI responsiveness for large collections. Consider using async alternatives:- const filesInCurrentDir = fs.readdirSync(currentPath); + const filesInCurrentDir = await fs.promises.readdir(currentPath);- const stats = fs.lstatSync(filePath); + const stats = await fs.promises.lstat(filePath);- const content = fs.readFileSync(filePath, 'utf8'); + const content = await fs.promises.readFile(filePath, 'utf8');Apply similar changes to all
readFileSynccalls within this handler (lines 1580, 1591, 1608, 1627, 1636).Committable suggestion skipped: line range outside the PR's diff.
packages/bruno-electron/src/ipc/collection.js-1568-1570 (1)
1568-1570: Directory filter logic uses full path instead of directory name.The conditions
!filePath.startsWith('.git')and!filePath.startsWith('node_modules')check the full path, not the directory name. A path like/home/user/.git-projects/collectionwould be incorrectly filtered.- if (stats.isDirectory() && !filePath.startsWith('.git') && !filePath.startsWith('node_modules')) { + if (stats.isDirectory() && !file.startsWith('.git') && file !== 'node_modules') {Committable suggestion skipped: line range outside the PR's diff.
packages/bruno-electron/src/ipc/workspace.js-153-188 (1)
153-188: Consider using yaml.safeLoad and validating API spec structure.The handler correctly validates workspacePath and resolves relative paths. However:
Security concern: Line 166 uses
yaml.load()which can execute arbitrary code. Useyaml.safeLoad()(deprecated) oryaml.load(yamlContent, { schema: yaml.SAFE_SCHEMA })for safer parsing.Data validation: The code assumes each apiSpec in the array has a
pathproperty (line 175). Consider validating the structure to prevent runtime errors.Apply this diff to improve security:
- const workspaceConfig = yaml.load(yamlContent); + const workspaceConfig = yaml.load(yamlContent, { schema: yaml.SAFE_SCHEMA });Additionally, consider adding validation:
const resolvedApiSpecs = apiSpecs.map((apiSpec) => { + if (!apiSpec || typeof apiSpec !== 'object') { + return null; + } if (apiSpec.path && !path.isAbsolute(apiSpec.path)) { return { ...apiSpec, path: path.join(workspacePath, apiSpec.path) }; } return apiSpec; - }); + }).filter(Boolean);packages/bruno-app/src/components/ApiSpecPanel/FileEditor/index.js-14-14 (1)
14-14: Stale state: content won't update when apiSpec changes.If the user switches to a different API spec,
contentwill retain the previous spec's value sinceuseStateonly uses the initial value once.Add an effect to sync state when
apiSpecchanges:const [content, setContent] = useState(apiSpec?.raw); +useEffect(() => { + setContent(apiSpec?.raw); +}, [apiSpec?.uid]);Don't forget to import
useEffectfrom React.Committable suggestion skipped: line range outside the PR's diff.
packages/bruno-electron/src/app/apiSpecs.js-25-27 (1)
25-27: Moverequirestatements to module top-level.
fs,path, andjs-yamlare required inline multiple times (lines 25-27, 83-84, 87-88, 90). This adds overhead and reduces readability. Move to top-level imports.const { dialog, ipcMain } = require('electron'); const { isDirectory, normalizeAndResolvePath } = require('../utils/filesystem'); const { generateUidBasedOnHash } = require('../utils/common'); +const fs = require('fs'); +const path = require('path'); +const yaml = require('js-yaml');Also applies to: 80-96
packages/bruno-app/src/utils/exporters/openapi-spec.js-332-341 (1)
332-341: Invalid security scheme typedigest.OpenAPI 3.0 doesn't support
type: 'digest'. Digest auth should usetype: 'http'withscheme: 'digest'.case 'digest': components.securitySchemes[securitySchemaId] = { - type: 'digest', + type: 'http', scheme: 'digest', description: 'Digest Authentication' };packages/bruno-app/src/utils/exporters/openapi-spec.js-299-318 (1)
299-318: Incorrect OAuth2 flow forclient_credentials.The
client_credentialsgrant type incorrectly uses thepasswordflow. OpenAPI 3.0 specifiesclientCredentialsfor this grant type.case 'client_credentials': components.securitySchemes[securitySchemaId] = { type: 'oauth2', flows: { - password: { + clientCredentials: { tokenUrl: accessTokenUrl,packages/bruno-app/src/utils/exporters/openapi-spec.js-256-256 (1)
256-256: Unsafe destructuring from optional chain.If
auth?.oauth2is nullish, destructuring throws. Add a fallback.- const { authorizationUrl, accessTokenUrl, callbackUrl, scope } = auth?.oauth2; + const { authorizationUrl, accessTokenUrl, callbackUrl, scope } = auth?.oauth2 || {};packages/bruno-app/src/utils/exporters/openapi-spec.js-35-35 (1)
35-35: Platform-specific path separator.Using
\\as separator only works on Windows. Consider using a platform-agnostic approach or regex that handles both/and\\.- const parts = pathname.split('\\'); + const parts = pathname.split(/[\\/]/);packages/bruno-app/src/utils/exporters/openapi-spec.js-61-81 (1)
61-81: Unsafe optional chaining with spread operator.Static analysis correctly flags lines 62, 69, 76: spreading
undefined(from short-circuited optional chains) into an array throwsTypeError. Add fallback empty arrays.const parameters = [ - ...params?.map((param) => ({ + ...(params?.map((param) => ({ name: param?.name, in: 'query', description: '', required: param?.enabled, example: param?.value - })), - ...headers?.map((header) => ({ + })) || []), + ...(headers?.map((header) => ({ name: header?.name, in: 'header', description: '', required: header?.enabled, example: header?.value - })), - ...pathMatches?.map((path) => ({ + })) || []), + ...(pathMatches?.map((path) => ({ name: path.slice(1, path.length - 1), in: 'path', required: true - })) + })) || []) ];packages/bruno-electron/src/app/apiSpecs.js-12-14 (1)
12-14: Missingawaitfor async function.
openApiSpecis async but not awaited. Errors thrown inside won't be caught by the surrounding try-catch.try { - openApiSpec(win, watcher, resolvedPath, options); + await openApiSpec(win, watcher, resolvedPath, options); } catch (err) {packages/bruno-electron/src/app/apiSpecs.js-6-8 (1)
6-8: Invalid dialog propertycreateFile.Electron's
dialog.showOpenDialogdoesn't supportcreateFile. Supported properties areopenFile,openDirectory,multiSelections,showHiddenFiles,createDirectory(macOS), andpromptToCreate(Windows). This unsupported property will be silently ignored.packages/bruno-electron/src/app/apiSpecsWatcher.js-32-68 (1)
32-68: Eliminate code duplication and redundant file reads.The
addandchangefunctions are nearly identical, differing only in the IPC event name. Additionally, both functions read the file twice: once inparseApiSpecContentand again to get the raw content.Refactor to a shared helper:
+const buildFilePayload = (pathname) => { + const basename = path.basename(pathname); + const raw = fs.readFileSync(pathname, 'utf8'); + const extension = path.extname(pathname).toLowerCase(); + + let apiSpecContent = null; + if (extension === '.yaml' || extension === '.yml') { + apiSpecContent = yaml.load(raw); + } else if (extension === '.json') { + apiSpecContent = safeParseJSON(raw); + } + + const file = { + raw, + name: apiSpecContent?.info?.title || basename.split('.')[0], + filename: basename, + pathname, + json: apiSpecContent + }; + + hydrateApiSpecWithUuid(file, pathname); + return file; +}; + const add = async (win, pathname) => { if (!hasApiSpecExtension(pathname)) return; try { - const basename = path.basename(pathname); - const file = {}; - const apiSpecContent = parseApiSpecContent(pathname); - - file.raw = fs.readFileSync(pathname, 'utf8'); - file.name = apiSpecContent?.info?.title || basename.split('.')[0]; - file.filename = basename; - file.pathname = pathname; - file.json = apiSpecContent; - hydrateApiSpecWithUuid(file, pathname); + const file = buildFilePayload(pathname); win.webContents.send('main:apispec-tree-updated', 'addFile', file); } catch (err) { console.error(err); } }; const change = async (win, pathname) => { if (!hasApiSpecExtension(pathname)) return; try { - const basename = path.basename(pathname); - const file = {}; - const apiSpecContent = parseApiSpecContent(pathname); - - file.raw = fs.readFileSync(pathname, 'utf8'); - file.name = apiSpecContent?.info?.title || basename.split('.')[0]; - file.filename = basename; - file.pathname = pathname; - file.json = apiSpecContent; - hydrateApiSpecWithUuid(file, pathname); + const file = buildFilePayload(pathname); win.webContents.send('main:apispec-tree-updated', 'changeFile', file); } catch (err) { console.error(err); } };Committable suggestion skipped: line range outside the PR's diff.
packages/bruno-electron/src/app/apiSpecsWatcher.js-113-114 (1)
113-114: Function signature mismatch: unused parameters.The
addandchangefunctions are called with 5 parameters (win, pathname, apiSpecUid, watchPath, workspacePath), but their definitions (lines 32, 51) only accept 2 parameters (win, pathname). The extra parameters are silently dropped.Verify whether
apiSpecUid,watchPath, andworkspacePathare needed for proper functionality. If they are required (e.g., for workspace association), update the function signatures:-const add = async (win, pathname) => { +const add = async (win, pathname, apiSpecUid, watchPath, workspacePath) => { if (!hasApiSpecExtension(pathname)) return; try { // ... existing logic ... + // Use apiSpecUid, watchPath, workspacePath as needed } }; -const change = async (win, pathname) => { +const change = async (win, pathname, apiSpecUid, watchPath, workspacePath) => { if (!hasApiSpecExtension(pathname)) return; try { // ... existing logic ... + // Use apiSpecUid, watchPath, workspacePath as needed } };If the parameters are not needed, remove them from the function calls.
Committable suggestion skipped: line range outside the PR's diff.
🟡 Minor comments (8)
packages/bruno-electron/src/ipc/collection.js-1609-1611 (1)
1609-1611: Unused variableenvironmentName.
environmentNameis computed but never used. Either remove it or use it as the key inenvVariables(line 1616) instead ofpath.basename(filePath)which includes the extension.const environmentFilepathBasename = path.basename(filePath); const environmentName = environmentFilepathBasename.substring(0, environmentFilepathBasename.length - 4); let data = await parseEnvironment(bruContent); variables = { ...variables, envVariables: { ...(variables?.envVariables || {}), - [path.basename(filePath)]: data.variables + [environmentName]: data.variables } };packages/bruno-app/src/components/Sidebar/ApiSpecs/ApiSpecItem/index.js-1-64 (1)
1-64: Tweak equality check and imports; behavior otherwise looks goodThe item wiring (open on click, set active UID, show Close modal via Dropdown) looks correct and aligns with the Redux state.
Two small improvements:
Use strict equality for active state
showApiSpecPage && apiSpec?.uid == activeApiSpecUid ? 'active' : ''To avoid unexpected type coercion, it’s safer to use strict equality:
showApiSpecPage && apiSpec?.uid === activeApiSpecUid ? 'active' : ''Minor import cleanup (optional)
You currently import fromreacttwice (useState/useRefandforwardRef). You can consolidate to a single import:import { useState, useRef, forwardRef } from 'react';Everything else in this component looks solid.
packages/bruno-app/src/components/ApiSpecPanel/FileEditor/index.js-44-44 (1)
44-44: Typo:oapcity-100should beopacity-100.This typo prevents the opacity class from applying.
- hasChanges ? 'cursor-pointer oapcity-100' : 'cursor-default opacity-50' + hasChanges ? 'cursor-pointer opacity-100' : 'cursor-default opacity-50'packages/bruno-app/src/components/ApiSpecPanel/Renderers/Swagger/index.js-8-8 (1)
8-8: Remove debug console.log before merging.This debug statement will pollute the console in production.
- console.log('string', string);packages/bruno-app/src/components/ApiSpecPanel/FileEditor/CodeEditor/index.js-34-34 (1)
34-34: Typo: "hightlighting" → "highlighting".- // YAML linting and hightlighting plugin + // YAML linting and highlighting pluginpackages/bruno-app/src/components/Sidebar/ApiSpecs/CreateApiSpec/index.js-110-110 (1)
110-110: Typo: "diolog" → "dialog".- // When the user closes the diolog without selecting anything dirPath will be false + // When the user closes the dialog without selecting anything dirPath will be falsepackages/bruno-electron/src/ipc/apiSpec.js-21-28 (1)
21-28: Missing return on success path.Line 24 creates a
Promise.resolve()that's not returned. On success, the handler returnsundefinedinstead of explicitly resolving. While this may work, it's inconsistent with the error path.ipcMain.handle('renderer:save-api-spec', async (event, pathname, content) => { try { await writeFile(pathname, content); - Promise.resolve(); + return; } catch (error) { return Promise.reject(error); } });packages/bruno-electron/src/app/apiSpecsWatcher.js-1-1 (1)
1-1: Remove unused lodash import.The lodash library is imported but never used in this file.
Apply this diff:
-const _ = require('lodash'); const fs = require('fs');
🧹 Nitpick comments (34)
packages/bruno-electron/src/ipc/collection.js (3)
1560-1562: Duplicate and unreliablenode_modulescheck.The
currentPath.includes('node_modules')check is duplicated (also at line 1568) and would incorrectly match paths like/home/user/node_modules_backup/. Since you're already filtering at the directory traversal level (line 1568), this check can be removed or tightened.- if (currentPath.includes('node_modules')) { - return; - }
1625-1634: Incomplete implementation flagged with TODO.The
isCollectionRootBruFileblock parses the collection but doesn't use the result. If this is intentional placeholder code, consider documenting the intended behavior or deferring this code until fully implemented.Would you like me to help flesh out the logic for processing
collection.brudata, or should this be tracked in a separate issue?
1550-1551: Missing input validation forcollectionPath.No validation that
collectionPathexists or is a directory before traversal begins. If an invalid path is passed, the error message fromreaddirSyncwon't be user-friendly.ipcMain.handle('renderer:get-collection-json', async (event, collectionPath) => { + if (!collectionPath || !fs.existsSync(collectionPath)) { + throw new Error(`Invalid collection path: ${collectionPath}`); + } let variables = {}; let name = '';packages/bruno-app/src/components/ApiSpecPanel/StyledWrapper.js (1)
1-20: Panel styling is fine; consider deduping danger menu item stylesThe styles for
.menu-iconand the dangerdropdown-item.menu-itemare correct and consistent with the sidebar. Longer term, you might centralize the “danger” dropdown-item styling (here and in the sidebar ApiSpecs wrapper) into a shared styled wrapper or mixin to avoid duplication, but it’s not blocking.packages/bruno-app/src/providers/ReduxStore/slices/app.js (1)
13-14: API spec page flags are wired correctly; double‑check interaction with Preferences
showApiSpecPageis added to initial state and reducers in a consistent way, andshowHomePagenow correctly clears the API Spec view. One thing to verify:showPreferencescurrently only togglesstate.showPreferencesand does not clearstate.showApiSpecPage, so it’s possible for both flags to betrue. If the intent is strict mutual exclusivity between the API Spec page and Preferences, consider also resettingshowApiSpecPagein the preferences-related reducer(s).Also applies to: 74-88, 126-147
packages/bruno-app/src/components/Sidebar/ApiSpecs/CloseApiSpec/index.js (1)
1-37: Close flow and async handling look solidThe modal correctly dispatches
closeApiSpecFile, handles the returned Promise, shows success/error toasts, and only closes on success. Implementation is straightforward and matches existing Modal patterns. As a tiny polish, you may want to standardize the capitalization of “API Spec” in the title/body for consistency, but functionally this is good to go.packages/bruno-electron/src/utils/workspace-config.js (1)
63-70: Workspace apiSpec wiring is correct; consider normalizing paths like collectionsThe new
apiSpecshandling (init increateWorkspaceConfig, plusgetWorkspaceApiSpecs/addApiSpecToWorkspace/removeApiSpecFromWorkspace) is consistent with the existing collections logic and should behave correctly with relative and absolute paths.For consistency with
getWorkspaceCollections, you may want to switch this:if (apiSpec.path && !path.isAbsolute(apiSpec.path)) { return { ...apiSpec, path: path.join(workspacePath, apiSpec.path) }; }to use
path.resolve(workspacePath, apiSpec.path)instead ofpath.join(...). That will normalize any..segments and mirror the collection path handling.Also applies to: 215-291, 293-309
packages/bruno-app/src/components/Sidebar/Collections/index.js (1)
10-57: ApiSpecs integration works; consider spec‑only workspaces and divider simplificationThe new
ApiSpecsblock is correctly wired into the Collections sidebar and will render underneath the collections list.Two small points to consider:
Visibility when there are no collections
Because of the early return whenworkspaceCollectionsis empty, theApiSpecssection won’t render at all in that case. If you expect workspaces that might only contain API specs (no collections), you may want to renderApiSpecsoutside that early return so they’re still accessible.Divider markup (optional)
The spacer<div className="w-full my-2" style={{ height: 1 }}></div>is effectively used just for vertical spacing. You could likely drop the inline
style={{ height: 1 }}and rely onmy-2, or replace it with a themed horizontal rule if you want a visible separator line.packages/bruno-app/src/components/ApiSpecPanel/Renderers/Swagger/index.js (1)
5-5: Consider a more descriptive prop name.
stringis ambiguous. A name likespecorcontentwould better convey the expected value.-const Swagger = ({ string }) => { +const Swagger = ({ spec }) => {Then update line 13:
- <SwaggerUI spec={string} /> + <SwaggerUI spec={spec} />packages/bruno-app/src/components/ApiSpecPanel/FileEditor/CodeEditor/StyledWrapper.js (3)
14-19: Use theme props instead of hardcoded colors.Hardcoded hex values break theme consistency. Based on learnings, use
props.themefor colors.input { background: transparent; - border: 1px solid #d3d6db; + border: 1px solid ${(props) => props.theme.input.border}; outline: none; border-radius: 0px; }
31-55: Address the TODO: Replace hardcoded Monokai overrides with theme-aware styling.The
!importantoverrides and hardcoded colors should be refactored to use theme props for maintainability and proper dark/light mode support.Would you like me to help design a theme-aware approach for these syntax highlighting colors?
57-62: Use semantic theme colors for validation indicators.Hardcoded
greenandredwon't adapt to themes. Consider using theme tokens likeprops.theme.colors.successandprops.theme.colors.danger.packages/bruno-app/src/components/ApiSpecPanel/FileEditor/index.js (1)
24-24: Use strict equality.Prefer
!==over!=for comparison.- const hasChanges = Boolean(content != apiSpec?.raw); + const hasChanges = Boolean(content !== apiSpec?.raw);packages/bruno-app/src/components/ApiSpecPanel/index.js (2)
31-37: MoveMenuIconoutside the component.Defining
MenuIconinsideApiSpecPanelrecreates it on every render, which can cause unnecessary re-renders for Tippy/Dropdown.+const MenuIcon = forwardRef((props, ref) => { + return ( + <div ref={ref}> + <IconDots size={22} /> + </div> + ); +}); const ApiSpecPanel = () => { // ... - const MenuIcon = forwardRef((props, ref) => { - return ( - <div ref={ref}> - <IconDots size={22} /> - </div> - ); - });
1-1: Consolidate React imports.Multiple React imports can be combined for cleaner code.
-import React, { forwardRef, useRef } from 'react'; +import React, { forwardRef, useRef, useState, Suspense } from 'react'; // ... -import { useState } from 'react'; // ... -import { Suspense } from 'react';Also applies to: 9-9, 11-11
packages/bruno-app/src/components/ApiSpecPanel/FileEditor/CodeEditor/Plugins/Yaml/index.js (2)
3-4: Preferconstovervar.Modern JavaScript convention.
varcan cause scoping issues.- var cons = ['true', 'false', 'on', 'off', 'yes', 'no']; - var keywordRegex = new RegExp('\\b((' + cons.join(')|(') + '))$', 'i'); + const cons = ['true', 'false', 'on', 'off', 'yes', 'no']; + const keywordRegex = new RegExp('\\b((' + cons.join(')|(') + '))$', 'i');
7-8: Consider usingconstforchandescas well.These variables are not reassigned within their scope.
- var ch = stream.peek(); - var esc = state.escaped; + const ch = stream.peek(); + const esc = state.escaped;packages/bruno-app/src/providers/ReduxStore/slices/workspaces/actions.js (1)
204-207: Use object shorthand.Per ES6 conventions,
apiSpecs: apiSpecscan be simplified.dispatch(updateWorkspace({ uid: workspaceUid, - apiSpecs: apiSpecs + apiSpecs }));packages/bruno-app/src/components/Sidebar/ApiSpecs/index.js (2)
23-33: Redundant dependency in useMemo.
activeWorkspace?.apiSpecsis derived fromactiveWorkspace, so listing it separately is unnecessary.- }, [allApiSpecs, activeWorkspace, activeWorkspace?.apiSpecs]); + }, [allApiSpecs, activeWorkspace]);
66-70: Redundant null check.The condition at line 47 already guarantees
apiSpecsis non-empty when this branch renders.<div className="flex flex-col top-32 bottom-10 left-0 right-0 py-4"> - {apiSpecs && apiSpecs.length - ? apiSpecs.map((apiSpec) => { - return <ApiSpecItem apiSpec={apiSpec} key={apiSpec.uid} />; - }) - : null} + {apiSpecs.map((apiSpec) => ( + <ApiSpecItem apiSpec={apiSpec} key={apiSpec.uid} /> + ))} </div>packages/bruno-app/src/components/ApiSpecPanel/FileEditor/CodeEditor/index.js (2)
74-78: Potential issue:editorreferenced before assignment.The Tab handler uses
editor.getLine()buteditoris assigned from the outer scope in the same expression. This works due to closure semantics, but it's fragile. Consider usingcmconsistently.'Tab': function (cm) { - cm.getSelection().includes('\n') || editor.getLine(cm.getCursor().line) == cm.getSelection() + cm.getSelection().includes('\n') || cm.getLine(cm.getCursor().line) === cm.getSelection() ? cm.execCommand('indentMore') : cm.replaceSelection(' ', 'end'); },
113-116: Side effect in render method.Calling
editor.refresh()inrender()is unconventional and can cause performance issues. Consider moving this tocomponentDidUpdatewhen relevant props change.packages/bruno-electron/src/ipc/apiSpec.js (3)
8-8: Unused parameterlastOpenedApiSpecs.The parameter is passed to both
registerRendererEventHandlersandregisterMainEventHandlersbut never referenced. Either remove it or document its intended future use.Also applies to: 106-106, 117-119
49-74: Inlinerequireand synchronous file I/O.The
js-yamlrequire at line 50 should be at module top-level. Additionally,readFileSync/writeFileSyncon lines 54 and 74 block the main process. Consider using async versions for better responsiveness.+const yaml = require('js-yaml'); // ... at top of file ipcMain.handle('renderer:remove-api-spec', async (event, pathname, workspacePath = null) => { try { if (watcher && mainWindow) { watcher.removeWatcher(pathname, mainWindow); removeApiSpecUid(pathname); if (workspacePath) { - const yaml = require('js-yaml'); const workspaceFilePath = path.join(workspacePath, 'workspace.yml'); if (fs.existsSync(workspaceFilePath)) { - const yamlContent = fs.readFileSync(workspaceFilePath, 'utf8'); + const yamlContent = await fs.promises.readFile(workspaceFilePath, 'utf8');
84-91: Consider adding timeout and error handling for fetch.The
fetchcall has no timeout, which could hang indefinitely on unresponsive URLs. Also, non-2xx responses won't throw and will return unexpected content.ipcMain.handle('renderer:fetch-api-spec', async (event, url) => { try { - const data = await fetch(url).then((res) => res.text()); + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30000); + const res = await fetch(url, { signal: controller.signal }); + clearTimeout(timeoutId); + if (!res.ok) { + throw new Error(`HTTP ${res.status}: ${res.statusText}`); + } + const data = await res.text(); return data; } catch (error) { return Promise.reject(error); } });packages/bruno-app/src/providers/ReduxStore/slices/apiSpec.js (3)
64-64: Use strict equality.Line 64 uses
==which allows type coercion. Prefer===for consistency with line 65.- let apiSpecIndex = state.apiSpecs.findIndex((c) => c.uid == uid); + let apiSpecIndex = state.apiSpecs.findIndex((c) => c.uid === uid);
137-160: Redundant null check and unreachable code.Line 140's
if (apiSpec)is always true after the early return on line 138. Line 160'sreturn;is unreachable.if (!apiSpec) { return reject(new Error('API Spec not found')); } - if (apiSpec) { - const { ipcRenderer } = window; + const { ipcRenderer } = window; // ... rest of logic - } - return; });
151-154: Dynamic require inside async callback.Using
require()inside the.then()callback works but is unconventional in ES modules context. Consider importing at the top level if circular dependencies allow.packages/bruno-app/src/components/Sidebar/ApiSpecs/CreateApiSpec/index.js (2)
1-1: Consolidate React imports.
useStateis imported separately on line 9 whileuseRef, useEffectare imported with React on line 1. Consolidate for consistency.-import React, { useRef, useEffect } from 'react'; +import React, { useRef, useEffect, useState } from 'react'; // ... -import { useState } from 'react';Also applies to: 9-9
60-73: Validation schema doesn't conditionally requirecollectionLocation.When
importFrom === 'collection',collectionLocationshould be required but the schema only specifiesmin(1). Consider using Yup'swhen()for conditional validation.-collectionLocation: Yup.string().min(1, 'location is required'), +collectionLocation: Yup.string().when('importFrom', { + is: 'collection', + then: (schema) => schema.min(1, 'location is required').required('Collection location is required'), + otherwise: (schema) => schema +}),packages/bruno-electron/src/app/apiSpecs.js (1)
86-95: Redundant file reads and potential race condition.Lines 83 and 88 both call
readFileSync(apiSpecPath, 'utf8'). The file is read twice, and if the file changes between reads,rawandjsoncould be inconsistent. Read once and reuse.+ const content = fs.readFileSync(apiSpecPath, 'utf8'); + const ext = path.extname(apiSpecPath).toLowerCase(); win.webContents.send('main:apispec-tree-updated', 'addFile', { pathname: apiSpecPath, uid: uid, - raw: require('fs').readFileSync(apiSpecPath, 'utf8'), - name: require('path').basename(apiSpecPath, require('path').extname(apiSpecPath)), - filename: require('path').basename(apiSpecPath), - json: (() => { - const ext = require('path').extname(apiSpecPath).toLowerCase(); - const content = require('fs').readFileSync(apiSpecPath, 'utf8'); - if (ext === '.yaml' || ext === '.yml') { - return require('js-yaml').load(content); - } else if (ext === '.json') { - return JSON.parse(content); - } - return null; - })() + raw: content, + name: path.basename(apiSpecPath, path.extname(apiSpecPath)), + filename: path.basename(apiSpecPath), + json: ext === '.yaml' || ext === '.yml' + ? yaml.load(content) + : ext === '.json' + ? JSON.parse(content) + : null });packages/bruno-electron/src/app/apiSpecsWatcher.js (3)
15-25: Add error handling for file I/O and parsing.Both
fs.readFileSync(line 17) andyaml.load(line 20) can throw exceptions. While callers currently wrap this in try-catch, error handling within the helper would make it more robust and reusable.Consider this approach:
const parseApiSpecContent = (pathname) => { + try { const extension = path.extname(pathname).toLowerCase(); let content = fs.readFileSync(pathname, 'utf8'); if (extension === '.yaml' || extension === '.yml') { return yaml.load(content); } else if (extension === '.json') { return safeParseJSON(content); } return null; + } catch (err) { + console.error('Error parsing API spec:', pathname, err); + return null; + } };
124-132: Usedeletefor consistent memory cleanup.Line 127 sets
this.watchers[watchPath] = null, which leaves the key in the object, while line 130 usesdeleteforwatcherWorkspaces. For proper memory management and consistency, usedeletefor both.removeWatcher(watchPath, win) { if (this.watchers[watchPath]) { this.watchers[watchPath].close(); - this.watchers[watchPath] = null; + delete this.watchers[watchPath]; } if (this.watcherWorkspaces[watchPath]) { delete this.watcherWorkspaces[watchPath]; } }
120-122: Consider explicit boolean return for clarity.The method name
hasWatcherimplies a boolean return, but it currently returns the watcher object or undefined. While this works for boolean checks, an explicit boolean return would be clearer.hasWatcher(watchPath) { - return this.watchers[watchPath]; + return !!this.watchers[watchPath]; }Alternatively, if external code needs the watcher instance, consider renaming to
getWatcher.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (37)
packages/bruno-app/package.json(2 hunks)packages/bruno-app/src/components/ApiSpecPanel/FileEditor/CodeEditor/Plugins/Yaml/index.js(1 hunks)packages/bruno-app/src/components/ApiSpecPanel/FileEditor/CodeEditor/StyledWrapper.js(1 hunks)packages/bruno-app/src/components/ApiSpecPanel/FileEditor/CodeEditor/index.js(1 hunks)packages/bruno-app/src/components/ApiSpecPanel/FileEditor/index.js(1 hunks)packages/bruno-app/src/components/ApiSpecPanel/Renderers/Swagger/StyledWrapper.js(1 hunks)packages/bruno-app/src/components/ApiSpecPanel/Renderers/Swagger/index.js(1 hunks)packages/bruno-app/src/components/ApiSpecPanel/StyledWrapper.js(1 hunks)packages/bruno-app/src/components/ApiSpecPanel/index.js(1 hunks)packages/bruno-app/src/components/Sidebar/ApiSpecs/ApiSpecBadge/StyledWrapper.js(1 hunks)packages/bruno-app/src/components/Sidebar/ApiSpecs/ApiSpecBadge/index.js(1 hunks)packages/bruno-app/src/components/Sidebar/ApiSpecs/ApiSpecItem/index.js(1 hunks)packages/bruno-app/src/components/Sidebar/ApiSpecs/CloseApiSpec/index.js(1 hunks)packages/bruno-app/src/components/Sidebar/ApiSpecs/CreateApiSpec/StyledWrapper.js(1 hunks)packages/bruno-app/src/components/Sidebar/ApiSpecs/CreateApiSpec/index.js(1 hunks)packages/bruno-app/src/components/Sidebar/ApiSpecs/StyledWrapper.js(1 hunks)packages/bruno-app/src/components/Sidebar/ApiSpecs/index.js(1 hunks)packages/bruno-app/src/components/Sidebar/Collections/index.js(2 hunks)packages/bruno-app/src/components/Sidebar/SidebarHeader/StyledWrapper.js(0 hunks)packages/bruno-app/src/components/Sidebar/SidebarHeader/index.js(7 hunks)packages/bruno-app/src/components/Sidebar/index.js(1 hunks)packages/bruno-app/src/pages/Bruno/index.js(3 hunks)packages/bruno-app/src/providers/App/useIpcEvents.js(3 hunks)packages/bruno-app/src/providers/ReduxStore/index.js(2 hunks)packages/bruno-app/src/providers/ReduxStore/slices/apiSpec.js(1 hunks)packages/bruno-app/src/providers/ReduxStore/slices/app.js(3 hunks)packages/bruno-app/src/providers/ReduxStore/slices/workspaces/actions.js(3 hunks)packages/bruno-app/src/utils/exporters/openapi-spec.js(1 hunks)packages/bruno-electron/src/app/apiSpecs.js(1 hunks)packages/bruno-electron/src/app/apiSpecsWatcher.js(1 hunks)packages/bruno-electron/src/cache/apiSpecUids.js(1 hunks)packages/bruno-electron/src/index.js(3 hunks)packages/bruno-electron/src/ipc/apiSpec.js(1 hunks)packages/bruno-electron/src/ipc/collection.js(3 hunks)packages/bruno-electron/src/ipc/workspace.js(1 hunks)packages/bruno-electron/src/utils/filesystem.js(2 hunks)packages/bruno-electron/src/utils/workspace-config.js(3 hunks)
💤 Files with no reviewable changes (1)
- packages/bruno-app/src/components/Sidebar/SidebarHeader/StyledWrapper.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/Sidebar/ApiSpecs/ApiSpecBadge/StyledWrapper.jspackages/bruno-app/src/providers/ReduxStore/index.jspackages/bruno-app/src/components/Sidebar/ApiSpecs/CloseApiSpec/index.jspackages/bruno-app/src/components/ApiSpecPanel/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/ApiSpecs/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/ApiSpecs/ApiSpecItem/index.jspackages/bruno-app/src/pages/Bruno/index.jspackages/bruno-app/src/components/ApiSpecPanel/Renderers/Swagger/index.jspackages/bruno-app/src/components/Sidebar/ApiSpecs/index.jspackages/bruno-electron/src/ipc/workspace.jspackages/bruno-app/src/components/ApiSpecPanel/FileEditor/CodeEditor/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/Collections/index.jspackages/bruno-app/src/components/Sidebar/ApiSpecs/ApiSpecBadge/index.jspackages/bruno-app/src/components/ApiSpecPanel/FileEditor/index.jspackages/bruno-app/src/components/Sidebar/index.jspackages/bruno-app/src/components/ApiSpecPanel/Renderers/Swagger/StyledWrapper.jspackages/bruno-electron/src/app/apiSpecs.jspackages/bruno-app/src/utils/exporters/openapi-spec.jspackages/bruno-electron/src/utils/workspace-config.jspackages/bruno-electron/src/cache/apiSpecUids.jspackages/bruno-app/src/components/Sidebar/ApiSpecs/CreateApiSpec/StyledWrapper.jspackages/bruno-electron/src/utils/filesystem.jspackages/bruno-app/src/providers/ReduxStore/slices/apiSpec.jspackages/bruno-app/src/components/ApiSpecPanel/FileEditor/CodeEditor/index.jspackages/bruno-app/src/components/ApiSpecPanel/index.jspackages/bruno-electron/src/ipc/apiSpec.jspackages/bruno-app/src/providers/ReduxStore/slices/workspaces/actions.jspackages/bruno-app/src/providers/ReduxStore/slices/app.jspackages/bruno-electron/src/index.jspackages/bruno-app/src/providers/App/useIpcEvents.jspackages/bruno-app/src/components/ApiSpecPanel/FileEditor/CodeEditor/Plugins/Yaml/index.jspackages/bruno-app/src/components/Sidebar/ApiSpecs/CreateApiSpec/index.jspackages/bruno-electron/src/app/apiSpecsWatcher.jspackages/bruno-electron/src/ipc/collection.jspackages/bruno-app/src/components/Sidebar/SidebarHeader/index.js
🧠 Learnings (3)
📚 Learning: 2025-12-05T20:31:32.975Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:32.975Z
Learning: Applies to **/*.{jsx,tsx} : Styled Components are used as wrappers to define both self and children components style; Tailwind classes are used specifically for layout based styles
Applied to files:
packages/bruno-app/src/components/Sidebar/ApiSpecs/ApiSpecBadge/StyledWrapper.jspackages/bruno-app/src/components/ApiSpecPanel/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/ApiSpecs/StyledWrapper.jspackages/bruno-app/src/components/ApiSpecPanel/FileEditor/CodeEditor/StyledWrapper.jspackages/bruno-app/src/components/ApiSpecPanel/Renderers/Swagger/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/ApiSpecs/CreateApiSpec/StyledWrapper.js
📚 Learning: 2025-12-05T20:31:32.975Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:32.975Z
Learning: Applies to **/*.{jsx,tsx} : Use styled component's theme prop to manage CSS colors and not CSS variables when in the context of a styled component or any React component using the styled component
Applied to files:
packages/bruno-app/src/components/Sidebar/ApiSpecs/ApiSpecBadge/StyledWrapper.jspackages/bruno-app/src/components/ApiSpecPanel/StyledWrapper.jspackages/bruno-app/src/components/ApiSpecPanel/FileEditor/CodeEditor/StyledWrapper.jspackages/bruno-app/src/components/ApiSpecPanel/Renderers/Swagger/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/ApiSpecs/CreateApiSpec/StyledWrapper.js
📚 Learning: 2025-12-05T20:31:32.975Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:32.975Z
Learning: Applies to **/*.{jsx,tsx} : Styled Component CSS might also change layout but Tailwind classes shouldn't define colors
Applied to files:
packages/bruno-app/src/components/Sidebar/ApiSpecs/ApiSpecBadge/StyledWrapper.jspackages/bruno-app/src/components/ApiSpecPanel/StyledWrapper.jspackages/bruno-app/src/components/ApiSpecPanel/FileEditor/CodeEditor/StyledWrapper.jspackages/bruno-app/src/components/ApiSpecPanel/Renderers/Swagger/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/ApiSpecs/CreateApiSpec/StyledWrapper.js
🧬 Code graph analysis (17)
packages/bruno-app/src/components/Sidebar/ApiSpecs/ApiSpecBadge/StyledWrapper.js (4)
packages/bruno-app/src/components/ApiSpecPanel/FileEditor/CodeEditor/StyledWrapper.js (1)
StyledWrapper(3-63)packages/bruno-app/src/components/ApiSpecPanel/StyledWrapper.js (1)
StyledWrapper(3-20)packages/bruno-app/src/components/Sidebar/ApiSpecs/CreateApiSpec/StyledWrapper.js (1)
StyledWrapper(3-13)packages/bruno-app/src/components/Sidebar/SidebarHeader/StyledWrapper.js (1)
StyledWrapper(3-104)
packages/bruno-app/src/components/Sidebar/ApiSpecs/CloseApiSpec/index.js (2)
packages/bruno-app/src/providers/ReduxStore/slices/apiSpec.js (5)
closeApiSpecFile(131-162)closeApiSpecFile(131-162)apiSpec(19-19)apiSpec(43-43)apiSpec(54-54)packages/bruno-app/src/components/Modal/index.js (1)
Modal(60-161)
packages/bruno-app/src/components/ApiSpecPanel/StyledWrapper.js (5)
packages/bruno-app/src/components/ApiSpecPanel/FileEditor/CodeEditor/StyledWrapper.js (1)
StyledWrapper(3-63)packages/bruno-app/src/components/ApiSpecPanel/Renderers/Swagger/StyledWrapper.js (1)
StyledWrapper(3-17)packages/bruno-app/src/components/Sidebar/ApiSpecs/ApiSpecBadge/StyledWrapper.js (1)
StyledWrapper(3-9)packages/bruno-app/src/components/Sidebar/ApiSpecs/CreateApiSpec/StyledWrapper.js (1)
StyledWrapper(3-13)packages/bruno-app/src/components/Sidebar/SidebarHeader/StyledWrapper.js (1)
StyledWrapper(3-104)
packages/bruno-app/src/components/Sidebar/ApiSpecs/ApiSpecItem/index.js (5)
packages/bruno-app/src/components/Sidebar/ApiSpecs/CloseApiSpec/index.js (2)
dispatch(9-9)CloseApiSpec(8-35)packages/bruno-app/src/components/Sidebar/ApiSpecs/index.js (2)
dispatch(16-16)handleOpenApiSpec(35-39)packages/bruno-app/src/pages/Bruno/index.js (2)
activeApiSpecUid(57-57)showApiSpecPage(60-60)packages/bruno-app/src/providers/ReduxStore/slices/apiSpec.js (3)
apiSpec(19-19)apiSpec(43-43)apiSpec(54-54)packages/bruno-app/src/components/Dropdown/index.js (1)
Dropdown(5-25)
packages/bruno-app/src/pages/Bruno/index.js (1)
packages/bruno-app/src/components/ApiSpecPanel/index.js (2)
useSelector(20-20)ApiSpecPanel(15-95)
packages/bruno-app/src/components/Sidebar/ApiSpecs/ApiSpecBadge/index.js (1)
packages/bruno-app/src/components/Sidebar/ApiSpecs/ApiSpecBadge/StyledWrapper.js (1)
StyledWrapper(3-9)
packages/bruno-app/src/components/ApiSpecPanel/FileEditor/index.js (2)
packages/bruno-app/src/providers/ReduxStore/slices/apiSpec.js (5)
apiSpec(19-19)apiSpec(43-43)apiSpec(54-54)saveApiSpecToFile(95-115)saveApiSpecToFile(95-115)packages/bruno-app/src/components/ApiSpecPanel/FileEditor/CodeEditor/index.js (1)
CodeEditor(19-138)
packages/bruno-app/src/components/Sidebar/index.js (1)
packages/bruno-app/src/components/Sidebar/Collections/index.js (1)
Collections(12-60)
packages/bruno-electron/src/app/apiSpecs.js (6)
packages/bruno-electron/src/app/apiSpecsWatcher.js (7)
require(5-5)require(7-7)require(8-8)fs(2-2)path(3-3)yaml(6-6)content(17-17)packages/bruno-electron/src/cache/apiSpecUids.js (3)
require(14-14)uid(17-17)uid(28-28)packages/bruno-electron/src/utils/filesystem.js (5)
normalizeAndResolvePath(46-61)fs(2-2)path(1-1)ext(100-100)ext(317-317)packages/bruno-app/src/providers/ReduxStore/slices/apiSpec.js (2)
openApiSpec(81-93)openApiSpec(81-93)packages/bruno-electron/src/ipc/workspace.js (3)
fs(1-1)path(2-2)yaml(7-7)packages/bruno-electron/src/utils/workspace-config.js (8)
fs(1-1)path(2-2)yaml(3-3)workspaceFilePath(48-48)workspaceFilePath(73-73)yamlContent(79-79)yamlContent(90-94)workspaceConfig(80-80)
packages/bruno-electron/src/utils/workspace-config.js (3)
packages/bruno-app/src/components/Sidebar/ApiSpecs/index.js (1)
apiSpecs(23-33)packages/bruno-electron/src/app/apiSpecs.js (1)
path(26-26)packages/bruno-electron/src/utils/filesystem.js (1)
path(1-1)
packages/bruno-electron/src/cache/apiSpecUids.js (1)
packages/bruno-electron/src/app/apiSpecsWatcher.js (3)
require(5-5)require(7-7)require(8-8)
packages/bruno-app/src/components/ApiSpecPanel/index.js (4)
packages/bruno-app/src/providers/ReduxStore/slices/apiSpec.js (5)
apiSpec(19-19)apiSpec(43-43)apiSpec(54-54)openApiSpec(81-93)openApiSpec(81-93)packages/bruno-app/src/components/ApiSpecPanel/Renderers/Swagger/StyledWrapper.js (1)
StyledWrapper(3-17)packages/bruno-app/src/components/Dropdown/index.js (1)
Dropdown(5-25)packages/bruno-app/src/components/ApiSpecPanel/Renderers/Swagger/index.js (1)
Swagger(5-17)
packages/bruno-electron/src/ipc/apiSpec.js (5)
packages/bruno-electron/src/app/apiSpecs.js (7)
path(26-26)fs(25-25)openApiSpecDialog(5-18)openApiSpec(20-105)yaml(27-27)workspaceFilePath(29-29)uid(22-22)packages/bruno-electron/src/cache/apiSpecUids.js (3)
removeApiSpecUid(36-38)uid(17-17)uid(28-28)packages/bruno-electron/src/index.js (2)
path(2-2)fs(1-1)packages/bruno-electron/src/ipc/workspace.js (3)
path(2-2)fs(1-1)yaml(7-7)packages/bruno-electron/src/utils/workspace-config.js (5)
path(2-2)fs(1-1)yaml(3-3)workspaceFilePath(48-48)workspaceFilePath(73-73)
packages/bruno-app/src/providers/ReduxStore/slices/app.js (1)
packages/bruno-app/src/providers/ReduxStore/middlewares/tasks/middleware.js (2)
state(23-23)state(64-64)
packages/bruno-electron/src/index.js (1)
packages/bruno-electron/src/ipc/apiSpec.js (1)
registerApiSpecIpc(117-120)
packages/bruno-app/src/components/Sidebar/ApiSpecs/CreateApiSpec/index.js (3)
packages/bruno-electron/src/utils/filesystem.js (3)
validateName(229-244)files(42-42)files(155-155)packages/bruno-app/src/utils/exporters/openapi-spec.js (2)
exportApiSpec(6-376)exportApiSpec(6-376)packages/bruno-app/src/providers/ReduxStore/slices/apiSpec.js (2)
createApiSpecFile(117-129)createApiSpecFile(117-129)
packages/bruno-app/src/components/Sidebar/SidebarHeader/index.js (4)
packages/bruno-app/src/components/Sidebar/ApiSpecs/ApiSpecItem/index.js (2)
handleOpenApiSpec(21-24)dispatch(11-11)packages/bruno-app/src/components/Sidebar/ApiSpecs/index.js (2)
handleOpenApiSpec(35-39)dispatch(16-16)packages/bruno-app/src/components/Sidebar/ApiSpecs/CreateApiSpec/index.js (2)
dispatch(28-28)CreateApiSpec(26-324)packages/bruno-app/src/providers/ReduxStore/slices/apiSpec.js (2)
openApiSpec(81-93)openApiSpec(81-93)
🪛 Biome (2.1.2)
packages/bruno-app/src/utils/exporters/openapi-spec.js
[error] 62-62: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
[error] 69-69: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
[error] 76-76: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
[error] 174-194: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 176-179: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 195-215: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 197-200: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 216-226: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 256-256: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 256-256: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
⏰ 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: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - macOS
- GitHub Check: Playwright E2E Tests
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
| componentWillMount() { | ||
| switch (this.props.mode) { | ||
| case 'yaml': | ||
| // YAML linting and hightlighting plugin | ||
| yamlPlugin(CodeMirror); | ||
| break; | ||
| default: | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
Deprecated lifecycle: componentWillMount removed in React 19.
This lifecycle method was deprecated in React 16.3 and removed in React 19. Move this logic to the constructor or componentDidMount.
constructor(props) {
super(props);
this.cachedValue = props.value || '';
this.variables = {};
this.lintOptions = {
esversion: 11,
expr: true,
asi: true
};
+ // Initialize mode-specific plugins
+ if (props.mode === 'yaml') {
+ yamlPlugin(CodeMirror);
+ }
}
-
- componentWillMount() {
- switch (this.props.mode) {
- case 'yaml':
- // YAML linting and hightlighting plugin
- yamlPlugin(CodeMirror);
- break;
- default:
- break;
- }
- }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/bruno-app/src/components/ApiSpecPanel/FileEditor/CodeEditor/index.js
around lines 31-40, componentWillMount is deprecated/removed; move the
yamlPlugin(CodeMirror) registration into the constructor or componentDidMount.
Replace the lifecycle with a one-time registration in the constructor (or
componentDidMount if it requires the DOM) and add a guard (static/closure
boolean) so the plugin is only registered once across instances to avoid
duplicate registrations.
| const handleOpenApiSpec = () => { | ||
| dispatch(openApiSpec()).catch( | ||
| (err) => console.log(err) && toast.error('An error occurred while opening the API spec') | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Bug: toast.error will never execute.
console.log() returns undefined, which is falsy, so the && short-circuits and toast.error() is never called.
const handleOpenApiSpec = () => {
- dispatch(openApiSpec()).catch(
- (err) => console.log(err) && toast.error('An error occurred while opening the API spec')
- );
+ dispatch(openApiSpec()).catch((err) => {
+ console.error(err);
+ toast.error('An error occurred while opening the API spec');
+ });
};📝 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.
| const handleOpenApiSpec = () => { | |
| dispatch(openApiSpec()).catch( | |
| (err) => console.log(err) && toast.error('An error occurred while opening the API spec') | |
| ); | |
| }; | |
| const handleOpenApiSpec = () => { | |
| dispatch(openApiSpec()).catch((err) => { | |
| console.error(err); | |
| toast.error('An error occurred while opening the API spec'); | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In packages/bruno-app/src/components/ApiSpecPanel/index.js around lines 39 to
43, the catch handler uses `console.log(err) && toast.error(...)` so
`toast.error` never runs because `console.log` returns undefined; change the
catch to call both functions explicitly (for example, use a block: `.catch(err
=> { console.log(err); toast.error('An error occurred while opening the API
spec'); });`) so the error is logged and the toast is shown.
| const handleOpenApiSpec = () => { | ||
| dispatch(openApiSpec()).catch( | ||
| (err) => console.log(err) && toast.error('An error occurred while opening the API spec') | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Bug: Toast error will never show.
console.log(err) returns undefined, so the && short-circuits and toast.error() never executes.
const handleOpenApiSpec = () => {
- dispatch(openApiSpec()).catch(
- (err) => console.log(err) && toast.error('An error occurred while opening the API spec')
- );
+ dispatch(openApiSpec()).catch((err) => {
+ console.error(err);
+ toast.error('An error occurred while opening the API spec');
+ });
};🤖 Prompt for AI Agents
In packages/bruno-app/src/components/Sidebar/ApiSpecs/index.js around lines 35
to 39, the .catch handler uses console.log(err) && toast.error(...), but
console.log returns undefined so the toast never runs; change the catch to a
proper block or sequence so both actions run (for example: .catch(err => {
console.log(err); toast.error('An error occurred while opening the API spec');
}) or use console.error(err); toast.error(...)) ensuring the error is logged and
the toast is shown.
| .catch((reject) => { | ||
| toast.error('Error saving file'); | ||
| resolve(); | ||
| }); |
There was a problem hiding this comment.
Critical bug: Error swallowed and incorrectly resolved.
The catch block shadows reject with the error parameter and then calls resolve() instead of rejecting. Errors will appear as successes to callers.
- .catch((reject) => {
+ .catch((error) => {
toast.error('Error saving file');
- resolve();
+ reject(error);
});🤖 Prompt for AI Agents
In packages/bruno-app/src/providers/ReduxStore/slices/apiSpec.js around lines
110 to 113, the catch handler incorrectly names the error parameter `reject` and
then calls resolve(), which swallows errors and signals success; change the
catch parameter to a descriptive name (e.g., err or error), call reject(err) (or
return Promise.reject(err)) so the caller receives the error, and optionally
include the error message in the toast (e.g., toast.error(`Error saving file:
${err.message}`)) to preserve original error semantics and logging.
| case 'multipartForm': | ||
| if (!body?.multipartForm) return; | ||
| let multipartFormToKeyValue = body?.multipartForm.reduce((acc, f) => { | ||
| acc[f?.name] = f.value; | ||
| return acc; | ||
| }, {}); | ||
| components.schemas[schemaId] = generateProperyShape(multipartFormToKeyValue); | ||
| components.requestBodies[requestBodyId] = { | ||
| content: { | ||
| 'multipart/form-data:': { | ||
| schema: { | ||
| $ref: `#/components/schemas/${schemaId}` | ||
| } | ||
| } | ||
| }, | ||
| description: '', | ||
| required: true | ||
| }; | ||
| pathBody['requestBody'] = { | ||
| $ref: `#/components/requestBodies/${requestBodyId}` | ||
| }; | ||
| case 'formUrlEncoded': | ||
| if (!body?.formUrlEncoded) return; | ||
| let formUrlEncodedToKeyValue = body?.formUrlEncoded.reduce((acc, f) => { | ||
| acc[f?.name] = f.value; | ||
| return acc; | ||
| }, {}); | ||
| components.schemas[schemaId] = generateProperyShape(formUrlEncodedToKeyValue); | ||
| components.requestBodies[requestBodyId] = { | ||
| content: { | ||
| 'application/x-www-form-urlencoded:': { | ||
| schema: { | ||
| $ref: `#/components/schemas/${schemaId}` | ||
| } | ||
| } | ||
| }, | ||
| description: '', | ||
| required: true | ||
| }; | ||
| pathBody['requestBody'] = { | ||
| $ref: `#/components/requestBodies/${requestBodyId}` | ||
| }; | ||
| case 'text': | ||
| if (!body?.text) return; | ||
| pathBody['requestBody'] = { | ||
| content: { | ||
| 'text/plain': { | ||
| schema: { | ||
| type: 'string' | ||
| } | ||
| } | ||
| } | ||
| }; | ||
| default: | ||
| break; |
There was a problem hiding this comment.
Critical: Missing break statements cause fallthrough.
Cases multipartForm, formUrlEncoded, and text all fall through. Additionally, they use return which exits the entire map callback, not just the switch. This corrupts the output for affected items.
case 'multipartForm':
- if (!body?.multipartForm) return;
+ if (!body?.multipartForm) break;
let multipartFormToKeyValue = body?.multipartForm.reduce((acc, f) => {
// ...
});
// ...
+ break;
case 'formUrlEncoded':
- if (!body?.formUrlEncoded) return;
+ if (!body?.formUrlEncoded) break;
let formUrlEncodedToKeyValue = body?.formUrlEncoded.reduce((acc, f) => {
// ...
});
// ...
+ break;
case 'text':
- if (!body?.text) return;
+ if (!body?.text) break;
pathBody['requestBody'] = { /* ... */ };
+ break;
default:
break;📝 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.
| case 'multipartForm': | |
| if (!body?.multipartForm) return; | |
| let multipartFormToKeyValue = body?.multipartForm.reduce((acc, f) => { | |
| acc[f?.name] = f.value; | |
| return acc; | |
| }, {}); | |
| components.schemas[schemaId] = generateProperyShape(multipartFormToKeyValue); | |
| components.requestBodies[requestBodyId] = { | |
| content: { | |
| 'multipart/form-data:': { | |
| schema: { | |
| $ref: `#/components/schemas/${schemaId}` | |
| } | |
| } | |
| }, | |
| description: '', | |
| required: true | |
| }; | |
| pathBody['requestBody'] = { | |
| $ref: `#/components/requestBodies/${requestBodyId}` | |
| }; | |
| case 'formUrlEncoded': | |
| if (!body?.formUrlEncoded) return; | |
| let formUrlEncodedToKeyValue = body?.formUrlEncoded.reduce((acc, f) => { | |
| acc[f?.name] = f.value; | |
| return acc; | |
| }, {}); | |
| components.schemas[schemaId] = generateProperyShape(formUrlEncodedToKeyValue); | |
| components.requestBodies[requestBodyId] = { | |
| content: { | |
| 'application/x-www-form-urlencoded:': { | |
| schema: { | |
| $ref: `#/components/schemas/${schemaId}` | |
| } | |
| } | |
| }, | |
| description: '', | |
| required: true | |
| }; | |
| pathBody['requestBody'] = { | |
| $ref: `#/components/requestBodies/${requestBodyId}` | |
| }; | |
| case 'text': | |
| if (!body?.text) return; | |
| pathBody['requestBody'] = { | |
| content: { | |
| 'text/plain': { | |
| schema: { | |
| type: 'string' | |
| } | |
| } | |
| } | |
| }; | |
| default: | |
| break; | |
| case 'multipartForm': | |
| if (!body?.multipartForm) break; | |
| let multipartFormToKeyValue = body?.multipartForm.reduce((acc, f) => { | |
| acc[f?.name] = f.value; | |
| return acc; | |
| }, {}); | |
| components.schemas[schemaId] = generateProperyShape(multipartFormToKeyValue); | |
| components.requestBodies[requestBodyId] = { | |
| content: { | |
| 'multipart/form-data:': { | |
| schema: { | |
| $ref: `#/components/schemas/${schemaId}` | |
| } | |
| } | |
| }, | |
| description: '', | |
| required: true | |
| }; | |
| pathBody['requestBody'] = { | |
| $ref: `#/components/requestBodies/${requestBodyId}` | |
| }; | |
| break; | |
| case 'formUrlEncoded': | |
| if (!body?.formUrlEncoded) break; | |
| let formUrlEncodedToKeyValue = body?.formUrlEncoded.reduce((acc, f) => { | |
| acc[f?.name] = f.value; | |
| return acc; | |
| }, {}); | |
| components.schemas[schemaId] = generateProperyShape(formUrlEncodedToKeyValue); | |
| components.requestBodies[requestBodyId] = { | |
| content: { | |
| 'application/x-www-form-urlencoded:': { | |
| schema: { | |
| $ref: `#/components/schemas/${schemaId}` | |
| } | |
| } | |
| }, | |
| description: '', | |
| required: true | |
| }; | |
| pathBody['requestBody'] = { | |
| $ref: `#/components/requestBodies/${requestBodyId}` | |
| }; | |
| break; | |
| case 'text': | |
| if (!body?.text) break; | |
| pathBody['requestBody'] = { | |
| content: { | |
| 'text/plain': { | |
| schema: { | |
| type: 'string' | |
| } | |
| } | |
| } | |
| }; | |
| break; | |
| default: | |
| break; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 174-194: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 176-179: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 195-215: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 197-200: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 216-226: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
🤖 Prompt for AI Agents
In packages/bruno-app/src/utils/exporters/openapi-spec.js around lines 174-228,
the switch cases for 'multipartForm', 'formUrlEncoded', and 'text' are missing
break statements and currently use `return` inside each case which exits the
surrounding map callback; change each `if (!body?....) return;` to `if
(!body?....) break;` and add an explicit `break;` at the end of each case after
assigning pathBody (so execution does not fall through to the next case),
ensuring the switch only handles the intended branch and the map callback
continues normally.
| components.schemas[schemaId] = generateProperyShape(multipartFormToKeyValue); | ||
| components.requestBodies[requestBodyId] = { | ||
| content: { | ||
| 'multipart/form-data:': { |
There was a problem hiding this comment.
Typo: Trailing colons in content-type keys.
'multipart/form-data:' and 'application/x-www-form-urlencoded:' have erroneous trailing colons, producing invalid OpenAPI specs.
- 'multipart/form-data:': {
+ 'multipart/form-data': {
// ...
- 'application/x-www-form-urlencoded:': {
+ 'application/x-www-form-urlencoded': {Also applies to: 204-204
🤖 Prompt for AI Agents
In packages/bruno-app/src/utils/exporters/openapi-spec.js around lines 183 and
204, the media type keys include stray trailing colons ('multipart/form-data:'
and 'application/x-www-form-urlencoded:'), which yields invalid OpenAPI. Remove
the trailing colons so the keys are exact media types ('multipart/form-data' and
'application/x-www-form-urlencoded'), and run a quick spec validation to confirm
the produced OpenAPI is valid.
| ipcMain.on('main:apispec-opened', (win, pathname, uid, workspacePath = null) => { | ||
| watcher.addWatcher(win, pathname, uid, {}, workspacePath); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "emit.*main:apispec-opened" --type=jsRepository: usebruno/bruno
Length of output: 172
🏁 Script executed:
rg -n "main:apispec-opened" --type=js -B 2 -A 2Repository: usebruno/bruno
Length of output: 172
🌐 Web query:
Electron ipcMain.on callback signature event argument
💡 Result:
The listener signature is:
- ipcMain.on(channel, (event: IpcMainEvent, ...args: any[]) => void)
The IpcMainEvent (event) provides members you typically use:
- event.sender — the WebContents that sent the message (use sender.send(...) to reply). [1]
- event.returnValue — set this to reply for synchronous messages. [1]
(See Electron docs for IpcMainEvent and ipcMain.on.) [1][2]
Sources:
[1] Electron docs — ipcMain (listener signature, event.returnValue, event.sender). [2] Electron docs — ipcRenderer/ipc-main event typing (IpcMainEvent, ...args).
Fix ipcMain.on callback signature — first argument should be event, not win.
The standard Electron ipcMain.on signature is (event: IpcMainEvent, ...args: any[]) => void. The first argument must be an event object that provides event.sender and event.returnValue. The current code incorrectly declares (win, pathname, uid, workspacePath) as parameters. This will cause runtime errors when the event is emitted, as the first argument will be an IpcMainEvent object, not a window.
🤖 Prompt for AI Agents
In packages/bruno-electron/src/ipc/apiSpec.js around lines 112-114, the
ipcMain.on callback currently declares (win, pathname, uid, workspacePath) but
Electron passes an IpcMainEvent as the first argument; change the parameter list
to (event, pathname, uid, workspacePath = null) and use event.sender (or
event.sender.getOwnerBrowserWindow() if a BrowserWindow is needed) when calling
watcher.addWatcher so the watcher receives the actual window/webContents; keep
workspacePath defaulting to null.
2 participants
Description
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
Release Notes
New Features
Dependencies
✏️ Tip: You can customize this high-level summary in your review settings.